diff --git a/src/Api/AdminConsole/Controllers/CollectionsController.cs b/src/Api/AdminConsole/Controllers/CollectionsController.cs index 9e599cb3dd38..9dee2a9f88e0 100644 --- a/src/Api/AdminConsole/Controllers/CollectionsController.cs +++ b/src/Api/AdminConsole/Controllers/CollectionsController.cs @@ -5,7 +5,11 @@ using Bit.Api.AdminConsole.Models.Request; using Bit.Api.AdminConsole.Models.Response; using Bit.Api.Models.Response; +using Bit.Core; +using Bit.Core.AdminConsole.AbilitiesCache; using Bit.Core.AdminConsole.OrganizationFeatures.Collections.Interfaces; +using Bit.Core.AdminConsole.OrganizationFeatures.Collections.ModifyGroupAccess; +using Bit.Core.AdminConsole.OrganizationFeatures.Collections.ModifyUserAccess; using Bit.Core.AdminConsole.Services; using Bit.Core.Context; using Bit.Core.Entities; @@ -21,7 +25,7 @@ namespace Bit.Api.AdminConsole.Controllers; [Route("organizations/{orgId}/collections")] [Authorize("Application")] -public class CollectionsController : Controller +public class CollectionsController : BaseAdminConsoleController { private readonly ICollectionRepository _collectionRepository; private readonly ICreateCollectionCommand _createCollectionCommand; @@ -32,6 +36,11 @@ public class CollectionsController : Controller private readonly ICurrentContext _currentContext; private readonly IBulkAddCollectionAccessCommand _bulkAddCollectionAccessCommand; private readonly IProviderService _providerService; + private readonly ICollectionAuthorizationService _collectionAuthorizationService; + private readonly IOrganizationAbilityCacheService _organizationAbilityCacheService; + private readonly IOrganizationUserRepository _organizationUserRepository; + private readonly IModifyCollectionUserAccessCommand _modifyCollectionUserAccessCommand; + private readonly IModifyCollectionGroupAccessCommand _modifyCollectionGroupAccessCommand; public CollectionsController( ICollectionRepository collectionRepository, @@ -42,7 +51,12 @@ public CollectionsController( IAuthorizationService authorizationService, ICurrentContext currentContext, IBulkAddCollectionAccessCommand bulkAddCollectionAccessCommand, - IProviderService providerService) + IProviderService providerService, + ICollectionAuthorizationService collectionAuthorizationService, + IOrganizationAbilityCacheService organizationAbilityCacheService, + IOrganizationUserRepository organizationUserRepository, + IModifyCollectionUserAccessCommand modifyCollectionUserAccessCommand, + IModifyCollectionGroupAccessCommand modifyCollectionGroupAccessCommand) { _collectionRepository = collectionRepository; _createCollectionCommand = createCollectionCommand; @@ -53,6 +67,11 @@ public CollectionsController( _currentContext = currentContext; _bulkAddCollectionAccessCommand = bulkAddCollectionAccessCommand; _providerService = providerService; + _collectionAuthorizationService = collectionAuthorizationService; + _organizationAbilityCacheService = organizationAbilityCacheService; + _organizationUserRepository = organizationUserRepository; + _modifyCollectionUserAccessCommand = modifyCollectionUserAccessCommand; + _modifyCollectionGroupAccessCommand = modifyCollectionGroupAccessCommand; } [HttpGet("{id}")] @@ -225,6 +244,71 @@ public async Task PostPut(Guid orgId, Guid id, [FromBod return await Put(orgId, id, model); } + /// + /// Like , but takes add/update/remove deltas for access instead of a full replace list. + /// + [HttpPatch("{id}")] + [Bitwarden.Server.Sdk.Features.RequireFeature(FeatureFlagKeys.PM12473CollectionUserAccessEndpoint)] + public async Task PatchWithDelta(Guid orgId, Guid id, [FromBody] UpdateCollectionWithDeltaRequestModel model) + { + var authorized = await _collectionAuthorizationService.AuthorizeUpdateAsync(orgId, id); + if (!authorized) + { + throw new NotFoundException(); + } + + // Persistence needs its own copy of the collection's current access details for the delta commands below. + var (collection, accessDetails) = await _collectionRepository.GetByIdWithAccessAsync(id); + if (collection is null || collection.OrganizationId != orgId) + { + throw new NotFoundException(); + } + + var userTargets = new[] { new CollectionUserAccessTarget(collection, accessDetails) }; + var groupTargets = new[] { new CollectionGroupAccessTarget(collection, accessDetails) }; + + var organizationAbility = await _organizationAbilityCacheService.GetOrganizationAbilityAsync(orgId); + var allowAdminAccessToAllCollectionItems = + organizationAbility is { AllowAdminAccessToAllCollectionItems: true }; + + var callerOrganizationUser = _currentContext.UserId.HasValue + ? await _organizationUserRepository.GetByOrganizationAsync(orgId, _currentContext.UserId.Value) + : null; + + if (string.IsNullOrEmpty(collection.DefaultUserCollectionEmail) && !string.IsNullOrWhiteSpace(model.Name)) + { + collection.Name = model.Name; + } + collection.ExternalId = model.ExternalId; + + await _updateCollectionCommand.UpdateAsync(collection); + + var userRequest = new ModifyCollectionUserAccessRequest( + userTargets, + model.Users.Add.Select(u => u.ToSelectionReadOnly()).ToList(), + model.Users.Update.Select(u => u.ToSelectionReadOnly()).ToList(), + model.Users.Remove.ToList(), + callerOrganizationUser?.Id, + allowAdminAccessToAllCollectionItems); + + var userResult = await _modifyCollectionUserAccessCommand.ModifyAsync(userRequest); + if (userResult.IsError) + { + return Handle(userResult, _ => TypedResults.NoContent()); + } + + var groupRequest = new ModifyCollectionGroupAccessRequest( + groupTargets, + model.Groups.Add.Select(g => g.ToSelectionReadOnly()).ToList(), + model.Groups.Update.Select(g => g.ToSelectionReadOnly()).ToList(), + model.Groups.Remove.ToList(), + callerOrganizationUser?.Id, + allowAdminAccessToAllCollectionItems); + + var groupResult = await _modifyCollectionGroupAccessCommand.ModifyAsync(groupRequest); + return Handle(groupResult); + } + [HttpPost("bulk-access")] public async Task PostBulkCollectionAccess(Guid orgId, [FromBody] BulkCollectionAccessRequestModel model) { diff --git a/src/Api/AdminConsole/Models/Request/CollectionGroupAccessDeltaRequestModel.cs b/src/Api/AdminConsole/Models/Request/CollectionGroupAccessDeltaRequestModel.cs new file mode 100644 index 000000000000..44a857074bbe --- /dev/null +++ b/src/Api/AdminConsole/Models/Request/CollectionGroupAccessDeltaRequestModel.cs @@ -0,0 +1,13 @@ +using Bit.Api.Models.Request; + +namespace Bit.Api.AdminConsole.Models.Request; + +/// +/// Explicit add/update/remove changes to a collection's group access, rather than the full desired list. +/// +public class CollectionGroupAccessDeltaRequestModel +{ + public IEnumerable Add { get; set; } = []; + public IEnumerable Update { get; set; } = []; + public IEnumerable Remove { get; set; } = []; +} diff --git a/src/Api/AdminConsole/Models/Request/CollectionUserAccessDeltaRequestModel.cs b/src/Api/AdminConsole/Models/Request/CollectionUserAccessDeltaRequestModel.cs new file mode 100644 index 000000000000..30b907558aaf --- /dev/null +++ b/src/Api/AdminConsole/Models/Request/CollectionUserAccessDeltaRequestModel.cs @@ -0,0 +1,13 @@ +using Bit.Api.Models.Request; + +namespace Bit.Api.AdminConsole.Models.Request; + +/// +/// Explicit add/update/remove changes to a collection's user access, rather than the full desired list. +/// +public class CollectionUserAccessDeltaRequestModel +{ + public IEnumerable Add { get; set; } = []; + public IEnumerable Update { get; set; } = []; + public IEnumerable Remove { get; set; } = []; +} diff --git a/src/Api/AdminConsole/Models/Request/UpdateCollectionWithDeltaRequestModel.cs b/src/Api/AdminConsole/Models/Request/UpdateCollectionWithDeltaRequestModel.cs new file mode 100644 index 000000000000..65a8ad884181 --- /dev/null +++ b/src/Api/AdminConsole/Models/Request/UpdateCollectionWithDeltaRequestModel.cs @@ -0,0 +1,22 @@ +#nullable enable +using System.ComponentModel.DataAnnotations; +using Bit.Core.Utilities; + +namespace Bit.Api.AdminConsole.Models.Request; + +/// +/// Updates a collection's metadata alongside add/update/remove deltas for its user and group access. +/// +public class UpdateCollectionWithDeltaRequestModel +{ + [EncryptedString] + [EncryptedStringLength(1000)] + public string? Name { get; set; } + + [StringLength(300)] + public string? ExternalId { get; set; } + + public CollectionUserAccessDeltaRequestModel Users { get; set; } = new(); + + public CollectionGroupAccessDeltaRequestModel Groups { get; set; } = new(); +} diff --git a/src/Core/AdminConsole/OrganizationFeatures/Collections/ModifyGroupAccess/Errors.cs b/src/Core/AdminConsole/OrganizationFeatures/Collections/ModifyGroupAccess/Errors.cs new file mode 100644 index 000000000000..e10eb9719b5c --- /dev/null +++ b/src/Core/AdminConsole/OrganizationFeatures/Collections/ModifyGroupAccess/Errors.cs @@ -0,0 +1,13 @@ +using Bit.Core.AdminConsole.Utilities.v2; + +namespace Bit.Core.AdminConsole.OrganizationFeatures.Collections.ModifyGroupAccess; + +public record DuplicateGroupId() : BadRequestError("A group id cannot be listed more than once within add or update."); +public record OverlappingGroupId() : BadRequestError("A group id cannot appear in more than one of add, update, or remove."); +public record CannotModifyDefaultUserCollectionAccess() : BadRequestError("You cannot modify group access on a collection with the type as DefaultUserCollection."); +public record GroupAlreadyHasAccess() : BadRequestError("Cannot add access for a group that already has access to this collection."); +public record GroupDoesNotHaveAccess() : BadRequestError("Cannot update access for a group that does not currently have access to this collection."); +public record GroupsNotFound() : BadRequestError("One or more groups do not exist."); +public record GroupsNotInOrganization() : BadRequestError("One or more groups do not belong to the same organization as the collection being assigned."); +public record NoRemainingManageAccess() : BadRequestError("At least one member or group must have can manage permission."); +public record InvalidManageAssociation() : BadRequestError("The Manage property is mutually exclusive and cannot be true while the ReadOnly or HidePasswords properties are also true."); diff --git a/src/Core/AdminConsole/OrganizationFeatures/Collections/ModifyGroupAccess/IModifyCollectionGroupAccessCommand.cs b/src/Core/AdminConsole/OrganizationFeatures/Collections/ModifyGroupAccess/IModifyCollectionGroupAccessCommand.cs new file mode 100644 index 000000000000..a992f70c9466 --- /dev/null +++ b/src/Core/AdminConsole/OrganizationFeatures/Collections/ModifyGroupAccess/IModifyCollectionGroupAccessCommand.cs @@ -0,0 +1,11 @@ +using Bit.Core.AdminConsole.Utilities.v2.Results; + +namespace Bit.Core.AdminConsole.OrganizationFeatures.Collections.ModifyGroupAccess; + +public interface IModifyCollectionGroupAccessCommand +{ + /// + /// Validates and applies an add/update/remove delta to one or more collections' group access. + /// + Task ModifyAsync(ModifyCollectionGroupAccessRequest request); +} diff --git a/src/Core/AdminConsole/OrganizationFeatures/Collections/ModifyGroupAccess/IModifyCollectionGroupAccessValidator.cs b/src/Core/AdminConsole/OrganizationFeatures/Collections/ModifyGroupAccess/IModifyCollectionGroupAccessValidator.cs new file mode 100644 index 000000000000..99d23ad2ea2b --- /dev/null +++ b/src/Core/AdminConsole/OrganizationFeatures/Collections/ModifyGroupAccess/IModifyCollectionGroupAccessValidator.cs @@ -0,0 +1,11 @@ +using Bit.Core.AdminConsole.Utilities.v2.Validation; + +namespace Bit.Core.AdminConsole.OrganizationFeatures.Collections.ModifyGroupAccess; + +/// +/// Checks whether an add/update/remove delta to collection group access may be applied. +/// +public interface IModifyCollectionGroupAccessValidator +{ + Task> ValidateAsync(ModifyCollectionGroupAccessRequest request); +} diff --git a/src/Core/AdminConsole/OrganizationFeatures/Collections/ModifyGroupAccess/ModifyCollectionGroupAccessCommand.cs b/src/Core/AdminConsole/OrganizationFeatures/Collections/ModifyGroupAccess/ModifyCollectionGroupAccessCommand.cs new file mode 100644 index 000000000000..4bb1a5c4e637 --- /dev/null +++ b/src/Core/AdminConsole/OrganizationFeatures/Collections/ModifyGroupAccess/ModifyCollectionGroupAccessCommand.cs @@ -0,0 +1,48 @@ +using Bit.Core.AdminConsole.Utilities.v2.Results; +using Bit.Core.Enums; +using Bit.Core.Repositories; +using Bit.Core.Services; +using OneOf.Types; + +namespace Bit.Core.AdminConsole.OrganizationFeatures.Collections.ModifyGroupAccess; + +public class ModifyCollectionGroupAccessCommand( + ICollectionRepository collectionRepository, + IModifyCollectionGroupAccessValidator validator, + IEventService eventService, + TimeProvider timeProvider) : IModifyCollectionGroupAccessCommand +{ + public async Task ModifyAsync(ModifyCollectionGroupAccessRequest request) + { + // Nothing to do, so skip saving and logging. + if (request.Add.Count == 0 && request.Update.Count == 0 && request.Remove.Count == 0) + { + return new None(); + } + + var validationResult = await validator.ValidateAsync(request); + if (validationResult.IsError) + { + return validationResult.AsError; + } + + var revisionDate = timeProvider.GetUtcNow().UtcDateTime; + var upserts = request.Add.Concat(request.Update).ToList(); + + // Drop ids that aren't members, so we don't bump an unrelated group's revision date. + var existingGroupIds = request.Targets + .SelectMany(t => t.AccessDetails.Groups.Select(g => g.Id)) + .ToHashSet(); + var removeIds = request.Remove.Where(existingGroupIds.Contains).ToList(); + + var organizationId = request.Targets.First().Collection.OrganizationId; + var collectionIds = request.Targets.Select(t => t.Collection.Id).ToList(); + + await collectionRepository.ModifyGroupAccessAsync(organizationId, collectionIds, upserts, removeIds, revisionDate); + + await eventService.LogCollectionEventsAsync( + request.Targets.Select(t => (t.Collection, EventType.Collection_Updated, (DateTime?)revisionDate))); + + return new None(); + } +} diff --git a/src/Core/AdminConsole/OrganizationFeatures/Collections/ModifyGroupAccess/ModifyCollectionGroupAccessRequest.cs b/src/Core/AdminConsole/OrganizationFeatures/Collections/ModifyGroupAccess/ModifyCollectionGroupAccessRequest.cs new file mode 100644 index 000000000000..9848a73953a7 --- /dev/null +++ b/src/Core/AdminConsole/OrganizationFeatures/Collections/ModifyGroupAccess/ModifyCollectionGroupAccessRequest.cs @@ -0,0 +1,14 @@ +using Bit.Core.Entities; +using Bit.Core.Models.Data; + +namespace Bit.Core.AdminConsole.OrganizationFeatures.Collections.ModifyGroupAccess; + +public record CollectionGroupAccessTarget(Collection Collection, CollectionAccessDetails AccessDetails); + +public record ModifyCollectionGroupAccessRequest( + IReadOnlyCollection Targets, + IReadOnlyCollection Add, + IReadOnlyCollection Update, + IReadOnlyCollection Remove, + Guid? PerformingOrganizationUserId, + bool AllowAdminAccessToAllCollectionItems); diff --git a/src/Core/AdminConsole/OrganizationFeatures/Collections/ModifyGroupAccess/ModifyCollectionGroupAccessValidator.cs b/src/Core/AdminConsole/OrganizationFeatures/Collections/ModifyGroupAccess/ModifyCollectionGroupAccessValidator.cs new file mode 100644 index 000000000000..a876c5a725a9 --- /dev/null +++ b/src/Core/AdminConsole/OrganizationFeatures/Collections/ModifyGroupAccess/ModifyCollectionGroupAccessValidator.cs @@ -0,0 +1,105 @@ +using Bit.Core.AdminConsole.Repositories; +using Bit.Core.AdminConsole.Utilities.v2.Validation; +using Bit.Core.Enums; +using Bit.Core.Models.Data; +using static Bit.Core.AdminConsole.Utilities.v2.Validation.ValidationResultHelpers; + +namespace Bit.Core.AdminConsole.OrganizationFeatures.Collections.ModifyGroupAccess; + +public class ModifyCollectionGroupAccessValidator(IGroupRepository groupRepository) + : IModifyCollectionGroupAccessValidator +{ + public async Task> ValidateAsync( + ModifyCollectionGroupAccessRequest request) + { + if (HasDuplicateIds(request.Add) || HasDuplicateIds(request.Update)) + { + return Invalid(request, new DuplicateGroupId()); + } + + var addIds = request.Add.Select(a => a.Id).ToHashSet(); + var updateIds = request.Update.Select(u => u.Id).ToHashSet(); + var removeIds = request.Remove.ToHashSet(); + + if (addIds.Overlaps(updateIds) || addIds.Overlaps(removeIds) || updateIds.Overlaps(removeIds)) + { + return Invalid(request, new OverlappingGroupId()); + } + + if (request.Add.Concat(request.Update).Any(s => s.Manage && (s.ReadOnly || s.HidePasswords))) + { + return Invalid(request, new InvalidManageAssociation()); + } + + if (request.Targets.Any(t => t.Collection.Type == CollectionType.DefaultUserCollection)) + { + return Invalid(request, new CannotModifyDefaultUserCollectionAccess()); + } + + // Only meaningful for a single collection: across several, a group may already have access to one + // target but not another. + if (request.Targets.Count == 1) + { + var existingIds = request.Targets.Single().AccessDetails.Groups.Select(g => g.Id).ToHashSet(); + if (addIds.Any(existingIds.Contains)) + { + return Invalid(request, new GroupAlreadyHasAccess()); + } + + if (updateIds.Any(id => !existingIds.Contains(id))) + { + return Invalid(request, new GroupDoesNotHaveAccess()); + } + } + + var upsertIds = addIds.Concat(updateIds).ToList(); + if (upsertIds.Count > 0) + { + var organizationId = request.Targets.First().Collection.OrganizationId; + var groups = await groupRepository.GetManyByManyIds(upsertIds); + if (groups.Count != upsertIds.Count) + { + return Invalid(request, new GroupsNotFound()); + } + + if (groups.Any(g => g.OrganizationId != organizationId)) + { + return Invalid(request, new GroupsNotInOrganization()); + } + } + + if (!request.AllowAdminAccessToAllCollectionItems + && request.Targets.Any(t => !HasRemainingManageAccess(t, request, removeIds))) + { + return Invalid(request, new NoRemainingManageAccess()); + } + + return Valid(request); + } + + private static bool HasRemainingManageAccess( + CollectionGroupAccessTarget target, ModifyCollectionGroupAccessRequest request, HashSet removeIds) + { + if (target.AccessDetails.Users.Any(u => u.Manage)) + { + return true; + } + + var existingIds = target.AccessDetails.Groups.Select(g => g.Id).ToHashSet(); + var updatedById = request.Update.ToDictionary(u => u.Id); + var finalGroups = target.AccessDetails.Groups + .Where(g => !removeIds.Contains(g.Id)) + .Select(g => updatedById.GetValueOrDefault(g.Id, g)) + .Concat(request.Add) + // An Update entry grants access on targets the group isn't a member of, so it counts as an Add here. + .Concat(request.Update.Where(u => !existingIds.Contains(u.Id))); + + return finalGroups.Any(g => g.Manage); + } + + private static bool HasDuplicateIds(IReadOnlyCollection selections) + { + var ids = selections.Select(s => s.Id).ToList(); + return ids.Count != ids.Distinct().Count(); + } +} diff --git a/src/Core/AdminConsole/OrganizationFeatures/Collections/ModifyUserAccess/Errors.cs b/src/Core/AdminConsole/OrganizationFeatures/Collections/ModifyUserAccess/Errors.cs new file mode 100644 index 000000000000..4a764dd136f3 --- /dev/null +++ b/src/Core/AdminConsole/OrganizationFeatures/Collections/ModifyUserAccess/Errors.cs @@ -0,0 +1,14 @@ +using Bit.Core.AdminConsole.Utilities.v2; + +namespace Bit.Core.AdminConsole.OrganizationFeatures.Collections.ModifyUserAccess; + +public record DuplicateOrganizationUserId() : BadRequestError("An organization user id cannot be listed more than once within add or update."); +public record OverlappingOrganizationUserId() : BadRequestError("An organization user id cannot appear in more than one of add, update, or remove."); +public record CannotModifyDefaultUserCollectionAccess() : BadRequestError("You cannot modify user access on a collection with the type as DefaultUserCollection."); +public record OrganizationUserAlreadyHasAccess() : BadRequestError("Cannot add access for a user who already has access to this collection."); +public record OrganizationUserDoesNotHaveAccess() : BadRequestError("Cannot update access for a user who does not currently have access to this collection."); +public record CannotAddSelfToCollection() : BadRequestError("You cannot add yourself to a collection."); +public record OrganizationUsersNotFound() : BadRequestError("One or more users do not exist."); +public record OrganizationUsersNotInOrganization() : BadRequestError("One or more users do not belong to the same organization as the collection being assigned."); +public record NoRemainingManageAccess() : BadRequestError("At least one member or group must have can manage permission."); +public record InvalidManageAssociation() : BadRequestError("The Manage property is mutually exclusive and cannot be true while the ReadOnly or HidePasswords properties are also true."); diff --git a/src/Core/AdminConsole/OrganizationFeatures/Collections/ModifyUserAccess/IModifyCollectionUserAccessCommand.cs b/src/Core/AdminConsole/OrganizationFeatures/Collections/ModifyUserAccess/IModifyCollectionUserAccessCommand.cs new file mode 100644 index 000000000000..b222e80c2521 --- /dev/null +++ b/src/Core/AdminConsole/OrganizationFeatures/Collections/ModifyUserAccess/IModifyCollectionUserAccessCommand.cs @@ -0,0 +1,11 @@ +using Bit.Core.AdminConsole.Utilities.v2.Results; + +namespace Bit.Core.AdminConsole.OrganizationFeatures.Collections.ModifyUserAccess; + +public interface IModifyCollectionUserAccessCommand +{ + /// + /// Validates and applies an add/update/remove delta to one or more collections' user access. + /// + Task ModifyAsync(ModifyCollectionUserAccessRequest request); +} diff --git a/src/Core/AdminConsole/OrganizationFeatures/Collections/ModifyUserAccess/IModifyCollectionUserAccessValidator.cs b/src/Core/AdminConsole/OrganizationFeatures/Collections/ModifyUserAccess/IModifyCollectionUserAccessValidator.cs new file mode 100644 index 000000000000..1a4e36be9342 --- /dev/null +++ b/src/Core/AdminConsole/OrganizationFeatures/Collections/ModifyUserAccess/IModifyCollectionUserAccessValidator.cs @@ -0,0 +1,11 @@ +using Bit.Core.AdminConsole.Utilities.v2.Validation; + +namespace Bit.Core.AdminConsole.OrganizationFeatures.Collections.ModifyUserAccess; + +/// +/// Checks whether an add/update/remove delta to collection user access may be applied. +/// +public interface IModifyCollectionUserAccessValidator +{ + Task> ValidateAsync(ModifyCollectionUserAccessRequest request); +} diff --git a/src/Core/AdminConsole/OrganizationFeatures/Collections/ModifyUserAccess/ModifyCollectionUserAccessCommand.cs b/src/Core/AdminConsole/OrganizationFeatures/Collections/ModifyUserAccess/ModifyCollectionUserAccessCommand.cs new file mode 100644 index 000000000000..13c17a642b7e --- /dev/null +++ b/src/Core/AdminConsole/OrganizationFeatures/Collections/ModifyUserAccess/ModifyCollectionUserAccessCommand.cs @@ -0,0 +1,48 @@ +using Bit.Core.AdminConsole.Utilities.v2.Results; +using Bit.Core.Enums; +using Bit.Core.Repositories; +using Bit.Core.Services; +using OneOf.Types; + +namespace Bit.Core.AdminConsole.OrganizationFeatures.Collections.ModifyUserAccess; + +public class ModifyCollectionUserAccessCommand( + ICollectionRepository collectionRepository, + IModifyCollectionUserAccessValidator validator, + IEventService eventService, + TimeProvider timeProvider) : IModifyCollectionUserAccessCommand +{ + public async Task ModifyAsync(ModifyCollectionUserAccessRequest request) + { + // Nothing to do, so skip saving and logging. + if (request.Add.Count == 0 && request.Update.Count == 0 && request.Remove.Count == 0) + { + return new None(); + } + + var validationResult = await validator.ValidateAsync(request); + if (validationResult.IsError) + { + return validationResult.AsError; + } + + var revisionDate = timeProvider.GetUtcNow().UtcDateTime; + var upserts = request.Add.Concat(request.Update).ToList(); + + // Drop ids that aren't members, so we don't bump an unrelated user's revision date. + var existingUserIds = request.Targets + .SelectMany(t => t.AccessDetails.Users.Select(u => u.Id)) + .ToHashSet(); + var removeIds = request.Remove.Where(existingUserIds.Contains).ToList(); + + var organizationId = request.Targets.First().Collection.OrganizationId; + var collectionIds = request.Targets.Select(t => t.Collection.Id).ToList(); + + await collectionRepository.ModifyUserAccessAsync(organizationId, collectionIds, upserts, removeIds, revisionDate); + + await eventService.LogCollectionEventsAsync( + request.Targets.Select(t => (t.Collection, EventType.Collection_Updated, (DateTime?)revisionDate))); + + return new None(); + } +} diff --git a/src/Core/AdminConsole/OrganizationFeatures/Collections/ModifyUserAccess/ModifyCollectionUserAccessRequest.cs b/src/Core/AdminConsole/OrganizationFeatures/Collections/ModifyUserAccess/ModifyCollectionUserAccessRequest.cs new file mode 100644 index 000000000000..2225d7fdecac --- /dev/null +++ b/src/Core/AdminConsole/OrganizationFeatures/Collections/ModifyUserAccess/ModifyCollectionUserAccessRequest.cs @@ -0,0 +1,14 @@ +using Bit.Core.Entities; +using Bit.Core.Models.Data; + +namespace Bit.Core.AdminConsole.OrganizationFeatures.Collections.ModifyUserAccess; + +public record CollectionUserAccessTarget(Collection Collection, CollectionAccessDetails AccessDetails); + +public record ModifyCollectionUserAccessRequest( + IReadOnlyCollection Targets, + IReadOnlyCollection Add, + IReadOnlyCollection Update, + IReadOnlyCollection Remove, + Guid? PerformingOrganizationUserId, + bool AllowAdminAccessToAllCollectionItems); diff --git a/src/Core/AdminConsole/OrganizationFeatures/Collections/ModifyUserAccess/ModifyCollectionUserAccessValidator.cs b/src/Core/AdminConsole/OrganizationFeatures/Collections/ModifyUserAccess/ModifyCollectionUserAccessValidator.cs new file mode 100644 index 000000000000..2b276196c8ea --- /dev/null +++ b/src/Core/AdminConsole/OrganizationFeatures/Collections/ModifyUserAccess/ModifyCollectionUserAccessValidator.cs @@ -0,0 +1,115 @@ +using Bit.Core.AdminConsole.Utilities.v2.Validation; +using Bit.Core.Enums; +using Bit.Core.Models.Data; +using Bit.Core.Repositories; +using static Bit.Core.AdminConsole.Utilities.v2.Validation.ValidationResultHelpers; + +namespace Bit.Core.AdminConsole.OrganizationFeatures.Collections.ModifyUserAccess; + +public class ModifyCollectionUserAccessValidator(IOrganizationUserRepository organizationUserRepository) + : IModifyCollectionUserAccessValidator +{ + public async Task> ValidateAsync( + ModifyCollectionUserAccessRequest request) + { + if (HasDuplicateIds(request.Add) || HasDuplicateIds(request.Update)) + { + return Invalid(request, new DuplicateOrganizationUserId()); + } + + var addIds = request.Add.Select(a => a.Id).ToHashSet(); + var updateIds = request.Update.Select(u => u.Id).ToHashSet(); + var removeIds = request.Remove.ToHashSet(); + + if (addIds.Overlaps(updateIds) || addIds.Overlaps(removeIds) || updateIds.Overlaps(removeIds)) + { + return Invalid(request, new OverlappingOrganizationUserId()); + } + + if (request.Add.Concat(request.Update).Any(s => s.Manage && (s.ReadOnly || s.HidePasswords))) + { + return Invalid(request, new InvalidManageAssociation()); + } + + if (request.Targets.Any(t => t.Collection.Type == CollectionType.DefaultUserCollection)) + { + return Invalid(request, new CannotModifyDefaultUserCollectionAccess()); + } + + // Only meaningful for a single collection: across several, a user may already have access to one + // target but not another. + if (request.Targets.Count == 1) + { + var existingIds = request.Targets.Single().AccessDetails.Users.Select(u => u.Id).ToHashSet(); + if (addIds.Any(existingIds.Contains)) + { + return Invalid(request, new OrganizationUserAlreadyHasAccess()); + } + + if (updateIds.Any(id => !existingIds.Contains(id))) + { + return Invalid(request, new OrganizationUserDoesNotHaveAccess()); + } + } + + var upsertIds = addIds.Concat(updateIds).ToList(); + if (upsertIds.Count > 0) + { + var organizationId = request.Targets.First().Collection.OrganizationId; + var organizationUsers = await organizationUserRepository.GetManyAsync(upsertIds); + if (organizationUsers.Count != upsertIds.Count) + { + return Invalid(request, new OrganizationUsersNotFound()); + } + + if (organizationUsers.Any(ou => ou.OrganizationId != organizationId)) + { + return Invalid(request, new OrganizationUsersNotInOrganization()); + } + } + + // Update is checked alongside Add, since it grants access on any target the user isn't yet on. Raising + // your own access on a collection you already belong to is fine; authorization required Manage on it. + if (request.PerformingOrganizationUserId is { } performingId + && (addIds.Contains(performingId) || updateIds.Contains(performingId)) + && !request.AllowAdminAccessToAllCollectionItems + && request.Targets.Any(t => !t.AccessDetails.Users.Any(u => u.Id == performingId))) + { + return Invalid(request, new CannotAddSelfToCollection()); + } + + if (!request.AllowAdminAccessToAllCollectionItems + && request.Targets.Any(t => !HasRemainingManageAccess(t, request, removeIds))) + { + return Invalid(request, new NoRemainingManageAccess()); + } + + return Valid(request); + } + + private static bool HasRemainingManageAccess( + CollectionUserAccessTarget target, ModifyCollectionUserAccessRequest request, HashSet removeIds) + { + if (target.AccessDetails.Groups.Any(g => g.Manage)) + { + return true; + } + + var existingIds = target.AccessDetails.Users.Select(u => u.Id).ToHashSet(); + var updatedById = request.Update.ToDictionary(u => u.Id); + var finalUsers = target.AccessDetails.Users + .Where(u => !removeIds.Contains(u.Id)) + .Select(u => updatedById.GetValueOrDefault(u.Id, u)) + .Concat(request.Add) + // An Update entry grants access on targets the user isn't a member of, so it counts as an Add here. + .Concat(request.Update.Where(u => !existingIds.Contains(u.Id))); + + return finalUsers.Any(u => u.Manage); + } + + private static bool HasDuplicateIds(IReadOnlyCollection selections) + { + var ids = selections.Select(s => s.Id).ToList(); + return ids.Count != ids.Distinct().Count(); + } +} diff --git a/src/Core/AdminConsole/Repositories/ICollectionRepository.cs b/src/Core/AdminConsole/Repositories/ICollectionRepository.cs index 53b60549414f..3bff5d991b49 100644 --- a/src/Core/AdminConsole/Repositories/ICollectionRepository.cs +++ b/src/Core/AdminConsole/Repositories/ICollectionRepository.cs @@ -69,6 +69,31 @@ public interface ICollectionRepository : IRepository Task ReplaceAsync(Collection obj, IEnumerable? groups, IEnumerable? users); Task DeleteUserAsync(Guid collectionId, Guid organizationUserId); + + /// + /// Atomically applies the same user-access upserts and removals to one or more collections. + /// + /// The Organization ID. + /// The Collection IDs to apply the change to. + /// The user access selections to create or update. + /// The Organization User IDs to remove access for. + /// The revision date to use for the collections. + Task ModifyUserAccessAsync(Guid organizationId, IEnumerable collectionIds, + IEnumerable upserts, IEnumerable removeOrganizationUserIds, + DateTime revisionDate); + + /// + /// Atomically applies the same group-access upserts and removals to one or more collections. + /// + /// The Organization ID. + /// The Collection IDs to apply the change to. + /// The group access selections to create or update. + /// The Group IDs to remove access for. + /// The revision date to use for the collections. + Task ModifyGroupAccessAsync(Guid organizationId, IEnumerable collectionIds, + IEnumerable upserts, IEnumerable removeGroupIds, + DateTime revisionDate); + Task UpdateUsersAsync(Guid id, IEnumerable users); Task> GetManyUsersByIdAsync(Guid id); Task DeleteManyAsync(IEnumerable collectionIds); diff --git a/src/Core/Constants.cs b/src/Core/Constants.cs index a7c96da65911..eb38f80f26b4 100644 --- a/src/Core/Constants.cs +++ b/src/Core/Constants.cs @@ -145,6 +145,7 @@ public static partial class FeatureFlagKeys public const string PoliciesInAcceptedState = "pm-34145-policies-in-accepted-state"; public const string ChangeMemberEmailNoMp = "pm-28365-change-member-email-no-mp"; public const string PM34423StagedStatus = "pm-34423-staged-status"; + public const string PM12473CollectionUserAccessEndpoint = "pm-12473-collection-user-access-endpoint"; /* Architecture */ public const string DesktopMigrationMilestone1 = "desktop-ui-migration-milestone-1"; diff --git a/src/Core/OrganizationFeatures/OrganizationServiceCollectionExtensions.cs b/src/Core/OrganizationFeatures/OrganizationServiceCollectionExtensions.cs index 88f631100866..f3faa89d8f0b 100644 --- a/src/Core/OrganizationFeatures/OrganizationServiceCollectionExtensions.cs +++ b/src/Core/OrganizationFeatures/OrganizationServiceCollectionExtensions.cs @@ -3,6 +3,8 @@ using Bit.Core.AdminConsole.OrganizationFeatures.AccountRecovery; using Bit.Core.AdminConsole.OrganizationFeatures.Collections; using Bit.Core.AdminConsole.OrganizationFeatures.Collections.Interfaces; +using Bit.Core.AdminConsole.OrganizationFeatures.Collections.ModifyGroupAccess; +using Bit.Core.AdminConsole.OrganizationFeatures.Collections.ModifyUserAccess; using Bit.Core.AdminConsole.OrganizationFeatures.Groups; using Bit.Core.AdminConsole.OrganizationFeatures.Groups.Interfaces; using Bit.Core.AdminConsole.OrganizationFeatures.Import; @@ -194,6 +196,10 @@ public static void AddOrganizationCollectionCommands(this IServiceCollection ser services.AddScoped(); services.AddScoped(); services.AddScoped(); + services.AddScoped(); + services.AddScoped(); + services.AddScoped(); + services.AddScoped(); } private static void AddOrganizationGroupCommands(this IServiceCollection services) diff --git a/src/Infrastructure.Dapper/AdminConsole/Repositories/CollectionRepository.cs b/src/Infrastructure.Dapper/AdminConsole/Repositories/CollectionRepository.cs index 4f6554b71a6d..e4e55b645e06 100644 --- a/src/Infrastructure.Dapper/AdminConsole/Repositories/CollectionRepository.cs +++ b/src/Infrastructure.Dapper/AdminConsole/Repositories/CollectionRepository.cs @@ -374,6 +374,106 @@ public async Task DeleteUserAsync(Guid collectionId, Guid organizationUserId) } } + public async Task ModifyUserAccessAsync(Guid organizationId, IEnumerable collectionIds, + IEnumerable upserts, IEnumerable removeOrganizationUserIds, + DateTime revisionDate) + { + collectionIds = collectionIds.ToList(); + upserts = upserts.ToList(); + removeOrganizationUserIds = removeOrganizationUserIds.ToList(); + + await using var connection = new SqlConnection(ConnectionString); + await connection.OpenAsync(); + await using var transaction = await connection.BeginTransactionAsync(); + + try + { + if (removeOrganizationUserIds.Any()) + { + await connection.ExecuteAsync( + $"[{Schema}].[CollectionUser_DeleteMany]", + new + { + CollectionIds = collectionIds.ToGuidIdArrayTVP(), + OrganizationUserIds = removeOrganizationUserIds.ToGuidIdArrayTVP() + }, + commandType: CommandType.StoredProcedure, + transaction: transaction); + } + + // Run this even with no upserts, so a remove-only request still bumps revision dates. + await connection.ExecuteAsync( + $"[{Schema}].[Collection_CreateOrUpdateAccessForMany]", + new + { + OrganizationId = organizationId, + CollectionIds = collectionIds.ToGuidIdArrayTVP(), + Users = upserts.ToArrayTVP(), + Groups = Enumerable.Empty().ToArrayTVP(), + RevisionDate = revisionDate + }, + commandType: CommandType.StoredProcedure, + transaction: transaction); + + await transaction.CommitAsync(); + } + catch + { + await transaction.RollbackAsync(); + throw; + } + } + + public async Task ModifyGroupAccessAsync(Guid organizationId, IEnumerable collectionIds, + IEnumerable upserts, IEnumerable removeGroupIds, + DateTime revisionDate) + { + collectionIds = collectionIds.ToList(); + upserts = upserts.ToList(); + removeGroupIds = removeGroupIds.ToList(); + + await using var connection = new SqlConnection(ConnectionString); + await connection.OpenAsync(); + await using var transaction = await connection.BeginTransactionAsync(); + + try + { + if (removeGroupIds.Any()) + { + await connection.ExecuteAsync( + $"[{Schema}].[CollectionGroup_DeleteMany]", + new + { + CollectionIds = collectionIds.ToGuidIdArrayTVP(), + GroupIds = removeGroupIds.ToGuidIdArrayTVP() + }, + commandType: CommandType.StoredProcedure, + transaction: transaction); + } + + // Run this even with no upserts, so a remove-only request still bumps revision dates. + await connection.ExecuteAsync( + $"[{Schema}].[Collection_CreateOrUpdateAccessForMany]", + new + { + OrganizationId = organizationId, + CollectionIds = collectionIds.ToGuidIdArrayTVP(), + Users = Enumerable.Empty().ToArrayTVP(), + Groups = upserts.ToArrayTVP(), + RevisionDate = revisionDate + }, + commandType: CommandType.StoredProcedure, + transaction: transaction); + + await transaction.CommitAsync(); + } + catch + { + await transaction.RollbackAsync(); + throw; + } + } + public async Task UpdateUsersAsync(Guid id, IEnumerable users) { using (var connection = new SqlConnection(ConnectionString)) diff --git a/src/Infrastructure.EntityFramework/AdminConsole/Repositories/CollectionRepository.cs b/src/Infrastructure.EntityFramework/AdminConsole/Repositories/CollectionRepository.cs index 5223f97b4a57..3b8726f9abb5 100644 --- a/src/Infrastructure.EntityFramework/AdminConsole/Repositories/CollectionRepository.cs +++ b/src/Infrastructure.EntityFramework/AdminConsole/Repositories/CollectionRepository.cs @@ -115,6 +115,202 @@ public async Task DeleteUserAsync(Guid collectionId, Guid organizationUserId) } } + public async Task ModifyUserAccessAsync(Guid organizationId, IEnumerable collectionIds, + IEnumerable upserts, IEnumerable removeOrganizationUserIds, + DateTime revisionDate) + { + collectionIds = collectionIds.ToList(); + upserts = upserts.ToList(); + removeOrganizationUserIds = removeOrganizationUserIds.ToList(); + + using (var scope = ServiceScopeFactory.CreateScope()) + { + var dbContext = GetDatabaseContext(scope); + await using var transaction = await dbContext.Database.BeginTransactionAsync(); + + try + { + if (removeOrganizationUserIds.Any()) + { + var toRemove = await dbContext.CollectionUsers + .Where(cu => collectionIds.Contains(cu.CollectionId) && + removeOrganizationUserIds.Contains(cu.OrganizationUserId)) + .ToListAsync(); + dbContext.RemoveRange(toRemove); + await dbContext.UserBumpAccountRevisionDateByOrganizationUserIdsAsync(removeOrganizationUserIds); + await dbContext.SaveChangesAsync(); + } + + if (upserts.Any()) + { + var upsertIds = upserts.Select(u => u.Id).ToList(); + var validOrganizationUserIds = (await dbContext.OrganizationUsers + .Where(ou => ou.OrganizationId == organizationId && upsertIds.Contains(ou.Id)) + .Select(ou => ou.Id) + .ToListAsync()).ToHashSet(); + + var existingCollectionUsers = await dbContext.CollectionUsers + .Where(cu => collectionIds.Contains(cu.CollectionId)) + .ToDictionaryAsync(x => (x.CollectionId, x.OrganizationUserId)); + + // Skip ids that aren't in this organization, same as the SQL version's join does. + var validUpserts = upserts.Where(u => validOrganizationUserIds.Contains(u.Id)).ToList(); + foreach (var collectionId in collectionIds) + { + foreach (var requestedUser in validUpserts) + { + if (!existingCollectionUsers.TryGetValue( + (collectionId, requestedUser.Id), out var existingCollectionUser)) + { + // This is a brand new entry + dbContext.CollectionUsers.Add(new CollectionUser + { + CollectionId = collectionId, + OrganizationUserId = requestedUser.Id, + HidePasswords = requestedUser.HidePasswords, + ReadOnly = requestedUser.ReadOnly, + Manage = requestedUser.Manage + }); + continue; + } + + // It already exists, update it + existingCollectionUser.HidePasswords = requestedUser.HidePasswords; + existingCollectionUser.ReadOnly = requestedUser.ReadOnly; + existingCollectionUser.Manage = requestedUser.Manage; + dbContext.CollectionUsers.Update(existingCollectionUser); + } + } + + await dbContext.SaveChangesAsync(); + } + + var collections = await dbContext.Collections + .Where(c => collectionIds.Contains(c.Id)) + .ToListAsync(); + foreach (var collection in collections) + { + collection.RevisionDate = revisionDate; + } + + // Bump everyone with access to a target collection now that the changes above are saved. + // Same order as CreateOrUpdateAccessForManyAsync. + await dbContext.UserBumpAccountRevisionDateByCollectionIdsAsync(collectionIds, organizationId); + await dbContext.SaveChangesAsync(); + + await transaction.CommitAsync(); + } + catch + { + await transaction.RollbackAsync(); + throw; + } + } + } + + public async Task ModifyGroupAccessAsync(Guid organizationId, IEnumerable collectionIds, + IEnumerable upserts, IEnumerable removeGroupIds, + DateTime revisionDate) + { + collectionIds = collectionIds.ToList(); + upserts = upserts.ToList(); + removeGroupIds = removeGroupIds.ToList(); + + using (var scope = ServiceScopeFactory.CreateScope()) + { + var dbContext = GetDatabaseContext(scope); + await using var transaction = await dbContext.Database.BeginTransactionAsync(); + + try + { + if (removeGroupIds.Any()) + { + var toRemove = await dbContext.CollectionGroups + .Where(cu => collectionIds.Contains(cu.CollectionId) && + removeGroupIds.Contains(cu.GroupId)) + .ToListAsync(); + dbContext.RemoveRange(toRemove); + + // Bump the revision date of the affected groups inline - there is no + // Group_BumpRevisionDateByIds sproc, so we mirror the SQL version's behavior here. + var removedGroups = await dbContext.Groups + .Where(g => removeGroupIds.Contains(g.Id)) + .ToListAsync(); + foreach (var g in removedGroups) + { + g.RevisionDate = DateTime.UtcNow; + } + + await dbContext.SaveChangesAsync(); + } + + if (upserts.Any()) + { + var upsertIds = upserts.Select(u => u.Id).ToList(); + var validGroupIds = (await dbContext.Groups + .Where(g => g.OrganizationId == organizationId && upsertIds.Contains(g.Id)) + .Select(g => g.Id) + .ToListAsync()).ToHashSet(); + + var existingCollectionGroups = await dbContext.CollectionGroups + .Where(cg => collectionIds.Contains(cg.CollectionId)) + .ToDictionaryAsync(x => (x.CollectionId, x.GroupId)); + + // Skip ids that aren't in this organization, same as the SQL version's join does. + var validUpserts = upserts.Where(u => validGroupIds.Contains(u.Id)).ToList(); + foreach (var collectionId in collectionIds) + { + foreach (var requestedGroup in validUpserts) + { + if (!existingCollectionGroups.TryGetValue( + (collectionId, requestedGroup.Id), out var existingCollectionGroup)) + { + // This is a brand new entry + dbContext.CollectionGroups.Add(new CollectionGroup + { + CollectionId = collectionId, + GroupId = requestedGroup.Id, + HidePasswords = requestedGroup.HidePasswords, + ReadOnly = requestedGroup.ReadOnly, + Manage = requestedGroup.Manage + }); + continue; + } + + // It already exists, update it + existingCollectionGroup.HidePasswords = requestedGroup.HidePasswords; + existingCollectionGroup.ReadOnly = requestedGroup.ReadOnly; + existingCollectionGroup.Manage = requestedGroup.Manage; + dbContext.CollectionGroups.Update(existingCollectionGroup); + } + } + + await dbContext.SaveChangesAsync(); + } + + var collections = await dbContext.Collections + .Where(c => collectionIds.Contains(c.Id)) + .ToListAsync(); + foreach (var collection in collections) + { + collection.RevisionDate = revisionDate; + } + + // Bump everyone with access to a target collection now that the changes above are saved. + // Same order as CreateOrUpdateAccessForManyAsync. + await dbContext.UserBumpAccountRevisionDateByCollectionIdsAsync(collectionIds, organizationId); + await dbContext.SaveChangesAsync(); + + await transaction.CommitAsync(); + } + catch + { + await transaction.RollbackAsync(); + throw; + } + } + } + public async Task> GetByIdWithAccessAsync(Guid id) { var collection = await base.GetByIdAsync(id); diff --git a/src/Sql/dbo/AdminConsole/Stored Procedures/CollectionGroup_DeleteMany.sql b/src/Sql/dbo/AdminConsole/Stored Procedures/CollectionGroup_DeleteMany.sql new file mode 100644 index 000000000000..de8089c244d1 --- /dev/null +++ b/src/Sql/dbo/AdminConsole/Stored Procedures/CollectionGroup_DeleteMany.sql @@ -0,0 +1,21 @@ +CREATE PROCEDURE [dbo].[CollectionGroup_DeleteMany] + @CollectionIds [dbo].[GuidIdArray] READONLY, + @GroupIds [dbo].[GuidIdArray] READONLY +AS +BEGIN + SET NOCOUNT ON + + DELETE + FROM + [dbo].[CollectionGroup] + WHERE + [CollectionId] IN (SELECT [Id] FROM @CollectionIds) + AND [GroupId] IN (SELECT [Id] FROM @GroupIds) + + UPDATE + [dbo].[Group] + SET + [RevisionDate] = GETUTCDATE() + WHERE + [Id] IN (SELECT [Id] FROM @GroupIds) +END diff --git a/src/Sql/dbo/AdminConsole/Stored Procedures/CollectionUser_DeleteMany.sql b/src/Sql/dbo/AdminConsole/Stored Procedures/CollectionUser_DeleteMany.sql new file mode 100644 index 000000000000..00aa81b4f05b --- /dev/null +++ b/src/Sql/dbo/AdminConsole/Stored Procedures/CollectionUser_DeleteMany.sql @@ -0,0 +1,16 @@ +CREATE PROCEDURE [dbo].[CollectionUser_DeleteMany] + @CollectionIds [dbo].[GuidIdArray] READONLY, + @OrganizationUserIds [dbo].[GuidIdArray] READONLY +AS +BEGIN + SET NOCOUNT ON + + DELETE + FROM + [dbo].[CollectionUser] + WHERE + [CollectionId] IN (SELECT [Id] FROM @CollectionIds) + AND [OrganizationUserId] IN (SELECT [Id] FROM @OrganizationUserIds) + + EXEC [dbo].[User_BumpAccountRevisionDateByOrganizationUserIds] @OrganizationUserIds +END diff --git a/test/Api.IntegrationTest/AdminConsole/Controllers/CollectionsControllerPatchWithDeltaTests.cs b/test/Api.IntegrationTest/AdminConsole/Controllers/CollectionsControllerPatchWithDeltaTests.cs new file mode 100644 index 000000000000..782ed387add5 --- /dev/null +++ b/test/Api.IntegrationTest/AdminConsole/Controllers/CollectionsControllerPatchWithDeltaTests.cs @@ -0,0 +1,105 @@ +using System.Net; +using Bit.Api.AdminConsole.Models.Request; +using Bit.Api.IntegrationTest.Factories; +using Bit.Api.IntegrationTest.Helpers; +using Bit.Api.Models.Request; +using Bit.Core; +using Bit.Core.AdminConsole.Entities; +using Bit.Core.Billing.Enums; +using Bit.Core.Enums; +using Bit.Core.Models.Data; +using Bit.Core.Platform.Push; +using Bit.Core.Repositories; +using NSubstitute; +using Xunit; + +namespace Bit.Api.IntegrationTest.AdminConsole.Controllers; + +public class CollectionsControllerPatchWithDeltaTests : IClassFixture, IAsyncLifetime +{ + private readonly HttpClient _client; + private readonly ApiApplicationFactory _factory; + private readonly LoginHelper _loginHelper; + + private string _ownerEmail = null!; + private Organization _organization = null!; + + public CollectionsControllerPatchWithDeltaTests(ApiApplicationFactory factory) + { + _factory = factory; + _factory.SubstituteService(_ => { }); + _factory.SubstituteService(_ => { }); + _factory.SubstituteService(featureService => + featureService.IsEnabled(FeatureFlagKeys.PM12473CollectionUserAccessEndpoint, Arg.Any()) + .Returns(true)); + _client = factory.CreateClient(); + _loginHelper = new LoginHelper(_factory, _client); + } + + public async Task InitializeAsync() + { + _ownerEmail = $"integration-test{Guid.NewGuid()}@bitwarden.com"; + await _factory.LoginWithNewAccount(_ownerEmail); + + (_organization, _) = await OrganizationTestHelpers.SignUpAsync(_factory, + plan: PlanType.EnterpriseAnnually, + ownerEmail: _ownerEmail, + passwordManagerSeats: 10, + paymentMethod: PaymentMethodType.Card); + + // The owner has no direct Manage access on the collection created below, so this is what + // authorizes them to change its metadata and other users' access to it. + _organization.AllowAdminAccessToAllCollectionItems = true; + await _factory.GetService().UpsertAsync(_organization); + + await _loginHelper.LoginAsync(_ownerEmail); + } + + public Task DisposeAsync() + { + _client.Dispose(); + return Task.CompletedTask; + } + + [Fact] + public async Task PatchWithDelta_UpdatesMetadataAndUserAccess_Success() + { + var (_, existingUser) = await OrganizationTestHelpers.CreateNewUserWithAccountAsync( + _factory, _organization.Id, OrganizationUserType.User); + var (_, newUser) = await OrganizationTestHelpers.CreateNewUserWithAccountAsync( + _factory, _organization.Id, OrganizationUserType.User); + + var collection = await OrganizationTestHelpers.CreateCollectionAsync( + _factory, + _organization.Id, + "Collection delta test", + externalId: "original-external", + users: [new CollectionAccessSelection { Id = existingUser.Id, ReadOnly = true }]); + + var model = new UpdateCollectionWithDeltaRequestModel + { + ExternalId = "updated-external", + Users = new CollectionUserAccessDeltaRequestModel + { + Add = [new SelectionReadOnlyRequestModel { Id = newUser.Id, ReadOnly = true }], + Update = [new SelectionReadOnlyRequestModel { Id = existingUser.Id, Manage = true }] + } + }; + + var response = await _client.PatchAsJsonAsync( + $"organizations/{_organization.Id}/collections/{collection.Id}", model); + + var body = await response.Content.ReadAsStringAsync(); + Assert.Equal(HttpStatusCode.NoContent, response.StatusCode); + + var (updatedCollection, accessDetails) = await _factory.GetService() + .GetByIdWithAccessAsync(collection.Id); + + Assert.NotNull(updatedCollection); + Assert.Equal("updated-external", updatedCollection.ExternalId); + + Assert.Equal(2, accessDetails.Users.Count()); + Assert.True(accessDetails.Users.Single(u => u.Id == existingUser.Id).Manage); + Assert.True(accessDetails.Users.Single(u => u.Id == newUser.Id).ReadOnly); + } +} diff --git a/test/Api.Test/AdminConsole/Controllers/CollectionsControllerTests.cs b/test/Api.Test/AdminConsole/Controllers/CollectionsControllerTests.cs index afa05ed06cd9..d839b811cfa8 100644 --- a/test/Api.Test/AdminConsole/Controllers/CollectionsControllerTests.cs +++ b/test/Api.Test/AdminConsole/Controllers/CollectionsControllerTests.cs @@ -6,7 +6,10 @@ using Bit.Api.Models.Request; using Bit.Core.AdminConsole.Entities; using Bit.Core.AdminConsole.OrganizationFeatures.Collections.Interfaces; +using Bit.Core.AdminConsole.OrganizationFeatures.Collections.ModifyGroupAccess; +using Bit.Core.AdminConsole.OrganizationFeatures.Collections.ModifyUserAccess; using Bit.Core.AdminConsole.Services; +using Bit.Core.AdminConsole.Utilities.v2.Results; using Bit.Core.Context; using Bit.Core.Entities; using Bit.Core.Exceptions; @@ -859,4 +862,131 @@ await sutProvider.GetDependency() .DidNotReceive() .LogProviderAccessToOrganizationAsync(Arg.Any()); } + + private static void AllowPatchWithDeltaAuthorization(SutProvider sutProvider, Guid orgId, Guid collectionId) + { + sutProvider.GetDependency() + .AuthorizeUpdateAsync(orgId, collectionId) + .Returns(true); + } + + private static CollectionAccessDetails MakeAccessDetails() => new() + { + Users = new List(), + Groups = new List() + }; + + [Theory, BitAutoData] + public async Task PatchWithDelta_AuthorizationFails_ThrowsNotFound( + Guid orgId, Guid collectionId, UpdateCollectionWithDeltaRequestModel model, + SutProvider sutProvider) + { + sutProvider.GetDependency() + .AuthorizeUpdateAsync(orgId, collectionId) + .Returns(false); + + await Assert.ThrowsAsync(() => + sutProvider.Sut.PatchWithDelta(orgId, collectionId, model)); + + await sutProvider.GetDependency().DidNotReceiveWithAnyArgs() + .UpdateAsync(default); + await sutProvider.GetDependency().DidNotReceiveWithAnyArgs() + .ModifyAsync(default); + await sutProvider.GetDependency().DidNotReceiveWithAnyArgs() + .ModifyAsync(default); + } + + [Theory, BitAutoData] + public async Task PatchWithDelta_AuthorizedButCollectionNotFound_ThrowsNotFound( + Guid orgId, Guid collectionId, UpdateCollectionWithDeltaRequestModel model, + SutProvider sutProvider) + { + // Defensive guard: the authorization service already fetched the collection and returned + // authorized, but the persistence-side fetch comes up empty (e.g. a delete raced in between). + AllowPatchWithDeltaAuthorization(sutProvider, orgId, collectionId); + sutProvider.GetDependency() + .GetByIdWithAccessAsync(collectionId) + .Returns(new Tuple(null, MakeAccessDetails())); + + await Assert.ThrowsAsync(() => + sutProvider.Sut.PatchWithDelta(orgId, collectionId, model)); + + await sutProvider.GetDependency().DidNotReceiveWithAnyArgs() + .UpdateAsync(default); + } + + [Theory, BitAutoData] + public async Task PatchWithDelta_AllChecksSucceed_UpdatesMetadataAndBothDeltas( + Guid orgId, Guid collectionId, Guid addUserId, Guid addGroupId, + SutProvider sutProvider) + { + var collection = new Collection + { + Id = collectionId, + OrganizationId = orgId, + Name = "original-name", + ExternalId = "original-external" + }; + var accessDetails = MakeAccessDetails(); + sutProvider.GetDependency() + .GetByIdWithAccessAsync(collectionId) + .Returns(new Tuple(collection, accessDetails)); + + AllowPatchWithDeltaAuthorization(sutProvider, orgId, collectionId); + + sutProvider.GetDependency() + .ModifyAsync(Arg.Any()) + .Returns(new CommandResult(new OneOf.Types.None())); + sutProvider.GetDependency() + .ModifyAsync(Arg.Any()) + .Returns(new CommandResult(new OneOf.Types.None())); + + var model = new UpdateCollectionWithDeltaRequestModel + { + ExternalId = "updated-external", + Users = new CollectionUserAccessDeltaRequestModel + { + Add = [new SelectionReadOnlyRequestModel { Id = addUserId, Manage = true }] + }, + Groups = new CollectionGroupAccessDeltaRequestModel + { + Add = [new SelectionReadOnlyRequestModel { Id = addGroupId, Manage = true }] + } + }; + + var result = await sutProvider.Sut.PatchWithDelta(orgId, collectionId, model); + + Assert.IsType(result); + await sutProvider.GetDependency().Received(1) + .UpdateAsync(Arg.Is(c => c.ExternalId == "updated-external")); + await sutProvider.GetDependency().Received(1) + .ModifyAsync(Arg.Is(r => r.Add.Any(s => s.Id == addUserId))); + await sutProvider.GetDependency().Received(1) + .ModifyAsync(Arg.Is(r => r.Add.Any(s => s.Id == addGroupId))); + } + + [Theory, BitAutoData] + public async Task PatchWithDelta_UserDeltaCommandFails_SkipsGroupDelta( + Guid orgId, Guid collectionId, UpdateCollectionWithDeltaRequestModel model, + SutProvider sutProvider) + { + var collection = new Collection { Id = collectionId, OrganizationId = orgId }; + var accessDetails = MakeAccessDetails(); + sutProvider.GetDependency() + .GetByIdWithAccessAsync(collectionId) + .Returns(new Tuple(collection, accessDetails)); + + AllowPatchWithDeltaAuthorization(sutProvider, orgId, collectionId); + sutProvider.GetDependency() + .ModifyAsync(Arg.Any()) + .Returns(new CommandResult(new DuplicateOrganizationUserId())); + + var result = await sutProvider.Sut.PatchWithDelta(orgId, collectionId, model); + + var badRequest = Assert.IsType>(result); + Assert.Equal(new DuplicateOrganizationUserId().Message, badRequest.Value!.Message); + + await sutProvider.GetDependency().DidNotReceiveWithAnyArgs() + .ModifyAsync(default); + } } diff --git a/test/Core.Test/AdminConsole/OrganizationFeatures/Collections/ModifyGroupAccess/ModifyCollectionGroupAccessCommandTests.cs b/test/Core.Test/AdminConsole/OrganizationFeatures/Collections/ModifyGroupAccess/ModifyCollectionGroupAccessCommandTests.cs new file mode 100644 index 000000000000..0f0ab5f9b6c7 --- /dev/null +++ b/test/Core.Test/AdminConsole/OrganizationFeatures/Collections/ModifyGroupAccess/ModifyCollectionGroupAccessCommandTests.cs @@ -0,0 +1,208 @@ +using Bit.Core.AdminConsole.OrganizationFeatures.Collections.ModifyGroupAccess; +using Bit.Core.AdminConsole.Utilities.v2.Validation; +using Bit.Core.Entities; +using Bit.Core.Enums; +using Bit.Core.Models.Data; +using Bit.Core.Repositories; +using Bit.Core.Services; +using Bit.Test.Common.AutoFixture; +using Bit.Test.Common.AutoFixture.Attributes; +using NSubstitute; +using Xunit; + +namespace Bit.Core.Test.AdminConsole.OrganizationFeatures.Collections.ModifyGroupAccess; + +[SutProviderCustomize] +public class ModifyCollectionGroupAccessCommandTests +{ + [Theory, BitAutoData] + public async Task ModifyAsync_ValidationFails_ReturnsErrorWithoutPersisting( + SutProvider sutProvider, + ModifyCollectionGroupAccessRequest request) + { + sutProvider.GetDependency().ValidateAsync(request) + .Returns(ValidationResultHelpers.Invalid(request, new DuplicateGroupId())); + + var result = await sutProvider.Sut.ModifyAsync(request); + + Assert.True(result.IsError); + Assert.IsType(result.AsError); + await sutProvider.GetDependency().DidNotReceiveWithAnyArgs() + .ModifyGroupAccessAsync(default, default, default, default, default); + await sutProvider.GetDependency().DidNotReceiveWithAnyArgs() + .LogCollectionEventsAsync(default); + } + + [Theory, BitAutoData] + public async Task ModifyAsync_AllEmpty_ReturnsSuccessWithoutValidatingOrPersisting( + SutProvider sutProvider, + Collection collection, + CollectionAccessDetails accessDetails) + { + var request = new ModifyCollectionGroupAccessRequest( + [new CollectionGroupAccessTarget(collection, accessDetails)], [], [], [], null, false); + + var result = await sutProvider.Sut.ModifyAsync(request); + + Assert.True(result.IsSuccess); + await sutProvider.GetDependency().DidNotReceiveWithAnyArgs() + .ValidateAsync(default); + await sutProvider.GetDependency().DidNotReceiveWithAnyArgs() + .ModifyGroupAccessAsync(default, default, default, default, default); + await sutProvider.GetDependency().DidNotReceiveWithAnyArgs() + .LogCollectionEventsAsync(default); + } + + [Theory, BitAutoData] + public async Task ModifyAsync_ValidRequest_UpsertsAddAndUpdateSelections( + SutProvider sutProvider, + Collection collection, + Guid addGroupId, + Guid updateGroupId) + { + var accessDetails = AccessDetails(updateGroupId); + var request = new ModifyCollectionGroupAccessRequest( + [new CollectionGroupAccessTarget(collection, accessDetails)], + [new CollectionAccessSelection { Id = addGroupId, Manage = true }], + [new CollectionAccessSelection { Id = updateGroupId, Manage = false }], + [], + null, + false); + + sutProvider.GetDependency().ValidateAsync(request) + .Returns(ValidationResultHelpers.Valid(request)); + + var result = await sutProvider.Sut.ModifyAsync(request); + + Assert.True(result.IsSuccess); + await sutProvider.GetDependency().Received(1).ModifyGroupAccessAsync( + collection.OrganizationId, + Arg.Is>(ids => ids.Single() == collection.Id), + Arg.Is>(selections => + selections.Any(s => s.Id == addGroupId) && selections.Any(s => s.Id == updateGroupId)), + Arg.Is>(ids => !ids.Any()), + Arg.Any()); + await sutProvider.GetDependency().Received(1).LogCollectionEventsAsync( + Arg.Is>(events => + events.Count() == 1 && events.Single().Item1 == collection + && events.Single().Item2 == EventType.Collection_Updated)); + } + + [Theory, BitAutoData] + public async Task ModifyAsync_ValidRequest_DeletesEachRemovedGroup( + SutProvider sutProvider, + Collection collection, + Guid removedGroupId1, + Guid removedGroupId2) + { + var accessDetails = AccessDetails(removedGroupId1, removedGroupId2); + var request = new ModifyCollectionGroupAccessRequest( + [new CollectionGroupAccessTarget(collection, accessDetails)], + [], [], [removedGroupId1, removedGroupId2], null, false); + + sutProvider.GetDependency().ValidateAsync(request) + .Returns(ValidationResultHelpers.Valid(request)); + + var result = await sutProvider.Sut.ModifyAsync(request); + + Assert.True(result.IsSuccess); + await sutProvider.GetDependency().Received(1).ModifyGroupAccessAsync( + collection.OrganizationId, + Arg.Is>(ids => ids.Single() == collection.Id), + Arg.Is>(selections => !selections.Any()), + Arg.Is>(ids => ids.Contains(removedGroupId1) && ids.Contains(removedGroupId2)), + Arg.Any()); + } + + [Theory, BitAutoData] + public async Task ModifyAsync_RemoveIdNotACollectionMember_FiltersItOutBeforePersisting( + SutProvider sutProvider, + Collection collection, + Guid actualMemberId, + Guid notAMemberId) + { + // notAMemberId is a valid id but was never granted access to this collection. It has to be dropped, + // not forwarded to the repository, or it would bump an unrelated group's revision date for no reason. + var accessDetails = AccessDetails(actualMemberId); + var request = new ModifyCollectionGroupAccessRequest( + [new CollectionGroupAccessTarget(collection, accessDetails)], + [], [], [actualMemberId, notAMemberId], null, false); + + sutProvider.GetDependency().ValidateAsync(request) + .Returns(ValidationResultHelpers.Valid(request)); + + var result = await sutProvider.Sut.ModifyAsync(request); + + Assert.True(result.IsSuccess); + await sutProvider.GetDependency().Received(1).ModifyGroupAccessAsync( + collection.OrganizationId, + Arg.Any>(), + Arg.Any>(), + Arg.Is>(ids => ids.Contains(actualMemberId) && !ids.Contains(notAMemberId)), + Arg.Any()); + } + + [Theory, BitAutoData] + public async Task ModifyAsync_ValidRequest_UpsertsAndRemovesInOneAtomicCall( + SutProvider sutProvider, + Collection collection, + Guid addGroupId, + Guid removedGroupId) + { + var accessDetails = AccessDetails(removedGroupId); + var request = new ModifyCollectionGroupAccessRequest( + [new CollectionGroupAccessTarget(collection, accessDetails)], + [new CollectionAccessSelection { Id = addGroupId, Manage = true }], [], [removedGroupId], null, false); + + sutProvider.GetDependency().ValidateAsync(request) + .Returns(ValidationResultHelpers.Valid(request)); + + var result = await sutProvider.Sut.ModifyAsync(request); + + Assert.True(result.IsSuccess); + // Regression test: upserts and removes must go through one repository call, not two independently-failable ones. + await sutProvider.GetDependency().Received(1).ModifyGroupAccessAsync( + collection.OrganizationId, + Arg.Is>(ids => ids.Single() == collection.Id), + Arg.Is>(selections => selections.Any(s => s.Id == addGroupId)), + Arg.Is>(ids => ids.Contains(removedGroupId)), + Arg.Any()); + } + + [Theory, BitAutoData] + public async Task ModifyAsync_MultipleTargets_AppliesSameDeltaToAllInOneCall( + SutProvider sutProvider, + Collection collectionA, + Collection collectionB, + Guid addGroupId) + { + var targets = new[] + { + new CollectionGroupAccessTarget(collectionA, AccessDetails()), + new CollectionGroupAccessTarget(collectionB, AccessDetails()) + }; + var request = new ModifyCollectionGroupAccessRequest( + targets, [new CollectionAccessSelection { Id = addGroupId, Manage = true }], [], [], null, false); + + sutProvider.GetDependency().ValidateAsync(request) + .Returns(ValidationResultHelpers.Valid(request)); + + var result = await sutProvider.Sut.ModifyAsync(request); + + Assert.True(result.IsSuccess); + await sutProvider.GetDependency().Received(1).ModifyGroupAccessAsync( + collectionA.OrganizationId, + Arg.Is>(ids => ids.Contains(collectionA.Id) && ids.Contains(collectionB.Id)), + Arg.Is>(selections => selections.Any(s => s.Id == addGroupId)), + Arg.Any>(), + Arg.Any()); + await sutProvider.GetDependency().Received(1).LogCollectionEventsAsync( + Arg.Is>(events => events.Count() == 2)); + } + + private static CollectionAccessDetails AccessDetails(params Guid[] existingMemberIds) => new() + { + Users = [], + Groups = existingMemberIds.Select(id => new CollectionAccessSelection { Id = id }).ToList() + }; +} diff --git a/test/Core.Test/AdminConsole/OrganizationFeatures/Collections/ModifyGroupAccess/ModifyCollectionGroupAccessValidatorTests.cs b/test/Core.Test/AdminConsole/OrganizationFeatures/Collections/ModifyGroupAccess/ModifyCollectionGroupAccessValidatorTests.cs new file mode 100644 index 000000000000..2892ab5c1e24 --- /dev/null +++ b/test/Core.Test/AdminConsole/OrganizationFeatures/Collections/ModifyGroupAccess/ModifyCollectionGroupAccessValidatorTests.cs @@ -0,0 +1,504 @@ +using Bit.Core.AdminConsole.Entities; +using Bit.Core.AdminConsole.OrganizationFeatures.Collections.ModifyGroupAccess; +using Bit.Core.AdminConsole.Repositories; +using Bit.Core.Entities; +using Bit.Core.Enums; +using Bit.Core.Models.Data; +using Bit.Test.Common.AutoFixture; +using Bit.Test.Common.AutoFixture.Attributes; +using NSubstitute; +using Xunit; + +namespace Bit.Core.Test.AdminConsole.OrganizationFeatures.Collections.ModifyGroupAccess; + +[SutProviderCustomize] +public class ModifyCollectionGroupAccessValidatorTests +{ + [Theory, BitAutoData] + public async Task ValidateAsync_AllEmpty_Succeeds( + SutProvider sutProvider, Guid existingManagerId) + { + // The command short-circuits on empty deltas, but the validator must still behave correctly if called directly. + var target = new CollectionGroupAccessTarget( + new Collection { Id = Guid.NewGuid(), OrganizationId = Guid.NewGuid() }, + new CollectionAccessDetails + { + Users = [], + Groups = [new CollectionAccessSelection { Id = existingManagerId, Manage = true }] + }); + var request = new ModifyCollectionGroupAccessRequest([target], [], [], [], null, false); + + var result = await sutProvider.Sut.ValidateAsync(request); + + Assert.True(result.IsValid); + } + + [Theory, BitAutoData] + public async Task ValidateAsync_DuplicateIdWithinAdd_ReturnsError( + SutProvider sutProvider, Guid newGroupId) + { + var target = new CollectionGroupAccessTarget( + new Collection { Id = Guid.NewGuid(), OrganizationId = Guid.NewGuid() }, + new CollectionAccessDetails { Users = [], Groups = [] }); + var add = new[] + { + new CollectionAccessSelection { Id = newGroupId }, + new CollectionAccessSelection { Id = newGroupId, Manage = true } + }; + var request = new ModifyCollectionGroupAccessRequest([target], add, [], [], null, false); + + var result = await sutProvider.Sut.ValidateAsync(request); + + Assert.True(result.IsError); + Assert.IsType(result.AsError); + } + + [Theory, BitAutoData] + public async Task ValidateAsync_DuplicateIdWithinUpdate_ReturnsError( + SutProvider sutProvider, Guid existingGroupId) + { + var target = new CollectionGroupAccessTarget( + new Collection { Id = Guid.NewGuid(), OrganizationId = Guid.NewGuid() }, + new CollectionAccessDetails + { + Users = [], + Groups = [new CollectionAccessSelection { Id = existingGroupId }] + }); + var update = new[] + { + new CollectionAccessSelection { Id = existingGroupId }, + new CollectionAccessSelection { Id = existingGroupId, Manage = true } + }; + var request = new ModifyCollectionGroupAccessRequest([target], [], update, [], null, false); + + var result = await sutProvider.Sut.ValidateAsync(request); + + Assert.True(result.IsError); + Assert.IsType(result.AsError); + } + + [Theory, BitAutoData] + public async Task ValidateAsync_IdInBothRemoveAndUpdate_ReturnsError( + SutProvider sutProvider, Guid conflictingGroupId) + { + var target = new CollectionGroupAccessTarget( + new Collection { Id = Guid.NewGuid(), OrganizationId = Guid.NewGuid() }, + new CollectionAccessDetails + { + Users = [], + Groups = [new CollectionAccessSelection { Id = conflictingGroupId }] + }); + var request = new ModifyCollectionGroupAccessRequest( + [target], [], [new CollectionAccessSelection { Id = conflictingGroupId }], [conflictingGroupId], null, false); + + var result = await sutProvider.Sut.ValidateAsync(request); + + Assert.True(result.IsError); + Assert.IsType(result.AsError); + } + + [Theory, BitAutoData] + public async Task ValidateAsync_AddManageWithReadOnlyOrHidePasswords_ReturnsError( + SutProvider sutProvider, Guid readOnlyGroupId, Guid hidePasswordsGroupId) + { + var target = new CollectionGroupAccessTarget( + new Collection { Id = Guid.NewGuid(), OrganizationId = Guid.NewGuid() }, + new CollectionAccessDetails { Users = [], Groups = [] }); + + var readOnlyRequest = new ModifyCollectionGroupAccessRequest( + [target], [new CollectionAccessSelection { Id = readOnlyGroupId, Manage = true, ReadOnly = true }], [], [], null, false); + var readOnlyResult = await sutProvider.Sut.ValidateAsync(readOnlyRequest); + Assert.True(readOnlyResult.IsError); + Assert.IsType(readOnlyResult.AsError); + + var hidePasswordsRequest = new ModifyCollectionGroupAccessRequest( + [target], [new CollectionAccessSelection { Id = hidePasswordsGroupId, Manage = true, HidePasswords = true }], [], [], null, false); + var hidePasswordsResult = await sutProvider.Sut.ValidateAsync(hidePasswordsRequest); + Assert.True(hidePasswordsResult.IsError); + Assert.IsType(hidePasswordsResult.AsError); + } + + [Theory, BitAutoData] + public async Task ValidateAsync_UpdateManageWithReadOnlyOrHidePasswords_ReturnsError( + SutProvider sutProvider, Guid existingGroupId) + { + var target = new CollectionGroupAccessTarget( + new Collection { Id = Guid.NewGuid(), OrganizationId = Guid.NewGuid() }, + new CollectionAccessDetails + { + Users = [], + Groups = [new CollectionAccessSelection { Id = existingGroupId }] + }); + + var readOnlyRequest = new ModifyCollectionGroupAccessRequest( + [target], [], [new CollectionAccessSelection { Id = existingGroupId, Manage = true, ReadOnly = true }], [], null, false); + var readOnlyResult = await sutProvider.Sut.ValidateAsync(readOnlyRequest); + Assert.True(readOnlyResult.IsError); + Assert.IsType(readOnlyResult.AsError); + + var hidePasswordsRequest = new ModifyCollectionGroupAccessRequest( + [target], [], [new CollectionAccessSelection { Id = existingGroupId, Manage = true, HidePasswords = true }], [], null, false); + var hidePasswordsResult = await sutProvider.Sut.ValidateAsync(hidePasswordsRequest); + Assert.True(hidePasswordsResult.IsError); + Assert.IsType(hidePasswordsResult.AsError); + } + + [Theory, BitAutoData] + public async Task ValidateAsync_DefaultUserCollection_ReturnsError( + SutProvider sutProvider, Guid newGroupId) + { + var target = new CollectionGroupAccessTarget( + new Collection { Id = Guid.NewGuid(), OrganizationId = Guid.NewGuid(), Type = CollectionType.DefaultUserCollection }, + new CollectionAccessDetails { Users = [], Groups = [] }); + var request = new ModifyCollectionGroupAccessRequest( + [target], [new CollectionAccessSelection { Id = newGroupId }], [], [], null, false); + + var result = await sutProvider.Sut.ValidateAsync(request); + + Assert.True(result.IsError); + Assert.IsType(result.AsError); + } + + [Theory, BitAutoData] + public async Task ValidateAsync_AddIdAlreadyExistingMember_ReturnsError( + SutProvider sutProvider, Guid existingGroupId) + { + var target = new CollectionGroupAccessTarget( + new Collection { Id = Guid.NewGuid(), OrganizationId = Guid.NewGuid() }, + new CollectionAccessDetails + { + Users = [], + Groups = [new CollectionAccessSelection { Id = existingGroupId }] + }); + var request = new ModifyCollectionGroupAccessRequest( + [target], [new CollectionAccessSelection { Id = existingGroupId }], [], [], null, false); + + var result = await sutProvider.Sut.ValidateAsync(request); + + Assert.True(result.IsError); + Assert.IsType(result.AsError); + } + + [Theory, BitAutoData] + public async Task ValidateAsync_UpdateIdNotExistingMember_ReturnsError( + SutProvider sutProvider, Guid nonMemberGroupId) + { + var target = new CollectionGroupAccessTarget( + new Collection { Id = Guid.NewGuid(), OrganizationId = Guid.NewGuid() }, + new CollectionAccessDetails { Users = [], Groups = [] }); + var request = new ModifyCollectionGroupAccessRequest( + [target], [], [new CollectionAccessSelection { Id = nonMemberGroupId }], [], null, false); + + var result = await sutProvider.Sut.ValidateAsync(request); + + Assert.True(result.IsError); + Assert.IsType(result.AsError); + } + + [Theory, BitAutoData] + public async Task ValidateAsync_AddTargetDoesNotExist_ReturnsError( + SutProvider sutProvider, Guid newGroupId) + { + var target = new CollectionGroupAccessTarget( + new Collection { Id = Guid.NewGuid(), OrganizationId = Guid.NewGuid() }, + new CollectionAccessDetails { Users = [], Groups = [] }); + var request = new ModifyCollectionGroupAccessRequest( + [target], [new CollectionAccessSelection { Id = newGroupId, Manage = true }], [], [], null, false); + + sutProvider.GetDependency() + .GetManyByManyIds(Arg.Any>()) + .Returns(new List()); + + var result = await sutProvider.Sut.ValidateAsync(request); + + Assert.True(result.IsError); + Assert.IsType(result.AsError); + } + + [Theory, BitAutoData] + public async Task ValidateAsync_AddTargetBelongsToDifferentOrganization_ReturnsError( + SutProvider sutProvider, Guid newGroupId, Guid otherOrganizationId) + { + var target = new CollectionGroupAccessTarget( + new Collection { Id = Guid.NewGuid(), OrganizationId = Guid.NewGuid() }, + new CollectionAccessDetails { Users = [], Groups = [] }); + var request = new ModifyCollectionGroupAccessRequest( + [target], [new CollectionAccessSelection { Id = newGroupId, Manage = true }], [], [], null, false); + + sutProvider.GetDependency() + .GetManyByManyIds(Arg.Any>()) + .Returns(new List { new() { Id = newGroupId, OrganizationId = otherOrganizationId } }); + + var result = await sutProvider.Sut.ValidateAsync(request); + + Assert.True(result.IsError); + Assert.IsType(result.AsError); + } + + [Theory, BitAutoData] + public async Task ValidateAsync_RemovingLastManager_WithoutAllowAdminAccess_ReturnsError( + SutProvider sutProvider, Guid managerGroupId, Guid organizationId) + { + var target = new CollectionGroupAccessTarget( + new Collection { Id = Guid.NewGuid(), OrganizationId = organizationId }, + new CollectionAccessDetails + { + Users = [], + Groups = [new CollectionAccessSelection { Id = managerGroupId, Manage = true }] + }); + ArrangeValidGroups(sutProvider, organizationId); + var request = new ModifyCollectionGroupAccessRequest([target], [], [], [managerGroupId], null, false); + + var result = await sutProvider.Sut.ValidateAsync(request); + + Assert.True(result.IsError); + Assert.IsType(result.AsError); + } + + [Theory, BitAutoData] + public async Task ValidateAsync_RemovingLastManager_WithAllowAdminAccess_Succeeds( + SutProvider sutProvider, Guid managerGroupId, Guid organizationId) + { + var target = new CollectionGroupAccessTarget( + new Collection { Id = Guid.NewGuid(), OrganizationId = organizationId }, + new CollectionAccessDetails + { + Users = [], + Groups = [new CollectionAccessSelection { Id = managerGroupId, Manage = true }] + }); + ArrangeValidGroups(sutProvider, organizationId); + var request = new ModifyCollectionGroupAccessRequest([target], [], [], [managerGroupId], null, true); + + var result = await sutProvider.Sut.ValidateAsync(request); + + Assert.True(result.IsValid); + } + + [Theory, BitAutoData] + public async Task ValidateAsync_RemovingOnlyManagingGroup_ButUserStillManages_Succeeds( + SutProvider sutProvider, Guid managerGroupId, Guid userId, Guid organizationId) + { + var target = new CollectionGroupAccessTarget( + new Collection { Id = Guid.NewGuid(), OrganizationId = organizationId }, + new CollectionAccessDetails + { + Users = [new CollectionAccessSelection { Id = userId, Manage = true }], + Groups = [new CollectionAccessSelection { Id = managerGroupId, Manage = true }] + }); + ArrangeValidGroups(sutProvider, organizationId); + var request = new ModifyCollectionGroupAccessRequest([target], [], [], [managerGroupId], null, false); + + var result = await sutProvider.Sut.ValidateAsync(request); + + Assert.True(result.IsValid); + } + + [Theory, BitAutoData] + public async Task ValidateAsync_UpdatingOnlyManagerToNonManage_NoOtherManager_ReturnsError( + SutProvider sutProvider, Guid managerGroupId, Guid organizationId) + { + var target = new CollectionGroupAccessTarget( + new Collection { Id = Guid.NewGuid(), OrganizationId = organizationId }, + new CollectionAccessDetails + { + Users = [], + Groups = [new CollectionAccessSelection { Id = managerGroupId, Manage = true }] + }); + ArrangeValidGroups(sutProvider, organizationId); + var request = new ModifyCollectionGroupAccessRequest( + [target], [], [new CollectionAccessSelection { Id = managerGroupId, Manage = false }], [], null, false); + + var result = await sutProvider.Sut.ValidateAsync(request); + + Assert.True(result.IsError); + Assert.IsType(result.AsError); + } + + [Theory, BitAutoData] + public async Task ValidateAsync_AddingNewManagerToOrphanedCollection_Succeeds( + SutProvider sutProvider, Guid newGroupId, Guid organizationId) + { + var target = new CollectionGroupAccessTarget( + new Collection { Id = Guid.NewGuid(), OrganizationId = organizationId }, + new CollectionAccessDetails { Users = [], Groups = [] }); + ArrangeValidGroups(sutProvider, organizationId); + var request = new ModifyCollectionGroupAccessRequest( + [target], [new CollectionAccessSelection { Id = newGroupId, Manage = true }], [], [], null, false); + + var result = await sutProvider.Sut.ValidateAsync(request); + + Assert.True(result.IsValid); + } + + [Theory, BitAutoData] + public async Task ValidateAsync_ValidDelta_Succeeds( + SutProvider sutProvider, + Guid existingGroupId, Guid newGroupId, Guid removedGroupId, Guid organizationId) + { + var target = new CollectionGroupAccessTarget( + new Collection { Id = Guid.NewGuid(), OrganizationId = organizationId }, + new CollectionAccessDetails + { + Users = [], + Groups = + [ + new CollectionAccessSelection { Id = existingGroupId }, + new CollectionAccessSelection { Id = removedGroupId } + ] + }); + ArrangeValidGroups(sutProvider, organizationId); + var request = new ModifyCollectionGroupAccessRequest( + [target], + [new CollectionAccessSelection { Id = newGroupId }], + [new CollectionAccessSelection { Id = existingGroupId, Manage = true }], + [removedGroupId], + null, false); + + var result = await sutProvider.Sut.ValidateAsync(request); + + Assert.True(result.IsValid); + } + + [Theory, BitAutoData] + public async Task ValidateAsync_MultipleTargets_AnyDefaultUserCollection_ReturnsError( + SutProvider sutProvider, Guid newGroupId) + { + var targets = new[] + { + new CollectionGroupAccessTarget( + new Collection { Id = Guid.NewGuid(), OrganizationId = Guid.NewGuid() }, + new CollectionAccessDetails { Users = [], Groups = [] }), + new CollectionGroupAccessTarget( + new Collection { Id = Guid.NewGuid(), OrganizationId = Guid.NewGuid(), Type = CollectionType.DefaultUserCollection }, + new CollectionAccessDetails { Users = [], Groups = [] }) + }; + var request = new ModifyCollectionGroupAccessRequest( + targets, [new CollectionAccessSelection { Id = newGroupId }], [], [], null, false); + + var result = await sutProvider.Sut.ValidateAsync(request); + + Assert.True(result.IsError); + Assert.IsType(result.AsError); + } + + [Theory, BitAutoData] + public async Task ValidateAsync_MultipleTargets_AlreadyMemberOfOneNotTheOther_Succeeds( + SutProvider sutProvider, Guid existingGroupId, Guid organizationId) + { + // The Add-must-be-new check only applies to a single collection. Across multiple targets the same + // group can already have access to one and not another, so we don't check it here. + var targets = new[] + { + new CollectionGroupAccessTarget( + new Collection { Id = Guid.NewGuid(), OrganizationId = organizationId }, + new CollectionAccessDetails + { + Users = [], + Groups = [new CollectionAccessSelection { Id = existingGroupId }] + }), + new CollectionGroupAccessTarget( + new Collection { Id = Guid.NewGuid(), OrganizationId = organizationId }, + new CollectionAccessDetails { Users = [], Groups = [] }) + }; + ArrangeValidGroups(sutProvider, organizationId); + var request = new ModifyCollectionGroupAccessRequest( + targets, [new CollectionAccessSelection { Id = existingGroupId, Manage = true }], [], [], null, false); + + var result = await sutProvider.Sut.ValidateAsync(request); + + Assert.True(result.IsValid); + } + + [Theory, BitAutoData] + public async Task ValidateAsync_MultipleTargets_RemovingOnlyManagerOfOneTarget_ReturnsError( + SutProvider sutProvider, + Guid managerGroupId, Guid otherManagerGroupId, Guid organizationId) + { + var targets = new[] + { + new CollectionGroupAccessTarget( + new Collection { Id = Guid.NewGuid(), OrganizationId = organizationId }, + new CollectionAccessDetails + { + Users = [], + Groups = [new CollectionAccessSelection { Id = managerGroupId, Manage = true }] + }), + new CollectionGroupAccessTarget( + new Collection { Id = Guid.NewGuid(), OrganizationId = organizationId }, + new CollectionAccessDetails + { + Users = [], + Groups = [new CollectionAccessSelection { Id = otherManagerGroupId, Manage = true }] + }) + }; + ArrangeValidGroups(sutProvider, organizationId); + var request = new ModifyCollectionGroupAccessRequest(targets, [], [], [managerGroupId], null, false); + + var result = await sutProvider.Sut.ValidateAsync(request); + + Assert.True(result.IsError); + Assert.IsType(result.AsError); + } + + [Theory, BitAutoData] + public async Task ValidateAsync_MultipleTargets_ValidDelta_Succeeds( + SutProvider sutProvider, Guid newGroupId, Guid organizationId) + { + var targets = new[] + { + new CollectionGroupAccessTarget( + new Collection { Id = Guid.NewGuid(), OrganizationId = organizationId }, + new CollectionAccessDetails { Users = [], Groups = [] }), + new CollectionGroupAccessTarget( + new Collection { Id = Guid.NewGuid(), OrganizationId = organizationId }, + new CollectionAccessDetails { Users = [], Groups = [] }) + }; + ArrangeValidGroups(sutProvider, organizationId); + var request = new ModifyCollectionGroupAccessRequest( + targets, [new CollectionAccessSelection { Id = newGroupId, Manage = true }], [], [], null, false); + + var result = await sutProvider.Sut.ValidateAsync(request); + + Assert.True(result.IsValid); + } + + [Theory, BitAutoData] + public async Task ValidateAsync_MultipleTargets_UpdateGrantsNewManagerOnOtherTarget_Succeeds( + SutProvider sutProvider, Guid managerGroupId, Guid organizationId) + { + // Regression test. An Update entry for a group that isn't yet a member of a target still upserts onto + // that target, since the same delta applies to every collection. It has to count toward that target's + // remaining manage access, or a valid request gets rejected as leaving the second target unmanaged. + var targets = new[] + { + new CollectionGroupAccessTarget( + new Collection { Id = Guid.NewGuid(), OrganizationId = organizationId }, + new CollectionAccessDetails + { + Users = [], + Groups = [new CollectionAccessSelection { Id = managerGroupId, Manage = true }] + }), + new CollectionGroupAccessTarget( + new Collection { Id = Guid.NewGuid(), OrganizationId = organizationId }, + new CollectionAccessDetails { Users = [], Groups = [] }) + }; + ArrangeValidGroups(sutProvider, organizationId); + var request = new ModifyCollectionGroupAccessRequest( + targets, [], [new CollectionAccessSelection { Id = managerGroupId, Manage = true }], [], null, false); + + var result = await sutProvider.Sut.ValidateAsync(request); + + Assert.True(result.IsValid); + } + + // Any id passed to GetManyByManyIds resolves as a valid group in the given org, unless overridden. + private static void ArrangeValidGroups( + SutProvider sutProvider, Guid organizationId) + { + sutProvider.GetDependency() + .GetManyByManyIds(Arg.Any>()) + .Returns(callInfo => callInfo.Arg>() + .Select(id => new Group { Id = id, OrganizationId = organizationId }) + .ToList()); + } +} diff --git a/test/Core.Test/AdminConsole/OrganizationFeatures/Collections/ModifyUserAccess/ModifyCollectionUserAccessCommandTests.cs b/test/Core.Test/AdminConsole/OrganizationFeatures/Collections/ModifyUserAccess/ModifyCollectionUserAccessCommandTests.cs new file mode 100644 index 000000000000..c9ca1f51b793 --- /dev/null +++ b/test/Core.Test/AdminConsole/OrganizationFeatures/Collections/ModifyUserAccess/ModifyCollectionUserAccessCommandTests.cs @@ -0,0 +1,208 @@ +using Bit.Core.AdminConsole.OrganizationFeatures.Collections.ModifyUserAccess; +using Bit.Core.AdminConsole.Utilities.v2.Validation; +using Bit.Core.Entities; +using Bit.Core.Enums; +using Bit.Core.Models.Data; +using Bit.Core.Repositories; +using Bit.Core.Services; +using Bit.Test.Common.AutoFixture; +using Bit.Test.Common.AutoFixture.Attributes; +using NSubstitute; +using Xunit; + +namespace Bit.Core.Test.AdminConsole.OrganizationFeatures.Collections.ModifyUserAccess; + +[SutProviderCustomize] +public class ModifyCollectionUserAccessCommandTests +{ + [Theory, BitAutoData] + public async Task ModifyAsync_ValidationFails_ReturnsErrorWithoutPersisting( + SutProvider sutProvider, + ModifyCollectionUserAccessRequest request) + { + sutProvider.GetDependency().ValidateAsync(request) + .Returns(ValidationResultHelpers.Invalid(request, new DuplicateOrganizationUserId())); + + var result = await sutProvider.Sut.ModifyAsync(request); + + Assert.True(result.IsError); + Assert.IsType(result.AsError); + await sutProvider.GetDependency().DidNotReceiveWithAnyArgs() + .ModifyUserAccessAsync(default, default, default, default, default); + await sutProvider.GetDependency().DidNotReceiveWithAnyArgs() + .LogCollectionEventsAsync(default); + } + + [Theory, BitAutoData] + public async Task ModifyAsync_AllEmpty_ReturnsSuccessWithoutValidatingOrPersisting( + SutProvider sutProvider, + Collection collection, + CollectionAccessDetails accessDetails) + { + var request = new ModifyCollectionUserAccessRequest( + [new CollectionUserAccessTarget(collection, accessDetails)], [], [], [], null, false); + + var result = await sutProvider.Sut.ModifyAsync(request); + + Assert.True(result.IsSuccess); + await sutProvider.GetDependency().DidNotReceiveWithAnyArgs() + .ValidateAsync(default); + await sutProvider.GetDependency().DidNotReceiveWithAnyArgs() + .ModifyUserAccessAsync(default, default, default, default, default); + await sutProvider.GetDependency().DidNotReceiveWithAnyArgs() + .LogCollectionEventsAsync(default); + } + + [Theory, BitAutoData] + public async Task ModifyAsync_ValidRequest_UpsertsAddAndUpdateSelections( + SutProvider sutProvider, + Collection collection, + Guid addUserId, + Guid updateUserId) + { + var accessDetails = AccessDetails(updateUserId); + var request = new ModifyCollectionUserAccessRequest( + [new CollectionUserAccessTarget(collection, accessDetails)], + [new CollectionAccessSelection { Id = addUserId, Manage = true }], + [new CollectionAccessSelection { Id = updateUserId, Manage = false }], + [], + null, + false); + + sutProvider.GetDependency().ValidateAsync(request) + .Returns(ValidationResultHelpers.Valid(request)); + + var result = await sutProvider.Sut.ModifyAsync(request); + + Assert.True(result.IsSuccess); + await sutProvider.GetDependency().Received(1).ModifyUserAccessAsync( + collection.OrganizationId, + Arg.Is>(ids => ids.Single() == collection.Id), + Arg.Is>(selections => + selections.Any(s => s.Id == addUserId) && selections.Any(s => s.Id == updateUserId)), + Arg.Is>(ids => !ids.Any()), + Arg.Any()); + await sutProvider.GetDependency().Received(1).LogCollectionEventsAsync( + Arg.Is>(events => + events.Count() == 1 && events.Single().Item1 == collection + && events.Single().Item2 == EventType.Collection_Updated)); + } + + [Theory, BitAutoData] + public async Task ModifyAsync_ValidRequest_DeletesEachRemovedUser( + SutProvider sutProvider, + Collection collection, + Guid removedUserId1, + Guid removedUserId2) + { + var accessDetails = AccessDetails(removedUserId1, removedUserId2); + var request = new ModifyCollectionUserAccessRequest( + [new CollectionUserAccessTarget(collection, accessDetails)], + [], [], [removedUserId1, removedUserId2], null, false); + + sutProvider.GetDependency().ValidateAsync(request) + .Returns(ValidationResultHelpers.Valid(request)); + + var result = await sutProvider.Sut.ModifyAsync(request); + + Assert.True(result.IsSuccess); + await sutProvider.GetDependency().Received(1).ModifyUserAccessAsync( + collection.OrganizationId, + Arg.Is>(ids => ids.Single() == collection.Id), + Arg.Is>(selections => !selections.Any()), + Arg.Is>(ids => ids.Contains(removedUserId1) && ids.Contains(removedUserId2)), + Arg.Any()); + } + + [Theory, BitAutoData] + public async Task ModifyAsync_RemoveIdNotACollectionMember_FiltersItOutBeforePersisting( + SutProvider sutProvider, + Collection collection, + Guid actualMemberId, + Guid notAMemberId) + { + // notAMemberId is a valid id but was never granted access to this collection. It has to be dropped, + // not forwarded to the repository, or it would bump an unrelated user's AccountRevisionDate for no reason. + var accessDetails = AccessDetails(actualMemberId); + var request = new ModifyCollectionUserAccessRequest( + [new CollectionUserAccessTarget(collection, accessDetails)], + [], [], [actualMemberId, notAMemberId], null, false); + + sutProvider.GetDependency().ValidateAsync(request) + .Returns(ValidationResultHelpers.Valid(request)); + + var result = await sutProvider.Sut.ModifyAsync(request); + + Assert.True(result.IsSuccess); + await sutProvider.GetDependency().Received(1).ModifyUserAccessAsync( + collection.OrganizationId, + Arg.Any>(), + Arg.Any>(), + Arg.Is>(ids => ids.Contains(actualMemberId) && !ids.Contains(notAMemberId)), + Arg.Any()); + } + + [Theory, BitAutoData] + public async Task ModifyAsync_ValidRequest_UpsertsAndRemovesInOneAtomicCall( + SutProvider sutProvider, + Collection collection, + Guid addUserId, + Guid removedUserId) + { + var accessDetails = AccessDetails(removedUserId); + var request = new ModifyCollectionUserAccessRequest( + [new CollectionUserAccessTarget(collection, accessDetails)], + [new CollectionAccessSelection { Id = addUserId, Manage = true }], [], [removedUserId], null, false); + + sutProvider.GetDependency().ValidateAsync(request) + .Returns(ValidationResultHelpers.Valid(request)); + + var result = await sutProvider.Sut.ModifyAsync(request); + + Assert.True(result.IsSuccess); + // Regression test: upserts and removes must go through one repository call, not two independently-failable ones. + await sutProvider.GetDependency().Received(1).ModifyUserAccessAsync( + collection.OrganizationId, + Arg.Is>(ids => ids.Single() == collection.Id), + Arg.Is>(selections => selections.Any(s => s.Id == addUserId)), + Arg.Is>(ids => ids.Contains(removedUserId)), + Arg.Any()); + } + + [Theory, BitAutoData] + public async Task ModifyAsync_MultipleTargets_AppliesSameDeltaToAllInOneCall( + SutProvider sutProvider, + Collection collectionA, + Collection collectionB, + Guid addUserId) + { + var targets = new[] + { + new CollectionUserAccessTarget(collectionA, AccessDetails()), + new CollectionUserAccessTarget(collectionB, AccessDetails()) + }; + var request = new ModifyCollectionUserAccessRequest( + targets, [new CollectionAccessSelection { Id = addUserId, Manage = true }], [], [], null, false); + + sutProvider.GetDependency().ValidateAsync(request) + .Returns(ValidationResultHelpers.Valid(request)); + + var result = await sutProvider.Sut.ModifyAsync(request); + + Assert.True(result.IsSuccess); + await sutProvider.GetDependency().Received(1).ModifyUserAccessAsync( + collectionA.OrganizationId, + Arg.Is>(ids => ids.Contains(collectionA.Id) && ids.Contains(collectionB.Id)), + Arg.Is>(selections => selections.Any(s => s.Id == addUserId)), + Arg.Any>(), + Arg.Any()); + await sutProvider.GetDependency().Received(1).LogCollectionEventsAsync( + Arg.Is>(events => events.Count() == 2)); + } + + private static CollectionAccessDetails AccessDetails(params Guid[] existingMemberIds) => new() + { + Users = existingMemberIds.Select(id => new CollectionAccessSelection { Id = id }).ToList(), + Groups = [] + }; +} diff --git a/test/Core.Test/AdminConsole/OrganizationFeatures/Collections/ModifyUserAccess/ModifyCollectionUserAccessValidatorTests.cs b/test/Core.Test/AdminConsole/OrganizationFeatures/Collections/ModifyUserAccess/ModifyCollectionUserAccessValidatorTests.cs new file mode 100644 index 000000000000..da2191c7e0ce --- /dev/null +++ b/test/Core.Test/AdminConsole/OrganizationFeatures/Collections/ModifyUserAccess/ModifyCollectionUserAccessValidatorTests.cs @@ -0,0 +1,613 @@ +using Bit.Core.AdminConsole.OrganizationFeatures.Collections.ModifyUserAccess; +using Bit.Core.Entities; +using Bit.Core.Enums; +using Bit.Core.Models.Data; +using Bit.Core.Repositories; +using Bit.Test.Common.AutoFixture; +using Bit.Test.Common.AutoFixture.Attributes; +using NSubstitute; +using Xunit; + +namespace Bit.Core.Test.AdminConsole.OrganizationFeatures.Collections.ModifyUserAccess; + +[SutProviderCustomize] +public class ModifyCollectionUserAccessValidatorTests +{ + [Theory, BitAutoData] + public async Task ValidateAsync_AllEmpty_Succeeds( + SutProvider sutProvider, Guid existingManagerId) + { + // The command short-circuits on empty deltas, but the validator must still behave correctly if called directly. + var target = new CollectionUserAccessTarget( + new Collection { Id = Guid.NewGuid(), OrganizationId = Guid.NewGuid() }, + new CollectionAccessDetails + { + Users = [new CollectionAccessSelection { Id = existingManagerId, Manage = true }], + Groups = [] + }); + var request = new ModifyCollectionUserAccessRequest([target], [], [], [], null, false); + + var result = await sutProvider.Sut.ValidateAsync(request); + + Assert.True(result.IsValid); + } + + [Theory, BitAutoData] + public async Task ValidateAsync_DuplicateIdWithinAdd_ReturnsError( + SutProvider sutProvider, Guid newUserId) + { + var target = new CollectionUserAccessTarget( + new Collection { Id = Guid.NewGuid(), OrganizationId = Guid.NewGuid() }, + new CollectionAccessDetails { Users = [], Groups = [] }); + var add = new[] + { + new CollectionAccessSelection { Id = newUserId }, + new CollectionAccessSelection { Id = newUserId, Manage = true } + }; + var request = new ModifyCollectionUserAccessRequest([target], add, [], [], null, false); + + var result = await sutProvider.Sut.ValidateAsync(request); + + Assert.True(result.IsError); + Assert.IsType(result.AsError); + } + + [Theory, BitAutoData] + public async Task ValidateAsync_DuplicateIdWithinUpdate_ReturnsError( + SutProvider sutProvider, Guid existingUserId) + { + var target = new CollectionUserAccessTarget( + new Collection { Id = Guid.NewGuid(), OrganizationId = Guid.NewGuid() }, + new CollectionAccessDetails + { + Users = [new CollectionAccessSelection { Id = existingUserId }], + Groups = [] + }); + var update = new[] + { + new CollectionAccessSelection { Id = existingUserId }, + new CollectionAccessSelection { Id = existingUserId, Manage = true } + }; + var request = new ModifyCollectionUserAccessRequest([target], [], update, [], null, false); + + var result = await sutProvider.Sut.ValidateAsync(request); + + Assert.True(result.IsError); + Assert.IsType(result.AsError); + } + + [Theory, BitAutoData] + public async Task ValidateAsync_IdInBothRemoveAndUpdate_ReturnsError( + SutProvider sutProvider, Guid conflictingUserId) + { + var target = new CollectionUserAccessTarget( + new Collection { Id = Guid.NewGuid(), OrganizationId = Guid.NewGuid() }, + new CollectionAccessDetails + { + Users = [new CollectionAccessSelection { Id = conflictingUserId }], + Groups = [] + }); + var request = new ModifyCollectionUserAccessRequest( + [target], [], [new CollectionAccessSelection { Id = conflictingUserId }], [conflictingUserId], null, false); + + var result = await sutProvider.Sut.ValidateAsync(request); + + Assert.True(result.IsError); + Assert.IsType(result.AsError); + } + + [Theory, BitAutoData] + public async Task ValidateAsync_AddManageWithReadOnlyOrHidePasswords_ReturnsError( + SutProvider sutProvider, Guid readOnlyUserId, Guid hidePasswordsUserId) + { + var target = new CollectionUserAccessTarget( + new Collection { Id = Guid.NewGuid(), OrganizationId = Guid.NewGuid() }, + new CollectionAccessDetails { Users = [], Groups = [] }); + + var readOnlyRequest = new ModifyCollectionUserAccessRequest( + [target], [new CollectionAccessSelection { Id = readOnlyUserId, Manage = true, ReadOnly = true }], [], [], null, false); + var readOnlyResult = await sutProvider.Sut.ValidateAsync(readOnlyRequest); + Assert.True(readOnlyResult.IsError); + Assert.IsType(readOnlyResult.AsError); + + var hidePasswordsRequest = new ModifyCollectionUserAccessRequest( + [target], [new CollectionAccessSelection { Id = hidePasswordsUserId, Manage = true, HidePasswords = true }], [], [], null, false); + var hidePasswordsResult = await sutProvider.Sut.ValidateAsync(hidePasswordsRequest); + Assert.True(hidePasswordsResult.IsError); + Assert.IsType(hidePasswordsResult.AsError); + } + + [Theory, BitAutoData] + public async Task ValidateAsync_UpdateManageWithReadOnlyOrHidePasswords_ReturnsError( + SutProvider sutProvider, Guid existingUserId) + { + var target = new CollectionUserAccessTarget( + new Collection { Id = Guid.NewGuid(), OrganizationId = Guid.NewGuid() }, + new CollectionAccessDetails + { + Users = [new CollectionAccessSelection { Id = existingUserId }], + Groups = [] + }); + + var readOnlyRequest = new ModifyCollectionUserAccessRequest( + [target], [], [new CollectionAccessSelection { Id = existingUserId, Manage = true, ReadOnly = true }], [], null, false); + var readOnlyResult = await sutProvider.Sut.ValidateAsync(readOnlyRequest); + Assert.True(readOnlyResult.IsError); + Assert.IsType(readOnlyResult.AsError); + + var hidePasswordsRequest = new ModifyCollectionUserAccessRequest( + [target], [], [new CollectionAccessSelection { Id = existingUserId, Manage = true, HidePasswords = true }], [], null, false); + var hidePasswordsResult = await sutProvider.Sut.ValidateAsync(hidePasswordsRequest); + Assert.True(hidePasswordsResult.IsError); + Assert.IsType(hidePasswordsResult.AsError); + } + + [Theory, BitAutoData] + public async Task ValidateAsync_DefaultUserCollection_ReturnsError( + SutProvider sutProvider, Guid newUserId) + { + var target = new CollectionUserAccessTarget( + new Collection { Id = Guid.NewGuid(), OrganizationId = Guid.NewGuid(), Type = CollectionType.DefaultUserCollection }, + new CollectionAccessDetails { Users = [], Groups = [] }); + var request = new ModifyCollectionUserAccessRequest( + [target], [new CollectionAccessSelection { Id = newUserId }], [], [], null, false); + + var result = await sutProvider.Sut.ValidateAsync(request); + + Assert.True(result.IsError); + Assert.IsType(result.AsError); + } + + [Theory, BitAutoData] + public async Task ValidateAsync_AddIdAlreadyExistingMember_ReturnsError( + SutProvider sutProvider, Guid existingUserId) + { + var target = new CollectionUserAccessTarget( + new Collection { Id = Guid.NewGuid(), OrganizationId = Guid.NewGuid() }, + new CollectionAccessDetails + { + Users = [new CollectionAccessSelection { Id = existingUserId }], + Groups = [] + }); + var request = new ModifyCollectionUserAccessRequest( + [target], [new CollectionAccessSelection { Id = existingUserId }], [], [], null, false); + + var result = await sutProvider.Sut.ValidateAsync(request); + + Assert.True(result.IsError); + Assert.IsType(result.AsError); + } + + [Theory, BitAutoData] + public async Task ValidateAsync_UpdateIdNotExistingMember_ReturnsError( + SutProvider sutProvider, Guid nonMemberUserId) + { + var target = new CollectionUserAccessTarget( + new Collection { Id = Guid.NewGuid(), OrganizationId = Guid.NewGuid() }, + new CollectionAccessDetails { Users = [], Groups = [] }); + var request = new ModifyCollectionUserAccessRequest( + [target], [], [new CollectionAccessSelection { Id = nonMemberUserId }], [], null, false); + + var result = await sutProvider.Sut.ValidateAsync(request); + + Assert.True(result.IsError); + Assert.IsType(result.AsError); + } + + [Theory, BitAutoData] + public async Task ValidateAsync_AddTargetDoesNotExist_ReturnsError( + SutProvider sutProvider, Guid newUserId) + { + var target = new CollectionUserAccessTarget( + new Collection { Id = Guid.NewGuid(), OrganizationId = Guid.NewGuid() }, + new CollectionAccessDetails { Users = [], Groups = [] }); + var request = new ModifyCollectionUserAccessRequest( + [target], [new CollectionAccessSelection { Id = newUserId, Manage = true }], [], [], null, false); + + sutProvider.GetDependency() + .GetManyAsync(Arg.Any>()) + .Returns(new List()); + + var result = await sutProvider.Sut.ValidateAsync(request); + + Assert.True(result.IsError); + Assert.IsType(result.AsError); + } + + [Theory, BitAutoData] + public async Task ValidateAsync_AddTargetBelongsToDifferentOrganization_ReturnsError( + SutProvider sutProvider, Guid newUserId, Guid otherOrganizationId) + { + var target = new CollectionUserAccessTarget( + new Collection { Id = Guid.NewGuid(), OrganizationId = Guid.NewGuid() }, + new CollectionAccessDetails { Users = [], Groups = [] }); + var request = new ModifyCollectionUserAccessRequest( + [target], [new CollectionAccessSelection { Id = newUserId, Manage = true }], [], [], null, false); + + sutProvider.GetDependency() + .GetManyAsync(Arg.Any>()) + .Returns([new OrganizationUser { Id = newUserId, OrganizationId = otherOrganizationId }]); + + var result = await sutProvider.Sut.ValidateAsync(request); + + Assert.True(result.IsError); + Assert.IsType(result.AsError); + } + + [Theory, BitAutoData] + public async Task ValidateAsync_AddingSelfAsNewMember_WithoutAllowAdminAccess_ReturnsError( + SutProvider sutProvider, Guid performingId, Guid organizationId) + { + var target = new CollectionUserAccessTarget( + new Collection { Id = Guid.NewGuid(), OrganizationId = organizationId }, + new CollectionAccessDetails { Users = [], Groups = [] }); + ArrangeValidOrganizationUsers(sutProvider, organizationId); + var request = new ModifyCollectionUserAccessRequest( + [target], [new CollectionAccessSelection { Id = performingId, Manage = true }], [], [], performingId, false); + + var result = await sutProvider.Sut.ValidateAsync(request); + + Assert.True(result.IsError); + Assert.IsType(result.AsError); + } + + [Theory, BitAutoData] + public async Task ValidateAsync_AddingSelfAsNewMember_WithAllowAdminAccess_Succeeds( + SutProvider sutProvider, Guid performingId, Guid organizationId) + { + var target = new CollectionUserAccessTarget( + new Collection { Id = Guid.NewGuid(), OrganizationId = organizationId }, + new CollectionAccessDetails { Users = [], Groups = [] }); + ArrangeValidOrganizationUsers(sutProvider, organizationId); + var request = new ModifyCollectionUserAccessRequest( + [target], [new CollectionAccessSelection { Id = performingId, Manage = true }], [], [], performingId, true); + + var result = await sutProvider.Sut.ValidateAsync(request); + + Assert.True(result.IsValid); + } + + [Theory, BitAutoData] + public async Task ValidateAsync_ChangingOwnExistingAccess_Succeeds( + SutProvider sutProvider, Guid performingId, Guid organizationId) + { + var target = new CollectionUserAccessTarget( + new Collection { Id = Guid.NewGuid(), OrganizationId = organizationId }, + new CollectionAccessDetails + { + Users = [new CollectionAccessSelection { Id = performingId }], + Groups = [] + }); + ArrangeValidOrganizationUsers(sutProvider, organizationId); + var request = new ModifyCollectionUserAccessRequest( + [target], [], [new CollectionAccessSelection { Id = performingId, Manage = true }], [], performingId, false); + + var result = await sutProvider.Sut.ValidateAsync(request); + + Assert.True(result.IsValid); + } + + [Theory, BitAutoData] + public async Task ValidateAsync_RemovingLastManager_WithoutAllowAdminAccess_ReturnsError( + SutProvider sutProvider, Guid managerId, Guid organizationId) + { + var target = new CollectionUserAccessTarget( + new Collection { Id = Guid.NewGuid(), OrganizationId = organizationId }, + new CollectionAccessDetails + { + Users = [new CollectionAccessSelection { Id = managerId, Manage = true }], + Groups = [] + }); + ArrangeValidOrganizationUsers(sutProvider, organizationId); + var request = new ModifyCollectionUserAccessRequest([target], [], [], [managerId], null, false); + + var result = await sutProvider.Sut.ValidateAsync(request); + + Assert.True(result.IsError); + Assert.IsType(result.AsError); + } + + [Theory, BitAutoData] + public async Task ValidateAsync_RemovingLastManager_WithAllowAdminAccess_Succeeds( + SutProvider sutProvider, Guid managerId, Guid organizationId) + { + var target = new CollectionUserAccessTarget( + new Collection { Id = Guid.NewGuid(), OrganizationId = organizationId }, + new CollectionAccessDetails + { + Users = [new CollectionAccessSelection { Id = managerId, Manage = true }], + Groups = [] + }); + ArrangeValidOrganizationUsers(sutProvider, organizationId); + var request = new ModifyCollectionUserAccessRequest([target], [], [], [managerId], null, true); + + var result = await sutProvider.Sut.ValidateAsync(request); + + Assert.True(result.IsValid); + } + + [Theory, BitAutoData] + public async Task ValidateAsync_RemovingOnlyManagingUser_ButGroupStillManages_Succeeds( + SutProvider sutProvider, Guid managerId, Guid groupId, Guid organizationId) + { + var target = new CollectionUserAccessTarget( + new Collection { Id = Guid.NewGuid(), OrganizationId = organizationId }, + new CollectionAccessDetails + { + Users = [new CollectionAccessSelection { Id = managerId, Manage = true }], + Groups = [new CollectionAccessSelection { Id = groupId, Manage = true }] + }); + ArrangeValidOrganizationUsers(sutProvider, organizationId); + var request = new ModifyCollectionUserAccessRequest([target], [], [], [managerId], null, false); + + var result = await sutProvider.Sut.ValidateAsync(request); + + Assert.True(result.IsValid); + } + + [Theory, BitAutoData] + public async Task ValidateAsync_UpdatingOnlyManagerToNonManage_NoOtherManager_ReturnsError( + SutProvider sutProvider, Guid managerId, Guid organizationId) + { + var target = new CollectionUserAccessTarget( + new Collection { Id = Guid.NewGuid(), OrganizationId = organizationId }, + new CollectionAccessDetails + { + Users = [new CollectionAccessSelection { Id = managerId, Manage = true }], + Groups = [] + }); + ArrangeValidOrganizationUsers(sutProvider, organizationId); + var request = new ModifyCollectionUserAccessRequest( + [target], [], [new CollectionAccessSelection { Id = managerId, Manage = false }], [], null, false); + + var result = await sutProvider.Sut.ValidateAsync(request); + + Assert.True(result.IsError); + Assert.IsType(result.AsError); + } + + [Theory, BitAutoData] + public async Task ValidateAsync_AddingNewManagerToOrphanedCollection_Succeeds( + SutProvider sutProvider, Guid newUserId, Guid organizationId) + { + var target = new CollectionUserAccessTarget( + new Collection { Id = Guid.NewGuid(), OrganizationId = organizationId }, + new CollectionAccessDetails { Users = [], Groups = [] }); + ArrangeValidOrganizationUsers(sutProvider, organizationId); + var request = new ModifyCollectionUserAccessRequest( + [target], [new CollectionAccessSelection { Id = newUserId, Manage = true }], [], [], null, false); + + var result = await sutProvider.Sut.ValidateAsync(request); + + Assert.True(result.IsValid); + } + + [Theory, BitAutoData] + public async Task ValidateAsync_ValidDelta_Succeeds( + SutProvider sutProvider, + Guid existingUserId, Guid newUserId, Guid removedUserId, Guid organizationId) + { + var target = new CollectionUserAccessTarget( + new Collection { Id = Guid.NewGuid(), OrganizationId = organizationId }, + new CollectionAccessDetails + { + Users = + [ + new CollectionAccessSelection { Id = existingUserId }, + new CollectionAccessSelection { Id = removedUserId } + ], + Groups = [] + }); + ArrangeValidOrganizationUsers(sutProvider, organizationId); + var request = new ModifyCollectionUserAccessRequest( + [target], + [new CollectionAccessSelection { Id = newUserId }], + [new CollectionAccessSelection { Id = existingUserId, Manage = true }], + [removedUserId], + null, false); + + var result = await sutProvider.Sut.ValidateAsync(request); + + Assert.True(result.IsValid); + } + + [Theory, BitAutoData] + public async Task ValidateAsync_MultipleTargets_AnyDefaultUserCollection_ReturnsError( + SutProvider sutProvider, Guid newUserId) + { + var targets = new[] + { + new CollectionUserAccessTarget( + new Collection { Id = Guid.NewGuid(), OrganizationId = Guid.NewGuid() }, + new CollectionAccessDetails { Users = [], Groups = [] }), + new CollectionUserAccessTarget( + new Collection { Id = Guid.NewGuid(), OrganizationId = Guid.NewGuid(), Type = CollectionType.DefaultUserCollection }, + new CollectionAccessDetails { Users = [], Groups = [] }) + }; + var request = new ModifyCollectionUserAccessRequest( + targets, [new CollectionAccessSelection { Id = newUserId }], [], [], null, false); + + var result = await sutProvider.Sut.ValidateAsync(request); + + Assert.True(result.IsError); + Assert.IsType(result.AsError); + } + + [Theory, BitAutoData] + public async Task ValidateAsync_MultipleTargets_AlreadyMemberOfOneNotTheOther_Succeeds( + SutProvider sutProvider, Guid existingUserId, Guid organizationId) + { + // The Add-must-be-new check only applies to a single collection. Across multiple targets the same + // user can already have access to one and not another, so we don't check it here. + var targets = new[] + { + new CollectionUserAccessTarget( + new Collection { Id = Guid.NewGuid(), OrganizationId = organizationId }, + new CollectionAccessDetails + { + Users = [new CollectionAccessSelection { Id = existingUserId }], + Groups = [] + }), + new CollectionUserAccessTarget( + new Collection { Id = Guid.NewGuid(), OrganizationId = organizationId }, + new CollectionAccessDetails { Users = [], Groups = [] }) + }; + ArrangeValidOrganizationUsers(sutProvider, organizationId); + var request = new ModifyCollectionUserAccessRequest( + targets, [new CollectionAccessSelection { Id = existingUserId, Manage = true }], [], [], null, false); + + var result = await sutProvider.Sut.ValidateAsync(request); + + Assert.True(result.IsValid); + } + + [Theory, BitAutoData] + public async Task ValidateAsync_MultipleTargets_RemovingOnlyManagerOfOneTarget_ReturnsError( + SutProvider sutProvider, + Guid managerId, Guid otherManagerId, Guid organizationId) + { + var targets = new[] + { + new CollectionUserAccessTarget( + new Collection { Id = Guid.NewGuid(), OrganizationId = organizationId }, + new CollectionAccessDetails + { + Users = [new CollectionAccessSelection { Id = managerId, Manage = true }], + Groups = [] + }), + new CollectionUserAccessTarget( + new Collection { Id = Guid.NewGuid(), OrganizationId = organizationId }, + new CollectionAccessDetails + { + Users = [new CollectionAccessSelection { Id = otherManagerId, Manage = true }], + Groups = [] + }) + }; + ArrangeValidOrganizationUsers(sutProvider, organizationId); + var request = new ModifyCollectionUserAccessRequest(targets, [], [], [managerId], null, false); + + var result = await sutProvider.Sut.ValidateAsync(request); + + Assert.True(result.IsError); + Assert.IsType(result.AsError); + } + + [Theory, BitAutoData] + public async Task ValidateAsync_MultipleTargets_AddingSelfAsNewMemberOnOneTarget_ReturnsError( + SutProvider sutProvider, Guid performingId, Guid organizationId) + { + var targets = new[] + { + new CollectionUserAccessTarget( + new Collection { Id = Guid.NewGuid(), OrganizationId = organizationId }, + new CollectionAccessDetails + { + Users = [new CollectionAccessSelection { Id = performingId }], + Groups = [] + }), + new CollectionUserAccessTarget( + new Collection { Id = Guid.NewGuid(), OrganizationId = organizationId }, + new CollectionAccessDetails { Users = [], Groups = [] }) + }; + ArrangeValidOrganizationUsers(sutProvider, organizationId); + var request = new ModifyCollectionUserAccessRequest( + targets, [new CollectionAccessSelection { Id = performingId }], [], [], performingId, false); + + var result = await sutProvider.Sut.ValidateAsync(request); + + Assert.True(result.IsError); + Assert.IsType(result.AsError); + } + + [Theory, BitAutoData] + public async Task ValidateAsync_MultipleTargets_ValidDelta_Succeeds( + SutProvider sutProvider, Guid newUserId, Guid organizationId) + { + var targets = new[] + { + new CollectionUserAccessTarget( + new Collection { Id = Guid.NewGuid(), OrganizationId = organizationId }, + new CollectionAccessDetails { Users = [], Groups = [] }), + new CollectionUserAccessTarget( + new Collection { Id = Guid.NewGuid(), OrganizationId = organizationId }, + new CollectionAccessDetails { Users = [], Groups = [] }) + }; + ArrangeValidOrganizationUsers(sutProvider, organizationId); + var request = new ModifyCollectionUserAccessRequest( + targets, [new CollectionAccessSelection { Id = newUserId, Manage = true }], [], [], null, false); + + var result = await sutProvider.Sut.ValidateAsync(request); + + Assert.True(result.IsValid); + } + + [Theory, BitAutoData] + public async Task ValidateAsync_MultipleTargets_UpdatingSelfAsNewMemberOnOneTarget_ReturnsError( + SutProvider sutProvider, Guid performingId, Guid organizationId) + { + // Regression test. Putting the performing user's own id in Update instead of Add must not bypass the + // self-add guard. The same delta hits every target, so this would otherwise give the performing user + // brand-new access on the second target. + var targets = new[] + { + new CollectionUserAccessTarget( + new Collection { Id = Guid.NewGuid(), OrganizationId = organizationId }, + new CollectionAccessDetails + { + Users = [new CollectionAccessSelection { Id = performingId }], + Groups = [] + }), + new CollectionUserAccessTarget( + new Collection { Id = Guid.NewGuid(), OrganizationId = organizationId }, + new CollectionAccessDetails { Users = [], Groups = [] }) + }; + ArrangeValidOrganizationUsers(sutProvider, organizationId); + var request = new ModifyCollectionUserAccessRequest( + targets, [], [new CollectionAccessSelection { Id = performingId, Manage = true }], [], performingId, false); + + var result = await sutProvider.Sut.ValidateAsync(request); + + Assert.True(result.IsError); + Assert.IsType(result.AsError); + } + + [Theory, BitAutoData] + public async Task ValidateAsync_MultipleTargets_UpdateGrantsNewManagerOnOtherTarget_Succeeds( + SutProvider sutProvider, Guid managerId, Guid organizationId) + { + // Regression test. An Update entry for a user who isn't yet a member of a target still upserts onto + // that target, since the same delta applies to every collection. It has to count toward that target's + // remaining manage access, or a valid request gets rejected as leaving the second target unmanaged. + var targets = new[] + { + new CollectionUserAccessTarget( + new Collection { Id = Guid.NewGuid(), OrganizationId = organizationId }, + new CollectionAccessDetails + { + Users = [new CollectionAccessSelection { Id = managerId, Manage = true }], + Groups = [] + }), + new CollectionUserAccessTarget( + new Collection { Id = Guid.NewGuid(), OrganizationId = organizationId }, + new CollectionAccessDetails { Users = [], Groups = [] }) + }; + ArrangeValidOrganizationUsers(sutProvider, organizationId); + var request = new ModifyCollectionUserAccessRequest( + targets, [], [new CollectionAccessSelection { Id = managerId, Manage = true }], [], null, false); + + var result = await sutProvider.Sut.ValidateAsync(request); + + Assert.True(result.IsValid); + } + + // Any id passed to GetManyAsync resolves as a valid organization user in the given org, unless overridden. + private static void ArrangeValidOrganizationUsers( + SutProvider sutProvider, Guid organizationId) + { + sutProvider.GetDependency() + .GetManyAsync(Arg.Any>()) + .Returns(callInfo => callInfo.Arg>() + .Select(id => new OrganizationUser { Id = id, OrganizationId = organizationId }) + .ToList()); + } +} diff --git a/test/Infrastructure.IntegrationTest/AdminConsole/Repositories/CollectionRepository/CollectionRepositoryTests.cs b/test/Infrastructure.IntegrationTest/AdminConsole/Repositories/CollectionRepository/CollectionRepositoryTests.cs index f8b6ee257d0f..a9c95e65da5d 100644 --- a/test/Infrastructure.IntegrationTest/AdminConsole/Repositories/CollectionRepository/CollectionRepositoryTests.cs +++ b/test/Infrastructure.IntegrationTest/AdminConsole/Repositories/CollectionRepository/CollectionRepositoryTests.cs @@ -646,4 +646,106 @@ await collectionRepository.CreateOrUpdateAccessForManyAsync( Assert.True(groupAccess.ReadOnly); Assert.True(groupAccess.HidePasswords); } + + [DatabaseTheory, DatabaseData] + public async Task ModifyUserAccessAsync_AppliesAddUpdateAndRemove_BumpsRevisionDate( + IOrganizationRepository organizationRepository, + IOrganizationUserRepository organizationUserRepository, + ICollectionRepository collectionRepository, + IUserRepository userRepository) + { + var organization = await organizationRepository.CreateTestOrganizationAsync(); + var updatedUser = await organizationUserRepository.CreateTestOrganizationUserAsync( + organization, await userRepository.CreateTestUserAsync()); + var addedUser = await organizationUserRepository.CreateTestOrganizationUserAsync( + organization, await userRepository.CreateTestUserAsync()); + var removedUser = await organizationUserRepository.CreateTestOrganizationUserAsync( + organization, await userRepository.CreateTestUserAsync()); + var collection = await collectionRepository.CreateTestCollectionAsync(organization); + + await collectionRepository.CreateOrUpdateAccessForManyAsync( + organization.Id, + [collection.Id], + [ + new CollectionAccessSelection { Id = updatedUser.Id, ReadOnly = true }, + new CollectionAccessSelection { Id = removedUser.Id, ReadOnly = true } + ], + [], + DateTime.UtcNow); + + var revisionDate = DateTime.UtcNow.AddMinutes(10); + + await collectionRepository.ModifyUserAccessAsync( + organization.Id, + [collection.Id], + [ + new CollectionAccessSelection { Id = updatedUser.Id, Manage = true }, + new CollectionAccessSelection { Id = addedUser.Id, ReadOnly = true } + ], + [removedUser.Id], + revisionDate); + + var (actualCollection, actualAccess) = await collectionRepository.GetByIdWithAccessAsync(collection.Id); + Assert.NotNull(actualCollection); + Assert.Equal(revisionDate, actualCollection.RevisionDate, TimeSpan.FromMilliseconds(10)); + + Assert.Equal(2, actualAccess.Users.Count()); + + var updatedAccess = actualAccess.Users.Single(u => u.Id == updatedUser.Id); + Assert.True(updatedAccess.Manage); + + var addedAccess = actualAccess.Users.Single(u => u.Id == addedUser.Id); + Assert.True(addedAccess.ReadOnly); + + Assert.DoesNotContain(actualAccess.Users, u => u.Id == removedUser.Id); + } + + [DatabaseTheory, DatabaseData] + public async Task ModifyGroupAccessAsync_AppliesAddUpdateAndRemove_BumpsRevisionDate( + IOrganizationRepository organizationRepository, + IGroupRepository groupRepository, + ICollectionRepository collectionRepository) + { + var organization = await organizationRepository.CreateTestOrganizationAsync(); + var updatedGroup = await groupRepository.CreateTestGroupAsync(organization); + var addedGroup = await groupRepository.CreateTestGroupAsync(organization); + var removedGroup = await groupRepository.CreateTestGroupAsync(organization); + var collection = await collectionRepository.CreateTestCollectionAsync(organization); + + await collectionRepository.CreateOrUpdateAccessForManyAsync( + organization.Id, + [collection.Id], + [], + [ + new CollectionAccessSelection { Id = updatedGroup.Id, ReadOnly = true }, + new CollectionAccessSelection { Id = removedGroup.Id, ReadOnly = true } + ], + DateTime.UtcNow); + + var revisionDate = DateTime.UtcNow.AddMinutes(10); + + await collectionRepository.ModifyGroupAccessAsync( + organization.Id, + [collection.Id], + [ + new CollectionAccessSelection { Id = updatedGroup.Id, Manage = true }, + new CollectionAccessSelection { Id = addedGroup.Id, ReadOnly = true } + ], + [removedGroup.Id], + revisionDate); + + var (actualCollection, actualAccess) = await collectionRepository.GetByIdWithAccessAsync(collection.Id); + Assert.NotNull(actualCollection); + Assert.Equal(revisionDate, actualCollection.RevisionDate, TimeSpan.FromMilliseconds(10)); + + Assert.Equal(2, actualAccess.Groups.Count()); + + var updatedAccess = actualAccess.Groups.Single(g => g.Id == updatedGroup.Id); + Assert.True(updatedAccess.Manage); + + var addedAccess = actualAccess.Groups.Single(g => g.Id == addedGroup.Id); + Assert.True(addedAccess.ReadOnly); + + Assert.DoesNotContain(actualAccess.Groups, g => g.Id == removedGroup.Id); + } } diff --git a/util/Migrator/DbScripts/2026-08-13_00_AddCollectionUserDeleteMany.sql b/util/Migrator/DbScripts/2026-08-13_00_AddCollectionUserDeleteMany.sql new file mode 100644 index 000000000000..927f037b8401 --- /dev/null +++ b/util/Migrator/DbScripts/2026-08-13_00_AddCollectionUserDeleteMany.sql @@ -0,0 +1,16 @@ +CREATE OR ALTER PROCEDURE [dbo].[CollectionUser_DeleteMany] + @CollectionIds [dbo].[GuidIdArray] READONLY, + @OrganizationUserIds [dbo].[GuidIdArray] READONLY +AS +BEGIN + SET NOCOUNT ON + + DELETE + FROM + [dbo].[CollectionUser] + WHERE + [CollectionId] IN (SELECT [Id] FROM @CollectionIds) + AND [OrganizationUserId] IN (SELECT [Id] FROM @OrganizationUserIds) + + EXEC [dbo].[User_BumpAccountRevisionDateByOrganizationUserIds] @OrganizationUserIds +END diff --git a/util/Migrator/DbScripts/2026-08-13_01_AddCollectionGroupDeleteMany.sql b/util/Migrator/DbScripts/2026-08-13_01_AddCollectionGroupDeleteMany.sql new file mode 100644 index 000000000000..ab830fc94649 --- /dev/null +++ b/util/Migrator/DbScripts/2026-08-13_01_AddCollectionGroupDeleteMany.sql @@ -0,0 +1,21 @@ +CREATE OR ALTER PROCEDURE [dbo].[CollectionGroup_DeleteMany] + @CollectionIds [dbo].[GuidIdArray] READONLY, + @GroupIds [dbo].[GuidIdArray] READONLY +AS +BEGIN + SET NOCOUNT ON + + DELETE + FROM + [dbo].[CollectionGroup] + WHERE + [CollectionId] IN (SELECT [Id] FROM @CollectionIds) + AND [GroupId] IN (SELECT [Id] FROM @GroupIds) + + UPDATE + [dbo].[Group] + SET + [RevisionDate] = GETUTCDATE() + WHERE + [Id] IN (SELECT [Id] FROM @GroupIds) +END