From 2c860df34bc245a621efb8e7799eef4f68b341ce Mon Sep 17 00:00:00 2001 From: Thomas Rittson <31796059+eliykat@users.noreply.github.com> Date: Thu, 11 Sep 2025 13:58:32 +1000 Subject: [PATCH] [PM-15621] Refactor delete claimed user command (#6221) - create vNext command - restructure command to simplify logic - move validation to a separate class - implement result types using OneOf library and demo their use here --- .../OrganizationUsersController.cs | 52 ++ .../OrganizationUserResponseModel.cs | 4 +- .../CommandResult.cs | 42 ++ ...imedOrganizationUserAccountCommandvNext.cs | 137 +++++ ...ClaimedOrganizationUserAccountValidator.cs | 76 +++ .../DeleteUserValidationRequest.cs | 13 + .../DeleteClaimedAccountvNext/Errors.cs | 21 + ...imedOrganizationUserAccountCommandvNext.cs | 17 + ...edOrganizationUserAccountValidatorvNext.cs | 6 + .../ValidationResult.cs | 41 ++ src/Core/Constants.cs | 1 + ...OrganizationServiceCollectionExtensions.cs | 5 + .../OrganizationUserControllerTests.cs | 121 ++++- .../OrganizationUsersControllerTests.cs | 21 - ...rganizationUserAccountCommandvNextTests.cs | 467 ++++++++++++++++ ...anizationUserAccountValidatorvNextTests.cs | 503 ++++++++++++++++++ 16 files changed, 1502 insertions(+), 25 deletions(-) create mode 100644 src/Core/AdminConsole/OrganizationFeatures/OrganizationUsers/DeleteClaimedAccountvNext/CommandResult.cs create mode 100644 src/Core/AdminConsole/OrganizationFeatures/OrganizationUsers/DeleteClaimedAccountvNext/DeleteClaimedOrganizationUserAccountCommandvNext.cs create mode 100644 src/Core/AdminConsole/OrganizationFeatures/OrganizationUsers/DeleteClaimedAccountvNext/DeleteClaimedOrganizationUserAccountValidator.cs create mode 100644 src/Core/AdminConsole/OrganizationFeatures/OrganizationUsers/DeleteClaimedAccountvNext/DeleteUserValidationRequest.cs create mode 100644 src/Core/AdminConsole/OrganizationFeatures/OrganizationUsers/DeleteClaimedAccountvNext/Errors.cs create mode 100644 src/Core/AdminConsole/OrganizationFeatures/OrganizationUsers/DeleteClaimedAccountvNext/IDeleteClaimedOrganizationUserAccountCommandvNext.cs create mode 100644 src/Core/AdminConsole/OrganizationFeatures/OrganizationUsers/DeleteClaimedAccountvNext/IDeleteClaimedOrganizationUserAccountValidatorvNext.cs create mode 100644 src/Core/AdminConsole/OrganizationFeatures/OrganizationUsers/DeleteClaimedAccountvNext/ValidationResult.cs create mode 100644 test/Core.Test/AdminConsole/OrganizationFeatures/OrganizationUsers/DeleteClaimedAccountvNext/DeleteClaimedOrganizationUserAccountCommandvNextTests.cs create mode 100644 test/Core.Test/AdminConsole/OrganizationFeatures/OrganizationUsers/DeleteClaimedAccountvNext/DeleteClaimedOrganizationUserAccountValidatorvNextTests.cs diff --git a/src/Api/AdminConsole/Controllers/OrganizationUsersController.cs b/src/Api/AdminConsole/Controllers/OrganizationUsersController.cs index 5183b59b00..16d6984334 100644 --- a/src/Api/AdminConsole/Controllers/OrganizationUsersController.cs +++ b/src/Api/AdminConsole/Controllers/OrganizationUsersController.cs @@ -11,6 +11,7 @@ using Bit.Api.Vault.AuthorizationHandlers.Collections; using Bit.Core; using Bit.Core.AdminConsole.Enums; using Bit.Core.AdminConsole.Models.Data.Organizations.Policies; +using Bit.Core.AdminConsole.OrganizationFeatures.OrganizationUsers.DeleteClaimedAccountvNext; using Bit.Core.AdminConsole.OrganizationFeatures.OrganizationUsers.Interfaces; using Bit.Core.AdminConsole.OrganizationFeatures.OrganizationUsers.InviteUsers; using Bit.Core.AdminConsole.OrganizationFeatures.OrganizationUsers.RestoreUser.v1; @@ -23,6 +24,7 @@ using Bit.Core.Billing.Pricing; using Bit.Core.Context; using Bit.Core.Enums; using Bit.Core.Exceptions; +using Bit.Core.Models.Api; using Bit.Core.Models.Business; using Bit.Core.Models.Data.Organizations.OrganizationUsers; using Bit.Core.OrganizationFeatures.OrganizationSubscriptions.Interface; @@ -59,6 +61,7 @@ public class OrganizationUsersController : Controller private readonly IOrganizationUserUserDetailsQuery _organizationUserUserDetailsQuery; private readonly IRemoveOrganizationUserCommand _removeOrganizationUserCommand; private readonly IDeleteClaimedOrganizationUserAccountCommand _deleteClaimedOrganizationUserAccountCommand; + private readonly IDeleteClaimedOrganizationUserAccountCommandvNext _deleteClaimedOrganizationUserAccountCommandvNext; private readonly IGetOrganizationUsersClaimedStatusQuery _getOrganizationUsersClaimedStatusQuery; private readonly IPolicyRequirementQuery _policyRequirementQuery; private readonly IFeatureService _featureService; @@ -87,6 +90,7 @@ public class OrganizationUsersController : Controller IOrganizationUserUserDetailsQuery organizationUserUserDetailsQuery, IRemoveOrganizationUserCommand removeOrganizationUserCommand, IDeleteClaimedOrganizationUserAccountCommand deleteClaimedOrganizationUserAccountCommand, + IDeleteClaimedOrganizationUserAccountCommandvNext deleteClaimedOrganizationUserAccountCommandvNext, IGetOrganizationUsersClaimedStatusQuery getOrganizationUsersClaimedStatusQuery, IPolicyRequirementQuery policyRequirementQuery, IFeatureService featureService, @@ -115,6 +119,7 @@ public class OrganizationUsersController : Controller _organizationUserUserDetailsQuery = organizationUserUserDetailsQuery; _removeOrganizationUserCommand = removeOrganizationUserCommand; _deleteClaimedOrganizationUserAccountCommand = deleteClaimedOrganizationUserAccountCommand; + _deleteClaimedOrganizationUserAccountCommandvNext = deleteClaimedOrganizationUserAccountCommandvNext; _getOrganizationUsersClaimedStatusQuery = getOrganizationUsersClaimedStatusQuery; _policyRequirementQuery = policyRequirementQuery; _featureService = featureService; @@ -536,6 +541,12 @@ public class OrganizationUsersController : Controller [Authorize] public async Task DeleteAccount(Guid orgId, Guid id) { + if (_featureService.IsEnabled(FeatureFlagKeys.DeleteClaimedUserAccountRefactor)) + { + await DeleteAccountvNext(orgId, id); + return; + } + var currentUser = await _userService.GetUserByPrincipalAsync(User); if (currentUser == null) { @@ -553,10 +564,33 @@ public class OrganizationUsersController : Controller await DeleteAccount(orgId, id); } + private async Task DeleteAccountvNext(Guid orgId, Guid id) + { + var currentUserId = _userService.GetProperUserId(User); + if (currentUserId == null) + { + return TypedResults.Unauthorized(); + } + + var commandResult = await _deleteClaimedOrganizationUserAccountCommandvNext.DeleteUserAsync(orgId, id, currentUserId.Value); + + return commandResult.Result.Match( + error => error is NotFoundError + ? TypedResults.NotFound(new ErrorResponseModel(error.Message)) + : TypedResults.BadRequest(new ErrorResponseModel(error.Message)), + TypedResults.Ok + ); + } + [HttpDelete("delete-account")] [Authorize] public async Task> BulkDeleteAccount(Guid orgId, [FromBody] OrganizationUserBulkRequestModel model) { + if (_featureService.IsEnabled(FeatureFlagKeys.DeleteClaimedUserAccountRefactor)) + { + return await BulkDeleteAccountvNext(orgId, model); + } + var currentUser = await _userService.GetUserByPrincipalAsync(User); if (currentUser == null) { @@ -577,6 +611,24 @@ public class OrganizationUsersController : Controller return await BulkDeleteAccount(orgId, model); } + private async Task> BulkDeleteAccountvNext(Guid orgId, [FromBody] OrganizationUserBulkRequestModel model) + { + var currentUserId = _userService.GetProperUserId(User); + if (currentUserId == null) + { + throw new UnauthorizedAccessException(); + } + + var result = await _deleteClaimedOrganizationUserAccountCommandvNext.DeleteManyUsersAsync(orgId, model.Ids, currentUserId.Value); + + var responses = result.Select(r => r.Result.Match( + error => new OrganizationUserBulkResponseModel(r.Id, error.Message), + _ => new OrganizationUserBulkResponseModel(r.Id, string.Empty) + )); + + return new ListResponseModel(responses); + } + [HttpPut("{id}/revoke")] [Authorize] public async Task RevokeAsync(Guid orgId, Guid id) diff --git a/src/Api/AdminConsole/Models/Response/Organizations/OrganizationUserResponseModel.cs b/src/Api/AdminConsole/Models/Response/Organizations/OrganizationUserResponseModel.cs index 7c31c2ae81..eb810599f3 100644 --- a/src/Api/AdminConsole/Models/Response/Organizations/OrganizationUserResponseModel.cs +++ b/src/Api/AdminConsole/Models/Response/Organizations/OrganizationUserResponseModel.cs @@ -236,8 +236,8 @@ public class OrganizationUserPublicKeyResponseModel : ResponseModel public class OrganizationUserBulkResponseModel : ResponseModel { - public OrganizationUserBulkResponseModel(Guid id, string error, - string obj = "OrganizationBulkConfirmResponseModel") : base(obj) + public OrganizationUserBulkResponseModel(Guid id, string error) + : base("OrganizationBulkConfirmResponseModel") { Id = id; Error = error; diff --git a/src/Core/AdminConsole/OrganizationFeatures/OrganizationUsers/DeleteClaimedAccountvNext/CommandResult.cs b/src/Core/AdminConsole/OrganizationFeatures/OrganizationUsers/DeleteClaimedAccountvNext/CommandResult.cs new file mode 100644 index 0000000000..3dfbe4dbda --- /dev/null +++ b/src/Core/AdminConsole/OrganizationFeatures/OrganizationUsers/DeleteClaimedAccountvNext/CommandResult.cs @@ -0,0 +1,42 @@ +using OneOf; +using OneOf.Types; + +namespace Bit.Core.AdminConsole.OrganizationFeatures.OrganizationUsers.DeleteClaimedAccountvNext; + +/// +/// Represents the result of a command. +/// This is a type that contains an Error if the command execution failed, or the result of the command if it succeeded. +/// +/// The type of the successful result. If there is no successful result (void), use . + +public class CommandResult(OneOf result) : OneOfBase(result) +{ + public bool IsError => IsT0; + public bool IsSuccess => IsT1; + public Error AsError => AsT0; + public T AsSuccess => AsT1; + + public static implicit operator CommandResult(T value) => new(value); + public static implicit operator CommandResult(Error error) => new(error); +} + +/// +/// Represents the result of a command where successful execution returns no value (void). +/// See for more information. +/// +public class CommandResult(OneOf result) : CommandResult(result) +{ + public static implicit operator CommandResult(None none) => new(none); + public static implicit operator CommandResult(Error error) => new(error); +} + +/// +/// A wrapper for with an ID, to identify the result in bulk operations. +/// +public record BulkCommandResult(Guid Id, CommandResult Result); + +/// +/// A wrapper for with an ID, to identify the result in bulk operations. +/// +public record BulkCommandResult(Guid Id, CommandResult Result); + diff --git a/src/Core/AdminConsole/OrganizationFeatures/OrganizationUsers/DeleteClaimedAccountvNext/DeleteClaimedOrganizationUserAccountCommandvNext.cs b/src/Core/AdminConsole/OrganizationFeatures/OrganizationUsers/DeleteClaimedAccountvNext/DeleteClaimedOrganizationUserAccountCommandvNext.cs new file mode 100644 index 0000000000..3064a426fa --- /dev/null +++ b/src/Core/AdminConsole/OrganizationFeatures/OrganizationUsers/DeleteClaimedAccountvNext/DeleteClaimedOrganizationUserAccountCommandvNext.cs @@ -0,0 +1,137 @@ +using Bit.Core.AdminConsole.OrganizationFeatures.OrganizationUsers.Interfaces; +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; +using Microsoft.Extensions.Logging; +using OneOf.Types; + +namespace Bit.Core.AdminConsole.OrganizationFeatures.OrganizationUsers.DeleteClaimedAccountvNext; + +public class DeleteClaimedOrganizationUserAccountCommandvNext( + IUserService userService, + IEventService eventService, + IGetOrganizationUsersClaimedStatusQuery getOrganizationUsersClaimedStatusQuery, + IOrganizationUserRepository organizationUserRepository, + IUserRepository userRepository, + IPushNotificationService pushService, + ILogger logger, + IDeleteClaimedOrganizationUserAccountValidatorvNext deleteClaimedOrganizationUserAccountValidatorvNext) + : IDeleteClaimedOrganizationUserAccountCommandvNext +{ + public async Task DeleteUserAsync(Guid organizationId, Guid organizationUserId, Guid deletingUserId) + { + var result = await DeleteManyUsersAsync(organizationId, [organizationUserId], deletingUserId); + return result.Single(); + } + + public async Task> DeleteManyUsersAsync(Guid organizationId, IEnumerable orgUserIds, Guid deletingUserId) + { + orgUserIds = orgUserIds.ToList(); + var orgUsers = await organizationUserRepository.GetManyAsync(orgUserIds); + var users = await GetUsersAsync(orgUsers); + var claimedStatuses = await getOrganizationUsersClaimedStatusQuery.GetUsersOrganizationClaimedStatusAsync(organizationId, orgUserIds); + + var internalRequests = CreateInternalRequests(organizationId, deletingUserId, orgUserIds, orgUsers, users, claimedStatuses); + var validationResults = (await deleteClaimedOrganizationUserAccountValidatorvNext.ValidateAsync(internalRequests)).ToList(); + + var validRequests = validationResults.ValidRequests(); + await CancelPremiumsAsync(validRequests); + await HandleUserDeletionsAsync(validRequests); + await LogDeletedOrganizationUsersAsync(validRequests); + + return validationResults.Select(v => v.Match( + error => new BulkCommandResult(v.Request.OrganizationUserId, error), + _ => new BulkCommandResult(v.Request.OrganizationUserId, new None()) + )); + } + + private static IEnumerable CreateInternalRequests( + Guid organizationId, + Guid deletingUserId, + IEnumerable orgUserIds, + ICollection orgUsers, + IEnumerable users, + IDictionary claimedStatuses) + { + foreach (var orgUserId in orgUserIds) + { + var orgUser = orgUsers.FirstOrDefault(orgUser => orgUser.Id == orgUserId); + var user = users.FirstOrDefault(user => user.Id == orgUser?.UserId); + claimedStatuses.TryGetValue(orgUserId, out var isClaimed); + + yield return new DeleteUserValidationRequest + { + User = user, + OrganizationUserId = orgUserId, + OrganizationUser = orgUser, + IsClaimed = isClaimed, + OrganizationId = organizationId, + DeletingUserId = deletingUserId, + }; + } + } + + private async Task> GetUsersAsync(ICollection orgUsers) + { + var userIds = orgUsers + .Where(orgUser => orgUser.UserId.HasValue) + .Select(orgUser => orgUser.UserId!.Value) + .ToList(); + + return await userRepository.GetManyAsync(userIds); + } + + private async Task LogDeletedOrganizationUsersAsync(IEnumerable requests) + { + var eventDate = DateTime.UtcNow; + + var events = requests + .Select(request => (request.OrganizationUser!, EventType.OrganizationUser_Deleted, (DateTime?)eventDate)) + .ToList(); + + if (events.Count != 0) + { + await eventService.LogOrganizationUserEventsAsync(events); + } + } + + private async Task HandleUserDeletionsAsync(IEnumerable requests) + { + var users = requests + .Select(request => request.User!) + .ToList(); + + if (users.Count == 0) + { + return; + } + + await userRepository.DeleteManyAsync(users); + + foreach (var user in users) + { + await pushService.PushLogOutAsync(user.Id); + } + } + + private async Task CancelPremiumsAsync(IEnumerable requests) + { + var users = requests.Select(request => request.User!); + + foreach (var user in users) + { + try + { + await userService.CancelPremiumAsync(user); + } + catch (GatewayException exception) + { + logger.LogWarning(exception, "Failed to cancel premium subscription for {userId}.", user.Id); + } + } + } +} + diff --git a/src/Core/AdminConsole/OrganizationFeatures/OrganizationUsers/DeleteClaimedAccountvNext/DeleteClaimedOrganizationUserAccountValidator.cs b/src/Core/AdminConsole/OrganizationFeatures/OrganizationUsers/DeleteClaimedAccountvNext/DeleteClaimedOrganizationUserAccountValidator.cs new file mode 100644 index 0000000000..7a88841d2f --- /dev/null +++ b/src/Core/AdminConsole/OrganizationFeatures/OrganizationUsers/DeleteClaimedAccountvNext/DeleteClaimedOrganizationUserAccountValidator.cs @@ -0,0 +1,76 @@ +using Bit.Core.AdminConsole.Repositories; +using Bit.Core.Context; +using Bit.Core.Enums; +using Bit.Core.Repositories; +using static Bit.Core.AdminConsole.OrganizationFeatures.OrganizationUsers.DeleteClaimedAccountvNext.ValidationResultHelpers; + +namespace Bit.Core.AdminConsole.OrganizationFeatures.OrganizationUsers.DeleteClaimedAccountvNext; + +public class DeleteClaimedOrganizationUserAccountValidatorvNext( + ICurrentContext currentContext, + IOrganizationUserRepository organizationUserRepository, + IProviderUserRepository providerUserRepository) : IDeleteClaimedOrganizationUserAccountValidatorvNext +{ + public async Task>> ValidateAsync(IEnumerable requests) + { + var tasks = requests.Select(ValidateAsync); + var results = await Task.WhenAll(tasks); + return results; + } + + private async Task> ValidateAsync(DeleteUserValidationRequest request) + { + // Ensure user exists + if (request.User == null || request.OrganizationUser == null) + { + return Invalid(request, new UserNotFoundError()); + } + + // Cannot delete invited users + if (request.OrganizationUser.Status == OrganizationUserStatusType.Invited) + { + return Invalid(request, new InvalidUserStatusError()); + } + + // Cannot delete yourself + if (request.OrganizationUser.UserId == request.DeletingUserId) + { + return Invalid(request, new CannotDeleteYourselfError()); + } + + // Can only delete a claimed user + if (!request.IsClaimed) + { + return Invalid(request, new UserNotClaimedError()); + } + + // Cannot delete an owner unless you are an owner or provider + if (request.OrganizationUser.Type == OrganizationUserType.Owner && + !await currentContext.OrganizationOwner(request.OrganizationId)) + { + return Invalid(request, new CannotDeleteOwnersError()); + } + + // Cannot delete a user who is the sole owner of an organization + var onlyOwnerCount = await organizationUserRepository.GetCountByOnlyOwnerAsync(request.User.Id); + if (onlyOwnerCount > 0) + { + return Invalid(request, new SoleOwnerError()); + } + + // Cannot delete a user who is the sole member of a provider + var onlyOwnerProviderCount = await providerUserRepository.GetCountByOnlyOwnerAsync(request.User.Id); + if (onlyOwnerProviderCount > 0) + { + return Invalid(request, new SoleProviderError()); + } + + // Custom users cannot delete admins + if (request.OrganizationUser.Type == OrganizationUserType.Admin && await currentContext.OrganizationCustom(request.OrganizationId)) + { + return Invalid(request, new CannotDeleteAdminsError()); + } + + return Valid(request); + } +} diff --git a/src/Core/AdminConsole/OrganizationFeatures/OrganizationUsers/DeleteClaimedAccountvNext/DeleteUserValidationRequest.cs b/src/Core/AdminConsole/OrganizationFeatures/OrganizationUsers/DeleteClaimedAccountvNext/DeleteUserValidationRequest.cs new file mode 100644 index 0000000000..5fd95dc73c --- /dev/null +++ b/src/Core/AdminConsole/OrganizationFeatures/OrganizationUsers/DeleteClaimedAccountvNext/DeleteUserValidationRequest.cs @@ -0,0 +1,13 @@ +using Bit.Core.Entities; + +namespace Bit.Core.AdminConsole.OrganizationFeatures.OrganizationUsers.DeleteClaimedAccountvNext; + +public class DeleteUserValidationRequest +{ + public Guid OrganizationId { get; init; } + public Guid OrganizationUserId { get; init; } + public OrganizationUser? OrganizationUser { get; init; } + public User? User { get; init; } + public Guid DeletingUserId { get; init; } + public bool IsClaimed { get; init; } +} diff --git a/src/Core/AdminConsole/OrganizationFeatures/OrganizationUsers/DeleteClaimedAccountvNext/Errors.cs b/src/Core/AdminConsole/OrganizationFeatures/OrganizationUsers/DeleteClaimedAccountvNext/Errors.cs new file mode 100644 index 0000000000..d991a882b8 --- /dev/null +++ b/src/Core/AdminConsole/OrganizationFeatures/OrganizationUsers/DeleteClaimedAccountvNext/Errors.cs @@ -0,0 +1,21 @@ +namespace Bit.Core.AdminConsole.OrganizationFeatures.OrganizationUsers.DeleteClaimedAccountvNext; + +/// +/// A strongly typed error containing a reason that an action failed. +/// This is used for business logic validation and other expected errors, not exceptions. +/// +public abstract record Error(string Message); +/// +/// An type that maps to a NotFoundResult at the api layer. +/// +/// +public abstract record NotFoundError(string Message) : Error(Message); + +public record UserNotFoundError() : NotFoundError("Invalid user."); +public record UserNotClaimedError() : Error("Member is not claimed by the organization."); +public record InvalidUserStatusError() : Error("You cannot delete a member with Invited status."); +public record CannotDeleteYourselfError() : Error("You cannot delete yourself."); +public record CannotDeleteOwnersError() : Error("Only owners can delete other owners."); +public record SoleOwnerError() : Error("Cannot delete this user because it is the sole owner of at least one organization. Please delete these organizations or upgrade another user."); +public record SoleProviderError() : Error("Cannot delete this user because it is the sole owner of at least one provider. Please delete these providers or upgrade another user."); +public record CannotDeleteAdminsError() : Error("Custom users can not delete admins."); diff --git a/src/Core/AdminConsole/OrganizationFeatures/OrganizationUsers/DeleteClaimedAccountvNext/IDeleteClaimedOrganizationUserAccountCommandvNext.cs b/src/Core/AdminConsole/OrganizationFeatures/OrganizationUsers/DeleteClaimedAccountvNext/IDeleteClaimedOrganizationUserAccountCommandvNext.cs new file mode 100644 index 0000000000..2c462a2acf --- /dev/null +++ b/src/Core/AdminConsole/OrganizationFeatures/OrganizationUsers/DeleteClaimedAccountvNext/IDeleteClaimedOrganizationUserAccountCommandvNext.cs @@ -0,0 +1,17 @@ +namespace Bit.Core.AdminConsole.OrganizationFeatures.OrganizationUsers.DeleteClaimedAccountvNext; + +public interface IDeleteClaimedOrganizationUserAccountCommandvNext +{ + /// + /// Removes a user from an organization and deletes all of their associated user data. + /// + Task DeleteUserAsync(Guid organizationId, Guid organizationUserId, Guid deletingUserId); + + /// + /// Removes multiple users from an organization and deletes all of their associated user data. + /// + /// + /// An error message for each user that could not be removed, otherwise null. + /// + Task> DeleteManyUsersAsync(Guid organizationId, IEnumerable orgUserIds, Guid deletingUserId); +} diff --git a/src/Core/AdminConsole/OrganizationFeatures/OrganizationUsers/DeleteClaimedAccountvNext/IDeleteClaimedOrganizationUserAccountValidatorvNext.cs b/src/Core/AdminConsole/OrganizationFeatures/OrganizationUsers/DeleteClaimedAccountvNext/IDeleteClaimedOrganizationUserAccountValidatorvNext.cs new file mode 100644 index 0000000000..f6125a0355 --- /dev/null +++ b/src/Core/AdminConsole/OrganizationFeatures/OrganizationUsers/DeleteClaimedAccountvNext/IDeleteClaimedOrganizationUserAccountValidatorvNext.cs @@ -0,0 +1,6 @@ +namespace Bit.Core.AdminConsole.OrganizationFeatures.OrganizationUsers.DeleteClaimedAccountvNext; + +public interface IDeleteClaimedOrganizationUserAccountValidatorvNext +{ + Task>> ValidateAsync(IEnumerable requests); +} diff --git a/src/Core/AdminConsole/OrganizationFeatures/OrganizationUsers/DeleteClaimedAccountvNext/ValidationResult.cs b/src/Core/AdminConsole/OrganizationFeatures/OrganizationUsers/DeleteClaimedAccountvNext/ValidationResult.cs new file mode 100644 index 0000000000..23d2fbb7ce --- /dev/null +++ b/src/Core/AdminConsole/OrganizationFeatures/OrganizationUsers/DeleteClaimedAccountvNext/ValidationResult.cs @@ -0,0 +1,41 @@ +using OneOf; +using OneOf.Types; + +namespace Bit.Core.AdminConsole.OrganizationFeatures.OrganizationUsers.DeleteClaimedAccountvNext; + +/// +/// Represents the result of validating a request. +/// This is for use within the Core layer, e.g. validating a command request. +/// +/// The request that has been validated. +/// A type that contains an Error if validation failed. +/// The request type. +public class ValidationResult(TRequest request, OneOf error) : OneOfBase(error) +{ + public TRequest Request { get; } = request; + + public bool IsError => IsT0; + public bool IsValid => IsT1; + public Error AsError => AsT0; +} + +public static class ValidationResultHelpers +{ + /// + /// Creates a successful with no error set. + /// + public static ValidationResult Valid(T request) => new(request, new None()); + /// + /// Creates a failed with the specified error. + /// + public static ValidationResult Invalid(T request, Error error) => new(request, error); + + /// + /// Extracts successfully validated requests from a sequence of . + /// + public static List ValidRequests(this IEnumerable> results) => + results + .Where(r => r.IsValid) + .Select(r => r.Request) + .ToList(); +} diff --git a/src/Core/Constants.cs b/src/Core/Constants.cs index cba060427c..3a825bc533 100644 --- a/src/Core/Constants.cs +++ b/src/Core/Constants.cs @@ -134,6 +134,7 @@ public static class FeatureFlagKeys public const string DirectoryConnectorPreventUserRemoval = "pm-24592-directory-connector-prevent-user-removal"; public const string CipherRepositoryBulkResourceCreation = "pm-24951-cipher-repository-bulk-resource-creation-service"; public const string CollectionVaultRefactor = "pm-25030-resolve-ts-upgrade-errors"; + public const string DeleteClaimedUserAccountRefactor = "pm-25094-refactor-delete-managed-organization-user-command"; /* Auth Team */ public const string TwoFactorExtensionDataPersistence = "pm-9115-two-factor-extension-data-persistence"; diff --git a/src/Core/OrganizationFeatures/OrganizationServiceCollectionExtensions.cs b/src/Core/OrganizationFeatures/OrganizationServiceCollectionExtensions.cs index bcbaccca7c..1c38a27d1e 100644 --- a/src/Core/OrganizationFeatures/OrganizationServiceCollectionExtensions.cs +++ b/src/Core/OrganizationFeatures/OrganizationServiceCollectionExtensions.cs @@ -13,6 +13,7 @@ using Bit.Core.AdminConsole.OrganizationFeatures.Organizations; using Bit.Core.AdminConsole.OrganizationFeatures.Organizations.Interfaces; using Bit.Core.AdminConsole.OrganizationFeatures.OrganizationUsers; using Bit.Core.AdminConsole.OrganizationFeatures.OrganizationUsers.Authorization; +using Bit.Core.AdminConsole.OrganizationFeatures.OrganizationUsers.DeleteClaimedAccountvNext; using Bit.Core.AdminConsole.OrganizationFeatures.OrganizationUsers.Interfaces; using Bit.Core.AdminConsole.OrganizationFeatures.OrganizationUsers.InviteUsers; using Bit.Core.AdminConsole.OrganizationFeatures.OrganizationUsers.InviteUsers.Validation; @@ -133,6 +134,10 @@ public static class OrganizationServiceCollectionExtensions services.AddScoped(); services.AddScoped(); services.AddScoped(); + + // vNext implementations (feature flagged) + services.AddScoped(); + services.AddScoped(); } private static void AddOrganizationApiKeyCommandsQueries(this IServiceCollection services) diff --git a/test/Api.IntegrationTest/AdminConsole/Controllers/OrganizationUserControllerTests.cs b/test/Api.IntegrationTest/AdminConsole/Controllers/OrganizationUserControllerTests.cs index 04ab72fad1..b7839467e8 100644 --- a/test/Api.IntegrationTest/AdminConsole/Controllers/OrganizationUserControllerTests.cs +++ b/test/Api.IntegrationTest/AdminConsole/Controllers/OrganizationUserControllerTests.cs @@ -1,10 +1,13 @@ using System.Net; using Bit.Api.AdminConsole.Models.Request.Organizations; +using Bit.Api.AdminConsole.Models.Response.Organizations; using Bit.Api.IntegrationTest.Factories; using Bit.Api.IntegrationTest.Helpers; using Bit.Api.Models.Request; +using Bit.Api.Models.Response; using Bit.Core; using Bit.Core.AdminConsole.Entities; +using Bit.Core.AdminConsole.OrganizationFeatures.OrganizationUsers.DeleteClaimedAccountvNext; using Bit.Core.AdminConsole.Repositories; using Bit.Core.Billing.Enums; using Bit.Core.Entities; @@ -30,6 +33,10 @@ public class OrganizationUserControllerTests : IClassFixture(); + var organizationUserRepository = _factory.GetService(); + + Assert.NotNull(orgUserToDelete.UserId); + Assert.NotNull(await userRepository.GetByIdAsync(orgUserToDelete.UserId.Value)); + Assert.NotNull(await organizationUserRepository.GetByIdAsync(orgUserToDelete.Id)); + + var request = new OrganizationUserBulkRequestModel + { + Ids = [orgUserToDelete.Id] + }; + + var httpResponse = await _client.PostAsJsonAsync($"organizations/{_organization.Id}/users/delete-account", request); + var content = await httpResponse.Content.ReadFromJsonAsync>(); + Assert.Single(content.Data, r => r.Id == orgUserToDelete.Id && r.Error == string.Empty); + + Assert.Equal(HttpStatusCode.OK, httpResponse.StatusCode); + Assert.Null(await userRepository.GetByIdAsync(orgUserToDelete.UserId.Value)); + Assert.Null(await organizationUserRepository.GetByIdAsync(orgUserToDelete.Id)); + } + + [Fact] + public async Task BulkDeleteAccount_MixedResults() + { + var (userEmail, _) = await OrganizationTestHelpers.CreateNewUserWithAccountAsync(_factory, + _organization.Id, OrganizationUserType.Admin); + + await _loginHelper.LoginAsync(userEmail); + + // Can delete users + var (_, validOrgUser) = await OrganizationTestHelpers.CreateNewUserWithAccountAsync(_factory, _organization.Id, OrganizationUserType.User); + // Cannot delete owners + var (_, invalidOrgUser) = await OrganizationTestHelpers.CreateNewUserWithAccountAsync(_factory, _organization.Id, OrganizationUserType.Owner); + await OrganizationTestHelpers.CreateVerifiedDomainAsync(_factory, _organization.Id, "bitwarden.com"); + + var userRepository = _factory.GetService(); + var organizationUserRepository = _factory.GetService(); + + Assert.NotNull(validOrgUser.UserId); + Assert.NotNull(invalidOrgUser.UserId); + + var arrangedUsers = + await userRepository.GetManyAsync([validOrgUser.UserId.Value, invalidOrgUser.UserId.Value]); + Assert.Equal(2, arrangedUsers.Count()); + + var arrangedOrgUsers = + await organizationUserRepository.GetManyAsync([validOrgUser.Id, invalidOrgUser.Id]); + Assert.Equal(2, arrangedOrgUsers.Count); + + var request = new OrganizationUserBulkRequestModel + { + Ids = [validOrgUser.Id, invalidOrgUser.Id] + }; + + var httpResponse = await _client.PostAsJsonAsync($"organizations/{_organization.Id}/users/delete-account", request); + + Assert.Equal(HttpStatusCode.OK, httpResponse.StatusCode); + var debug = await httpResponse.Content.ReadAsStringAsync(); + var content = await httpResponse.Content.ReadFromJsonAsync>(); + Assert.Equal(2, content.Data.Count()); + Assert.Contains(content.Data, r => r.Id == validOrgUser.Id && r.Error == string.Empty); + Assert.Contains(content.Data, r => + r.Id == invalidOrgUser.Id && + string.Equals(r.Error, new CannotDeleteOwnersError().Message, StringComparison.Ordinal)); + + var actualUsers = + await userRepository.GetManyAsync([validOrgUser.UserId.Value, invalidOrgUser.UserId.Value]); + Assert.Single(actualUsers, u => u.Id == invalidOrgUser.UserId.Value); + + var actualOrgUsers = + await organizationUserRepository.GetManyAsync([validOrgUser.Id, invalidOrgUser.Id]); + Assert.Single(actualOrgUsers, ou => ou.Id == invalidOrgUser.Id); + } + [Theory] [InlineData(OrganizationUserType.User)] [InlineData(OrganizationUserType.Custom)] @@ -57,11 +149,36 @@ public class OrganizationUserControllerTests : IClassFixture { Guid.NewGuid() } }; - var httpResponse = await _client.PostAsJsonAsync($"organizations/{_organization.Id}/users/remove", request); + var httpResponse = await _client.PostAsJsonAsync($"organizations/{_organization.Id}/users/delete-account", request); Assert.Equal(HttpStatusCode.Forbidden, httpResponse.StatusCode); } + [Fact] + public async Task DeleteAccount_Success() + { + var (userEmail, _) = await OrganizationTestHelpers.CreateNewUserWithAccountAsync(_factory, + _organization.Id, OrganizationUserType.Owner); + + await _loginHelper.LoginAsync(userEmail); + + var (_, orgUserToDelete) = await OrganizationTestHelpers.CreateNewUserWithAccountAsync(_factory, _organization.Id, OrganizationUserType.User); + await OrganizationTestHelpers.CreateVerifiedDomainAsync(_factory, _organization.Id, "bitwarden.com"); + + var userRepository = _factory.GetService(); + var organizationUserRepository = _factory.GetService(); + + Assert.NotNull(orgUserToDelete.UserId); + Assert.NotNull(await userRepository.GetByIdAsync(orgUserToDelete.UserId.Value)); + Assert.NotNull(await organizationUserRepository.GetByIdAsync(orgUserToDelete.Id)); + + var httpResponse = await _client.DeleteAsync($"organizations/{_organization.Id}/users/{orgUserToDelete.Id}/delete-account"); + + Assert.Equal(HttpStatusCode.OK, httpResponse.StatusCode); + Assert.Null(await userRepository.GetByIdAsync(orgUserToDelete.UserId.Value)); + Assert.Null(await organizationUserRepository.GetByIdAsync(orgUserToDelete.Id)); + } + [Theory] [InlineData(OrganizationUserType.User)] [InlineData(OrganizationUserType.Custom)] @@ -74,7 +191,7 @@ public class OrganizationUserControllerTests : IClassFixture deleteResults, SutProvider sutProvider) - { - sutProvider.GetDependency().ManageUsers(orgId).Returns(true); - sutProvider.GetDependency().GetUserByPrincipalAsync(default).ReturnsForAnyArgs(currentUser); - sutProvider.GetDependency() - .DeleteManyUsersAsync(orgId, model.Ids, currentUser.Id) - .Returns(deleteResults); - - var response = await sutProvider.Sut.BulkDeleteAccount(orgId, model); - - Assert.Equal(deleteResults.Count, response.Data.Count()); - Assert.True(response.Data.All(r => deleteResults.Any(res => res.Item1 == r.Id && res.Item2 == r.Error))); - await sutProvider.GetDependency() - .Received(1) - .DeleteManyUsersAsync(orgId, model.Ids, currentUser.Id); - } - [Theory] [BitAutoData] public async Task BulkDeleteAccount_WhenCurrentUserNotFound_ThrowsUnauthorizedAccessException( diff --git a/test/Core.Test/AdminConsole/OrganizationFeatures/OrganizationUsers/DeleteClaimedAccountvNext/DeleteClaimedOrganizationUserAccountCommandvNextTests.cs b/test/Core.Test/AdminConsole/OrganizationFeatures/OrganizationUsers/DeleteClaimedAccountvNext/DeleteClaimedOrganizationUserAccountCommandvNextTests.cs new file mode 100644 index 0000000000..679c1914c6 --- /dev/null +++ b/test/Core.Test/AdminConsole/OrganizationFeatures/OrganizationUsers/DeleteClaimedAccountvNext/DeleteClaimedOrganizationUserAccountCommandvNextTests.cs @@ -0,0 +1,467 @@ +using Bit.Core.AdminConsole.OrganizationFeatures.OrganizationUsers.DeleteClaimedAccountvNext; +using Bit.Core.AdminConsole.OrganizationFeatures.OrganizationUsers.Interfaces; +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; +using Bit.Core.Test.AutoFixture.OrganizationUserFixtures; +using Bit.Test.Common.AutoFixture; +using Bit.Test.Common.AutoFixture.Attributes; +using Microsoft.Extensions.Logging; +using NSubstitute; +using NSubstitute.ExceptionExtensions; +using Xunit; + +namespace Bit.Core.Test.AdminConsole.OrganizationFeatures.OrganizationUsers.DeleteClaimedAccountvNext; + +[SutProviderCustomize] +public class DeleteClaimedOrganizationUserAccountCommandvNextTests +{ + [Theory] + [BitAutoData] + public async Task DeleteUserAsync_WithValidSingleUser_CallsDeleteManyUsersAsync( + SutProvider sutProvider, + User user, + Guid organizationId, + Guid deletingUserId, + [OrganizationUser] OrganizationUser organizationUser) + { + organizationUser.UserId = user.Id; + organizationUser.OrganizationId = organizationId; + + var request = new DeleteUserValidationRequest + { + OrganizationId = organizationId, + OrganizationUserId = organizationUser.Id, + OrganizationUser = organizationUser, + User = user, + DeletingUserId = deletingUserId, + IsClaimed = true + }; + var validationResult = CreateSuccessfulValidationResult(request); + + SetupRepositoryMocks(sutProvider, + new List { organizationUser }, + [user], + organizationId, + new Dictionary { { organizationUser.Id, true } }); + + SetupValidatorMock(sutProvider, [validationResult]); + + var result = await sutProvider.Sut.DeleteUserAsync(organizationId, organizationUser.Id, deletingUserId); + + Assert.Equal(organizationUser.Id, result.Id); + Assert.True(result.Result.IsSuccess); + + await sutProvider.GetDependency() + .Received(1) + .GetManyAsync(Arg.Is>(ids => ids.Contains(organizationUser.Id))); + + await AssertSuccessfulUserOperations(sutProvider, [user], [organizationUser]); + } + + [Theory] + [BitAutoData] + public async Task DeleteManyUsersAsync_WithEmptyUserIds_ReturnsEmptyResults( + SutProvider sutProvider, + Guid organizationId, + Guid deletingUserId) + { + var results = await sutProvider.Sut.DeleteManyUsersAsync(organizationId, [], deletingUserId); + + Assert.Empty(results); + } + + [Theory] + [BitAutoData] + public async Task DeleteManyUsersAsync_WithValidUsers_DeletesUsersAndLogsEvents( + SutProvider sutProvider, + User user1, + User user2, + Guid organizationId, + Guid deletingUserId, + [OrganizationUser] OrganizationUser orgUser1, + [OrganizationUser] OrganizationUser orgUser2) + { + // Arrange + orgUser1.OrganizationId = orgUser2.OrganizationId = organizationId; + orgUser1.UserId = user1.Id; + orgUser2.UserId = user2.Id; + + var request1 = new DeleteUserValidationRequest + { + OrganizationId = organizationId, + OrganizationUserId = orgUser1.Id, + OrganizationUser = orgUser1, + User = user1, + DeletingUserId = deletingUserId, + IsClaimed = true + }; + var request2 = new DeleteUserValidationRequest + { + OrganizationId = organizationId, + OrganizationUserId = orgUser2.Id, + OrganizationUser = orgUser2, + User = user2, + DeletingUserId = deletingUserId, + IsClaimed = true + }; + + var validationResults = new[] + { + CreateSuccessfulValidationResult(request1), + CreateSuccessfulValidationResult(request2) + }; + + SetupRepositoryMocks(sutProvider, + new List { orgUser1, orgUser2 }, + [user1, user2], + organizationId, + new Dictionary { { orgUser1.Id, true }, { orgUser2.Id, true } }); + + SetupValidatorMock(sutProvider, validationResults); + + var results = await sutProvider.Sut.DeleteManyUsersAsync(organizationId, [orgUser1.Id, orgUser2.Id], deletingUserId); + + var resultsList = results.ToList(); + Assert.Equal(2, resultsList.Count); + Assert.All(resultsList, result => Assert.True(result.Result.IsSuccess)); + + await AssertSuccessfulUserOperations(sutProvider, [user1, user2], [orgUser1, orgUser2]); + } + + [Theory] + [BitAutoData] + public async Task DeleteManyUsersAsync_WithValidationErrors_ReturnsErrorResults( + SutProvider sutProvider, + Guid organizationId, + Guid orgUserId1, + Guid orgUserId2, + Guid deletingUserId) + { + // Arrange + var request1 = new DeleteUserValidationRequest + { + OrganizationId = organizationId, + OrganizationUserId = orgUserId1, + DeletingUserId = deletingUserId + }; + var request2 = new DeleteUserValidationRequest + { + OrganizationId = organizationId, + OrganizationUserId = orgUserId2, + DeletingUserId = deletingUserId + }; + + var validationResults = new[] + { + CreateFailedValidationResult(request1, new UserNotClaimedError()), + CreateFailedValidationResult(request2, new InvalidUserStatusError()) + }; + + SetupRepositoryMocks(sutProvider, [], [], organizationId, new Dictionary()); + SetupValidatorMock(sutProvider, validationResults); + + var results = await sutProvider.Sut.DeleteManyUsersAsync(organizationId, [orgUserId1, orgUserId2], deletingUserId); + + var resultsList = results.ToList(); + Assert.Equal(2, resultsList.Count); + + Assert.Equal(orgUserId1, resultsList[0].Id); + Assert.True(resultsList[0].Result.IsError); + Assert.IsType(resultsList[0].Result.AsError); + + Assert.Equal(orgUserId2, resultsList[1].Id); + Assert.True(resultsList[1].Result.IsError); + Assert.IsType(resultsList[1].Result.AsError); + + await AssertNoUserOperations(sutProvider); + } + + [Theory] + [BitAutoData] + public async Task DeleteManyUsersAsync_WithMixedValidationResults_HandlesPartialSuccessCorrectly( + SutProvider sutProvider, + User validUser, + Guid organizationId, + Guid validOrgUserId, + Guid invalidOrgUserId, + Guid deletingUserId, + [OrganizationUser] OrganizationUser validOrgUser) + { + validOrgUser.Id = validOrgUserId; + validOrgUser.UserId = validUser.Id; + validOrgUser.OrganizationId = organizationId; + + var validRequest = new DeleteUserValidationRequest + { + OrganizationId = organizationId, + OrganizationUserId = validOrgUserId, + OrganizationUser = validOrgUser, + User = validUser, + DeletingUserId = deletingUserId, + IsClaimed = true + }; + var invalidRequest = new DeleteUserValidationRequest + { + OrganizationId = organizationId, + OrganizationUserId = invalidOrgUserId, + DeletingUserId = deletingUserId + }; + + var validationResults = new[] + { + CreateSuccessfulValidationResult(validRequest), + CreateFailedValidationResult(invalidRequest, new UserNotFoundError()) + }; + + SetupRepositoryMocks(sutProvider, + new List { validOrgUser }, + [validUser], + organizationId, + new Dictionary { { validOrgUserId, true } }); + + SetupValidatorMock(sutProvider, validationResults); + + var results = await sutProvider.Sut.DeleteManyUsersAsync(organizationId, [validOrgUserId, invalidOrgUserId], deletingUserId); + + var resultsList = results.ToList(); + Assert.Equal(2, resultsList.Count); + + var validResult = resultsList.First(r => r.Id == validOrgUserId); + var invalidResult = resultsList.First(r => r.Id == invalidOrgUserId); + + Assert.True(validResult.Result.IsSuccess); + Assert.True(invalidResult.Result.IsError); + Assert.IsType(invalidResult.Result.AsError); + + await AssertSuccessfulUserOperations(sutProvider, [validUser], [validOrgUser]); + } + + [Theory] + [BitAutoData] + public async Task DeleteManyUsersAsync_CancelPremiumsAsync_HandlesGatewayExceptionAndLogsWarning( + SutProvider sutProvider, + User user, + Guid organizationId, + Guid deletingUserId, + [OrganizationUser] OrganizationUser orgUser) + { + orgUser.UserId = user.Id; + orgUser.OrganizationId = organizationId; + + var request = new DeleteUserValidationRequest + { + OrganizationId = organizationId, + OrganizationUserId = orgUser.Id, + OrganizationUser = orgUser, + User = user, + DeletingUserId = deletingUserId, + IsClaimed = true + }; + var validationResult = CreateSuccessfulValidationResult(request); + + SetupRepositoryMocks(sutProvider, + new List { orgUser }, + [user], + organizationId, + new Dictionary { { orgUser.Id, true } }); + + SetupValidatorMock(sutProvider, [validationResult]); + + var gatewayException = new GatewayException("Payment gateway error"); + sutProvider.GetDependency() + .CancelPremiumAsync(user) + .ThrowsAsync(gatewayException); + + var results = await sutProvider.Sut.DeleteManyUsersAsync(organizationId, [orgUser.Id], deletingUserId); + + var resultsList = results.ToList(); + Assert.Single(resultsList); + Assert.True(resultsList.First().Result.IsSuccess); + + await sutProvider.GetDependency().Received(1).CancelPremiumAsync(user); + await AssertSuccessfulUserOperations(sutProvider, [user], [orgUser]); + + sutProvider.GetDependency>() + .Received(1) + .Log( + LogLevel.Warning, + Arg.Any(), + Arg.Is(o => o.ToString()!.Contains($"Failed to cancel premium subscription for {user.Id}")), + gatewayException, + Arg.Any>()); + } + + + [Theory] + [BitAutoData] + public async Task CreateInternalRequests_CreatesCorrectRequestsForAllUsers( + SutProvider sutProvider, + User user1, + User user2, + Guid organizationId, + Guid deletingUserId, + [OrganizationUser] OrganizationUser orgUser1, + [OrganizationUser] OrganizationUser orgUser2) + { + orgUser1.UserId = user1.Id; + orgUser2.UserId = user2.Id; + var orgUserIds = new[] { orgUser1.Id, orgUser2.Id }; + var orgUsers = new List { orgUser1, orgUser2 }; + var users = new[] { user1, user2 }; + var claimedStatuses = new Dictionary { { orgUser1.Id, true }, { orgUser2.Id, false } }; + + sutProvider.GetDependency() + .GetManyAsync(Arg.Any>()) + .Returns(orgUsers); + + sutProvider.GetDependency() + .GetManyAsync(Arg.Is>(ids => ids.Contains(user1.Id) && ids.Contains(user2.Id))) + .Returns(users); + + sutProvider.GetDependency() + .GetUsersOrganizationClaimedStatusAsync(organizationId, Arg.Any>()) + .Returns(claimedStatuses); + + sutProvider.GetDependency() + .ValidateAsync(Arg.Any>()) + .Returns(callInfo => + { + var requests = callInfo.Arg>(); + return requests.Select(r => CreateFailedValidationResult(r, new UserNotFoundError())); + }); + + // Act + await sutProvider.Sut.DeleteManyUsersAsync(organizationId, orgUserIds, deletingUserId); + + // Assert + await sutProvider.GetDependency() + .Received(1) + .ValidateAsync(Arg.Is>(requests => + requests.Count() == 2 && + requests.Any(r => r.OrganizationUserId == orgUser1.Id && + r.OrganizationId == organizationId && + r.OrganizationUser == orgUser1 && + r.User == user1 && + r.DeletingUserId == deletingUserId && + r.IsClaimed == true) && + requests.Any(r => r.OrganizationUserId == orgUser2.Id && + r.OrganizationId == organizationId && + r.OrganizationUser == orgUser2 && + r.User == user2 && + r.DeletingUserId == deletingUserId && + r.IsClaimed == false))); + } + + [Theory] + [BitAutoData] + public async Task GetUsersAsync_WithNullUserIds_ReturnsEmptyCollection( + SutProvider sutProvider, + Guid organizationId, + Guid deletingUserId, + [OrganizationUser] OrganizationUser orgUserWithoutUserId) + { + orgUserWithoutUserId.UserId = null; // Intentionally setting to null for test case + + sutProvider.GetDependency() + .GetManyAsync(Arg.Any>()) + .Returns(new List { orgUserWithoutUserId }); + + sutProvider.GetDependency() + .GetManyAsync(Arg.Is>(ids => !ids.Any())) + .Returns([]); + + sutProvider.GetDependency() + .ValidateAsync(Arg.Any>()) + .Returns(callInfo => + { + var requests = callInfo.Arg>(); + return requests.Select(r => CreateFailedValidationResult(r, new UserNotFoundError())); + }); + + // Act + await sutProvider.Sut.DeleteManyUsersAsync(organizationId, [orgUserWithoutUserId.Id], deletingUserId); + + // Assert + await sutProvider.GetDependency() + .Received(1) + .ValidateAsync(Arg.Is>(requests => + requests.Count() == 1 && + requests.Single().User == null)); + + await sutProvider.GetDependency().Received(1) + .GetManyAsync(Arg.Is>(ids => !ids.Any())); + } + + private static ValidationResult CreateSuccessfulValidationResult( + DeleteUserValidationRequest request) => + ValidationResultHelpers.Valid(request); + + private static ValidationResult CreateFailedValidationResult( + DeleteUserValidationRequest request, + Error error) => + ValidationResultHelpers.Invalid(request, error); + + private static void SetupRepositoryMocks( + SutProvider sutProvider, + ICollection orgUsers, + IEnumerable users, + Guid organizationId, + Dictionary claimedStatuses) + { + sutProvider.GetDependency() + .GetManyAsync(Arg.Any>()) + .Returns(orgUsers); + + sutProvider.GetDependency() + .GetManyAsync(Arg.Any>()) + .Returns(users); + + sutProvider.GetDependency() + .GetUsersOrganizationClaimedStatusAsync(organizationId, Arg.Any>()) + .Returns(claimedStatuses); + } + + private static void SetupValidatorMock( + SutProvider sutProvider, + IEnumerable> validationResults) + { + sutProvider.GetDependency() + .ValidateAsync(Arg.Any>()) + .Returns(validationResults); + } + + private static async Task AssertSuccessfulUserOperations( + SutProvider sutProvider, + IEnumerable expectedUsers, + IEnumerable expectedOrgUsers) + { + var userList = expectedUsers.ToList(); + var orgUserList = expectedOrgUsers.ToList(); + + await sutProvider.GetDependency().Received(1) + .DeleteManyAsync(Arg.Is>(users => + userList.All(expectedUser => users.Any(u => u.Id == expectedUser.Id)))); + + foreach (var user in userList) + { + await sutProvider.GetDependency().Received(1).PushLogOutAsync(user.Id); + } + + await sutProvider.GetDependency().Received(1) + .LogOrganizationUserEventsAsync(Arg.Is>(events => + orgUserList.All(expectedOrgUser => + events.Any(e => e.Item1.Id == expectedOrgUser.Id && e.Item2 == EventType.OrganizationUser_Deleted)))); + } + + private static async Task AssertNoUserOperations(SutProvider sutProvider) + { + await sutProvider.GetDependency().DidNotReceiveWithAnyArgs().DeleteManyAsync(default); + await sutProvider.GetDependency().DidNotReceiveWithAnyArgs().PushLogOutAsync(default); + await sutProvider.GetDependency().DidNotReceiveWithAnyArgs() + .LogOrganizationUserEventsAsync(default(IEnumerable<(OrganizationUser, EventType, DateTime?)>)); + } +} diff --git a/test/Core.Test/AdminConsole/OrganizationFeatures/OrganizationUsers/DeleteClaimedAccountvNext/DeleteClaimedOrganizationUserAccountValidatorvNextTests.cs b/test/Core.Test/AdminConsole/OrganizationFeatures/OrganizationUsers/DeleteClaimedAccountvNext/DeleteClaimedOrganizationUserAccountValidatorvNextTests.cs new file mode 100644 index 0000000000..e51df6a626 --- /dev/null +++ b/test/Core.Test/AdminConsole/OrganizationFeatures/OrganizationUsers/DeleteClaimedAccountvNext/DeleteClaimedOrganizationUserAccountValidatorvNextTests.cs @@ -0,0 +1,503 @@ +using Bit.Core.AdminConsole.OrganizationFeatures.OrganizationUsers.DeleteClaimedAccountvNext; +using Bit.Core.AdminConsole.Repositories; +using Bit.Core.Context; +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.DeleteClaimedAccountvNext; + +[SutProviderCustomize] +public class DeleteClaimedOrganizationUserAccountValidatorvNextTests +{ + [Theory] + [BitAutoData] + public async Task ValidateAsync_WithValidSingleRequest_ReturnsValidResult( + SutProvider sutProvider, + User user, + Guid organizationId, + Guid deletingUserId, + [OrganizationUser] OrganizationUser organizationUser) + { + organizationUser.UserId = user.Id; + organizationUser.OrganizationId = organizationId; + + var request = new DeleteUserValidationRequest + { + OrganizationId = organizationId, + OrganizationUserId = organizationUser.Id, + OrganizationUser = organizationUser, + User = user, + DeletingUserId = deletingUserId, + IsClaimed = true + }; + + SetupMocks(sutProvider, organizationId, user.Id); + + var results = await sutProvider.Sut.ValidateAsync([request]); + + var resultsList = results.ToList(); + Assert.Single(resultsList); + Assert.True(resultsList[0].IsValid); + Assert.Equal(request, resultsList[0].Request); + } + + [Theory] + [BitAutoData] + public async Task ValidateAsync_WithMultipleValidRequests_ReturnsAllValidResults( + SutProvider sutProvider, + User user1, + User user2, + Guid organizationId, + Guid deletingUserId, + [OrganizationUser] OrganizationUser orgUser1, + [OrganizationUser(OrganizationUserStatusType.Accepted)] OrganizationUser orgUser2) + { + orgUser1.UserId = user1.Id; + orgUser1.OrganizationId = organizationId; + + orgUser2.UserId = user2.Id; + orgUser2.OrganizationId = organizationId; + + var request1 = new DeleteUserValidationRequest + { + OrganizationId = organizationId, + OrganizationUserId = orgUser1.Id, + OrganizationUser = orgUser1, + User = user1, + DeletingUserId = deletingUserId, + IsClaimed = true + }; + + var request2 = new DeleteUserValidationRequest + { + OrganizationId = organizationId, + OrganizationUserId = orgUser2.Id, + OrganizationUser = orgUser2, + User = user2, + DeletingUserId = deletingUserId, + IsClaimed = true + }; + + SetupMocks(sutProvider, organizationId, user1.Id); + SetupMocks(sutProvider, organizationId, user2.Id); + + var results = await sutProvider.Sut.ValidateAsync([request1, request2]); + + var resultsList = results.ToList(); + Assert.Equal(2, resultsList.Count); + Assert.All(resultsList, result => Assert.True(result.IsValid)); + } + + [Theory] + [BitAutoData] + public async Task ValidateAsync_WithNullUser_ReturnsUserNotFoundError( + SutProvider sutProvider, + Guid organizationId, + Guid deletingUserId, + [OrganizationUser] OrganizationUser organizationUser) + { + var request = new DeleteUserValidationRequest + { + OrganizationId = organizationId, + OrganizationUserId = organizationUser.Id, + OrganizationUser = organizationUser, + User = null, + DeletingUserId = deletingUserId, + IsClaimed = true + }; + + var results = await sutProvider.Sut.ValidateAsync([request]); + + var resultsList = results.ToList(); + Assert.Single(resultsList); + Assert.True(resultsList[0].IsError); + Assert.IsType(resultsList[0].AsError); + } + + [Theory] + [BitAutoData] + public async Task ValidateAsync_WithNullOrganizationUser_ReturnsUserNotFoundError( + SutProvider sutProvider, + User user, + Guid organizationId, + Guid deletingUserId) + { + var request = new DeleteUserValidationRequest + { + OrganizationId = organizationId, + OrganizationUserId = Guid.NewGuid(), + OrganizationUser = null, + User = user, + DeletingUserId = deletingUserId, + IsClaimed = true + }; + + var results = await sutProvider.Sut.ValidateAsync([request]); + + var resultsList = results.ToList(); + Assert.Single(resultsList); + Assert.True(resultsList[0].IsError); + Assert.IsType(resultsList[0].AsError); + } + + [Theory] + [BitAutoData] + public async Task ValidateAsync_WithInvitedUser_ReturnsInvalidUserStatusError( + SutProvider sutProvider, + User user, + Guid organizationId, + Guid deletingUserId, + [OrganizationUser(OrganizationUserStatusType.Invited)] OrganizationUser organizationUser) + { + organizationUser.UserId = user.Id; + + var request = new DeleteUserValidationRequest + { + OrganizationId = organizationId, + OrganizationUserId = organizationUser.Id, + OrganizationUser = organizationUser, + User = user, + DeletingUserId = deletingUserId, + IsClaimed = true + }; + + var results = await sutProvider.Sut.ValidateAsync([request]); + + var resultsList = results.ToList(); + Assert.Single(resultsList); + Assert.True(resultsList[0].IsError); + Assert.IsType(resultsList[0].AsError); + } + + [Theory] + [BitAutoData] + public async Task ValidateAsync_WhenDeletingYourself_ReturnsCannotDeleteYourselfError( + SutProvider sutProvider, + User user, + Guid organizationId, + [OrganizationUser] OrganizationUser organizationUser) + { + organizationUser.UserId = user.Id; + + var request = new DeleteUserValidationRequest + { + OrganizationId = organizationId, + OrganizationUserId = organizationUser.Id, + OrganizationUser = organizationUser, + User = user, + DeletingUserId = user.Id, + IsClaimed = true + }; + + var results = await sutProvider.Sut.ValidateAsync([request]); + + var resultsList = results.ToList(); + Assert.Single(resultsList); + Assert.True(resultsList[0].IsError); + Assert.IsType(resultsList[0].AsError); + } + + [Theory] + [BitAutoData] + public async Task ValidateAsync_WithUnclaimedUser_ReturnsUserNotClaimedError( + SutProvider sutProvider, + User user, + Guid organizationId, + Guid deletingUserId, + [OrganizationUser] OrganizationUser organizationUser) + { + organizationUser.UserId = user.Id; + + var request = new DeleteUserValidationRequest + { + OrganizationId = organizationId, + OrganizationUserId = organizationUser.Id, + OrganizationUser = organizationUser, + User = user, + DeletingUserId = deletingUserId, + IsClaimed = false + }; + + var results = await sutProvider.Sut.ValidateAsync([request]); + + var resultsList = results.ToList(); + Assert.Single(resultsList); + Assert.True(resultsList[0].IsError); + Assert.IsType(resultsList[0].AsError); + } + + [Theory] + [BitAutoData] + public async Task ValidateAsync_DeletingOwnerWhenCurrentUserIsNotOwner_ReturnsCannotDeleteOwnersError( + SutProvider sutProvider, + User user, + Guid organizationId, + Guid deletingUserId, + [OrganizationUser(OrganizationUserStatusType.Confirmed, OrganizationUserType.Owner)] OrganizationUser organizationUser) + { + organizationUser.UserId = user.Id; + + var request = new DeleteUserValidationRequest + { + OrganizationId = organizationId, + OrganizationUserId = organizationUser.Id, + OrganizationUser = organizationUser, + User = user, + DeletingUserId = deletingUserId, + IsClaimed = true + }; + + SetupMocks(sutProvider, organizationId, user.Id, OrganizationUserType.Admin); + + var results = await sutProvider.Sut.ValidateAsync([request]); + + var resultsList = results.ToList(); + Assert.Single(resultsList); + Assert.True(resultsList[0].IsError); + Assert.IsType(resultsList[0].AsError); + } + + [Theory] + [BitAutoData] + public async Task ValidateAsync_DeletingOwnerWhenCurrentUserIsOwner_ReturnsValidResult( + SutProvider sutProvider, + User user, + Guid organizationId, + Guid deletingUserId, + [OrganizationUser(OrganizationUserStatusType.Confirmed, OrganizationUserType.Owner)] OrganizationUser organizationUser) + { + organizationUser.UserId = user.Id; + + var request = new DeleteUserValidationRequest + { + OrganizationId = organizationId, + OrganizationUserId = organizationUser.Id, + OrganizationUser = organizationUser, + User = user, + DeletingUserId = deletingUserId, + IsClaimed = true + }; + + SetupMocks(sutProvider, organizationId, user.Id); + + var results = await sutProvider.Sut.ValidateAsync([request]); + + var resultsList = results.ToList(); + Assert.Single(resultsList); + Assert.True(resultsList[0].IsValid); + } + + [Theory] + [BitAutoData] + public async Task ValidateAsync_WithSoleOwnerOfOrganization_ReturnsSoleOwnerError( + SutProvider sutProvider, + User user, + Guid organizationId, + Guid deletingUserId, + [OrganizationUser] OrganizationUser organizationUser) + { + organizationUser.UserId = user.Id; + + var request = new DeleteUserValidationRequest + { + OrganizationId = organizationId, + OrganizationUserId = organizationUser.Id, + OrganizationUser = organizationUser, + User = user, + DeletingUserId = deletingUserId, + IsClaimed = true + }; + + SetupMocks(sutProvider, organizationId, user.Id); + + sutProvider.GetDependency() + .GetCountByOnlyOwnerAsync(user.Id) + .Returns(1); + + var results = await sutProvider.Sut.ValidateAsync([request]); + + var resultsList = results.ToList(); + Assert.Single(resultsList); + Assert.True(resultsList[0].IsError); + Assert.IsType(resultsList[0].AsError); + } + + [Theory] + [BitAutoData] + public async Task ValidateAsync_WithSoleProviderOwner_ReturnsSoleProviderError( + SutProvider sutProvider, + User user, + Guid organizationId, + Guid deletingUserId, + [OrganizationUser] OrganizationUser organizationUser) + { + organizationUser.UserId = user.Id; + + var request = new DeleteUserValidationRequest + { + OrganizationId = organizationId, + OrganizationUserId = organizationUser.Id, + OrganizationUser = organizationUser, + User = user, + DeletingUserId = deletingUserId, + IsClaimed = true + }; + + SetupMocks(sutProvider, organizationId, user.Id); + + sutProvider.GetDependency() + .GetCountByOnlyOwnerAsync(user.Id) + .Returns(1); + + var results = await sutProvider.Sut.ValidateAsync([request]); + + var resultsList = results.ToList(); + Assert.Single(resultsList); + Assert.True(resultsList[0].IsError); + Assert.IsType(resultsList[0].AsError); + } + + [Theory] + [BitAutoData] + public async Task ValidateAsync_CustomUserDeletingAdmin_ReturnsCannotDeleteAdminsError( + SutProvider sutProvider, + User user, + Guid organizationId, + Guid deletingUserId, + [OrganizationUser(OrganizationUserStatusType.Confirmed, OrganizationUserType.Admin)] OrganizationUser organizationUser) + { + organizationUser.UserId = user.Id; + + var request = new DeleteUserValidationRequest + { + OrganizationId = organizationId, + OrganizationUserId = organizationUser.Id, + OrganizationUser = organizationUser, + User = user, + DeletingUserId = deletingUserId, + IsClaimed = true + }; + + SetupMocks(sutProvider, organizationId, user.Id, OrganizationUserType.Custom); + + var results = await sutProvider.Sut.ValidateAsync([request]); + + var resultsList = results.ToList(); + Assert.Single(resultsList); + Assert.True(resultsList[0].IsError); + Assert.IsType(resultsList[0].AsError); + } + + [Theory] + [BitAutoData] + public async Task ValidateAsync_AdminDeletingAdmin_ReturnsValidResult( + SutProvider sutProvider, + User user, + Guid organizationId, + Guid deletingUserId, + [OrganizationUser(OrganizationUserStatusType.Confirmed, OrganizationUserType.Admin)] OrganizationUser organizationUser) + { + organizationUser.UserId = user.Id; + + var request = new DeleteUserValidationRequest + { + OrganizationId = organizationId, + OrganizationUserId = organizationUser.Id, + OrganizationUser = organizationUser, + User = user, + DeletingUserId = deletingUserId, + IsClaimed = true + }; + + SetupMocks(sutProvider, organizationId, user.Id, OrganizationUserType.Admin); + + var results = await sutProvider.Sut.ValidateAsync([request]); + + var resultsList = results.ToList(); + Assert.Single(resultsList); + Assert.True(resultsList[0].IsValid); + } + + [Theory] + [BitAutoData] + public async Task ValidateAsync_WithMixedValidAndInvalidRequests_ReturnsCorrespondingResults( + SutProvider sutProvider, + User validUser, + User invalidUser, + Guid organizationId, + Guid deletingUserId, + [OrganizationUser] OrganizationUser validOrgUser, + [OrganizationUser(OrganizationUserStatusType.Invited)] OrganizationUser invalidOrgUser) + { + validOrgUser.UserId = validUser.Id; + + invalidOrgUser.UserId = invalidUser.Id; + + var validRequest = new DeleteUserValidationRequest + { + OrganizationId = organizationId, + OrganizationUserId = validOrgUser.Id, + OrganizationUser = validOrgUser, + User = validUser, + DeletingUserId = deletingUserId, + IsClaimed = true + }; + + var invalidRequest = new DeleteUserValidationRequest + { + OrganizationId = organizationId, + OrganizationUserId = invalidOrgUser.Id, + OrganizationUser = invalidOrgUser, + User = invalidUser, + DeletingUserId = deletingUserId, + IsClaimed = true + }; + + SetupMocks(sutProvider, organizationId, validUser.Id); + + var results = await sutProvider.Sut.ValidateAsync([validRequest, invalidRequest]); + + var resultsList = results.ToList(); + Assert.Equal(2, resultsList.Count); + + var validResult = resultsList.First(r => r.Request == validRequest); + var invalidResult = resultsList.First(r => r.Request == invalidRequest); + + Assert.True(validResult.IsValid); + Assert.True(invalidResult.IsError); + Assert.IsType(invalidResult.AsError); + } + + private static void SetupMocks( + SutProvider sutProvider, + Guid organizationId, + Guid userId, + OrganizationUserType currentUserType = OrganizationUserType.Owner) + { + sutProvider.GetDependency() + .OrganizationOwner(organizationId) + .Returns(currentUserType == OrganizationUserType.Owner); + + sutProvider.GetDependency() + .OrganizationAdmin(organizationId) + .Returns(currentUserType is OrganizationUserType.Owner or OrganizationUserType.Admin); + + sutProvider.GetDependency() + .OrganizationCustom(organizationId) + .Returns(currentUserType is OrganizationUserType.Custom); + + sutProvider.GetDependency() + .GetCountByOnlyOwnerAsync(userId) + .Returns(0); + + sutProvider.GetDependency() + .GetCountByOnlyOwnerAsync(userId) + .Returns(0); + } +}