diff --git a/src/Api/AdminConsole/Authorization/AuthorizationHandlerCollectionExtensions.cs b/src/Api/AdminConsole/Authorization/AuthorizationHandlerCollectionExtensions.cs index 3f80d5b918a6..113798e786a4 100644 --- a/src/Api/AdminConsole/Authorization/AuthorizationHandlerCollectionExtensions.cs +++ b/src/Api/AdminConsole/Authorization/AuthorizationHandlerCollectionExtensions.cs @@ -15,6 +15,7 @@ public static void AddAdminConsoleAuthorizationHandlers(this IServiceCollection ServiceDescriptor.Scoped(), ServiceDescriptor.Scoped(), ServiceDescriptor.Scoped(), + ServiceDescriptor.Scoped(), ServiceDescriptor.Scoped(), ServiceDescriptor.Scoped(), ServiceDescriptor.Scoped(), diff --git a/src/Api/AdminConsole/Authorization/Collections/CollectionGroupAccessResource.cs b/src/Api/AdminConsole/Authorization/Collections/CollectionGroupAccessResource.cs new file mode 100644 index 000000000000..cfcd8c6be008 --- /dev/null +++ b/src/Api/AdminConsole/Authorization/Collections/CollectionGroupAccessResource.cs @@ -0,0 +1,8 @@ +using Bit.Core.Entities; +using Bit.Core.Models.Data; + +namespace Bit.Api.AdminConsole.Authorization.Collections; + +public record CollectionGroupAccessResource( + Collection Collection, + CollectionAccessDetails AccessDetails); diff --git a/src/Api/AdminConsole/Authorization/Collections/CollectionGroupAuthorizationHandler.cs b/src/Api/AdminConsole/Authorization/Collections/CollectionGroupAuthorizationHandler.cs new file mode 100644 index 000000000000..f42fecf695da --- /dev/null +++ b/src/Api/AdminConsole/Authorization/Collections/CollectionGroupAuthorizationHandler.cs @@ -0,0 +1,89 @@ +#nullable enable +using Bit.Core.AdminConsole.AbilitiesCache; +using Bit.Core.Context; +using Bit.Core.Exceptions; +using Bit.Core.Repositories; +using Bit.Core.Utilities; +using Microsoft.AspNetCore.Authorization; + +namespace Bit.Api.AdminConsole.Authorization.Collections; + +/// +/// Checks whether the caller can change one or more groups' access to one or more collections. +/// All the collections must be in the same organization. +/// +public class CollectionGroupAuthorizationHandler + : BulkAuthorizationHandler +{ + private readonly ICurrentContext _currentContext; + private readonly ICollectionRepository _collectionRepository; + private readonly IOrganizationAbilityCacheService _organizationAbilityCacheService; + private HashSet? _managedCollectionIds; + + public CollectionGroupAuthorizationHandler( + ICurrentContext currentContext, + ICollectionRepository collectionRepository, + IOrganizationAbilityCacheService organizationAbilityCacheService) + { + _currentContext = currentContext; + _collectionRepository = collectionRepository; + _organizationAbilityCacheService = organizationAbilityCacheService; + } + + protected override async Task HandleRequirementAsync(AuthorizationHandlerContext context, + CollectionGroupOperationRequirement requirement, ICollection resources) + { + if (resources.Count == 0) + { + return; + } + + if (!_currentContext.UserId.HasValue) + { + return; + } + + var organizationId = resources.First().Collection.OrganizationId; + if (resources.Any(r => r.Collection.OrganizationId != organizationId)) + { + throw new BadRequestException("Requested collections must belong to the same organization."); + } + + var organization = _currentContext.GetOrganization(organizationId); + var organizationAbility = await _organizationAbilityCacheService.GetOrganizationAbilityAsync(organizationId); + var allowAdminAccessToAllCollectionItems = organizationAbility is { AllowAdminAccessToAllCollectionItems: true }; + + var authorized = true; + foreach (var resource in resources) + { + var callerManagesCollection = await CallerManagesCollectionAsync(resource.Collection.Id); + if (!CollectionGroupAuthorizationRules.CanModifyGroupAccess( + resource.AccessDetails, organization, allowAdminAccessToAllCollectionItems, callerManagesCollection)) + { + authorized = false; + break; + } + } + + if (!authorized) + { + authorized = await _currentContext.ProviderUserForOrgAsync(organizationId); + } + + if (authorized) + { + context.Succeed(requirement); + } + } + + private async Task CallerManagesCollectionAsync(Guid collectionId) + { + if (_managedCollectionIds == null) + { + var callerCollections = await _collectionRepository.GetManyByUserIdAsync(_currentContext.UserId!.Value); + _managedCollectionIds = callerCollections.Where(c => c.Manage).Select(c => c.Id).ToHashSet(); + } + + return _managedCollectionIds.Contains(collectionId); + } +} diff --git a/src/Api/AdminConsole/Authorization/Collections/CollectionGroupAuthorizationRules.cs b/src/Api/AdminConsole/Authorization/Collections/CollectionGroupAuthorizationRules.cs new file mode 100644 index 000000000000..aca240d9511b --- /dev/null +++ b/src/Api/AdminConsole/Authorization/Collections/CollectionGroupAuthorizationRules.cs @@ -0,0 +1,52 @@ +#nullable enable +using Bit.Core.Context; +using Bit.Core.Models.Data; + +namespace Bit.Api.AdminConsole.Authorization.Collections; + +/// +/// Decides whether a user can change a group's access to a collection. +/// +public static class CollectionGroupAuthorizationRules +{ + /// + /// Returns true if the acting user can add, change, or remove a group's access to this collection. + /// covers Manage granted directly or through a group. + /// + public static bool CanModifyGroupAccess( + CollectionAccessDetails accessDetails, + CurrentContextOrganization? organization, + bool allowAdminAccessToAllCollectionItems, + bool callerManagesCollection) + { + if (organization is { Permissions.EditAnyCollection: true }) + { + return true; + } + + if (allowAdminAccessToAllCollectionItems && organization is { Permissions.ManageUsers: true }) + { + return true; + } + + if (allowAdminAccessToAllCollectionItems && organization is { IsAdminOrOwner: true }) + { + return true; + } + + if (callerManagesCollection) + { + return true; + } + + // Owners and Admins can still manage an orphaned collection even when + // AllowAdminAccessToAllCollectionItems is off. + if (organization is not { IsAdminOrOwner: true }) + { + return false; + } + + var isOrphaned = !accessDetails.Users.Any(u => u.Manage) && !accessDetails.Groups.Any(g => g.Manage); + return isOrphaned; + } +} diff --git a/src/Api/AdminConsole/Authorization/Collections/CollectionGroupOperations.cs b/src/Api/AdminConsole/Authorization/Collections/CollectionGroupOperations.cs new file mode 100644 index 000000000000..8d0d0acc9e5a --- /dev/null +++ b/src/Api/AdminConsole/Authorization/Collections/CollectionGroupOperations.cs @@ -0,0 +1,12 @@ +using Microsoft.AspNetCore.Authorization.Infrastructure; + +namespace Bit.Api.AdminConsole.Authorization.Collections; + +public class CollectionGroupOperationRequirement : OperationAuthorizationRequirement { } + +public static class CollectionGroupOperations +{ + public static readonly CollectionGroupOperationRequirement Create = new() { Name = nameof(Create) }; + public static readonly CollectionGroupOperationRequirement Update = new() { Name = nameof(Update) }; + public static readonly CollectionGroupOperationRequirement Delete = new() { Name = nameof(Delete) }; +} 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..acd5ea667990 --- /dev/null +++ b/src/Core/AdminConsole/OrganizationFeatures/Collections/ModifyGroupAccess/IModifyCollectionGroupAccessCommand.cs @@ -0,0 +1,14 @@ +using Bit.Core.AdminConsole.Utilities.v2.Results; + +namespace Bit.Core.AdminConsole.OrganizationFeatures.Collections.ModifyGroupAccess; + +/// +/// Applies an add/update/remove delta to one or more collections' group access. +/// +public interface IModifyCollectionGroupAccessCommand +{ + /// + /// Validates and persists the delta. + /// + 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/Repositories/ICollectionRepository.cs b/src/Core/AdminConsole/Repositories/ICollectionRepository.cs index 64c351db56f5..13b6c6dcd1d4 100644 --- a/src/Core/AdminConsole/Repositories/ICollectionRepository.cs +++ b/src/Core/AdminConsole/Repositories/ICollectionRepository.cs @@ -70,6 +70,18 @@ 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/OrganizationFeatures/OrganizationServiceCollectionExtensions.cs b/src/Core/OrganizationFeatures/OrganizationServiceCollectionExtensions.cs index 58275b709d9a..91669bb64f57 100644 --- a/src/Core/OrganizationFeatures/OrganizationServiceCollectionExtensions.cs +++ b/src/Core/OrganizationFeatures/OrganizationServiceCollectionExtensions.cs @@ -3,6 +3,7 @@ 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; @@ -193,6 +194,8 @@ public static void AddOrganizationCollectionCommands(this IServiceCollection ser 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 74fd79ae5b79..82813153c366 100644 --- a/src/Infrastructure.Dapper/AdminConsole/Repositories/CollectionRepository.cs +++ b/src/Infrastructure.Dapper/AdminConsole/Repositories/CollectionRepository.cs @@ -396,6 +396,56 @@ await connection.ExecuteAsync( } } + 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 c85de61c47e9..ee0e80e4c09a 100644 --- a/src/Infrastructure.EntityFramework/AdminConsole/Repositories/CollectionRepository.cs +++ b/src/Infrastructure.EntityFramework/AdminConsole/Repositories/CollectionRepository.cs @@ -208,6 +208,109 @@ public async Task ModifyUserAccessAsync(Guid organizationId, IEnumerable c } } + 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/test/Api.Test/AdminConsole/Authorization/CollectionGroupAuthorizationHandlerTests.cs b/test/Api.Test/AdminConsole/Authorization/CollectionGroupAuthorizationHandlerTests.cs new file mode 100644 index 000000000000..b086f67b9b59 --- /dev/null +++ b/test/Api.Test/AdminConsole/Authorization/CollectionGroupAuthorizationHandlerTests.cs @@ -0,0 +1,215 @@ +using System.Security.Claims; +using Bit.Api.AdminConsole.Authorization.Collections; +using Bit.Core.Context; +using Bit.Core.Entities; +using Bit.Core.Enums; +using Bit.Core.Exceptions; +using Bit.Core.Models.Data; +using Bit.Core.Repositories; +using Bit.Core.Test.Vault.AutoFixture; +using Bit.Test.Common.AutoFixture; +using Bit.Test.Common.AutoFixture.Attributes; +using Microsoft.AspNetCore.Authorization; +using NSubstitute; +using Xunit; + +namespace Bit.Api.Test.AdminConsole.Authorization; + +[SutProviderCustomize] +public class CollectionGroupAuthorizationHandlerTests +{ + [Theory, BitAutoData, CollectionCustomization] + public async Task HandleRequirementAsync_MissingUserId_NoSuccess( + SutProvider sutProvider, + CollectionGroupAccessResource resource) + { + sutProvider.GetDependency().UserId.Returns((Guid?)null); + + var context = new AuthorizationHandlerContext( + new[] { CollectionGroupOperations.Update }, new ClaimsPrincipal(), resource); + + await sutProvider.Sut.HandleAsync(context); + + Assert.False(context.HasSucceeded); + } + + [Theory, BitAutoData, CollectionCustomization] + public async Task HandleRequirementAsync_WithEditAnyCollectionPermission_Success( + SutProvider sutProvider, + CollectionGroupAccessResource resource, + CurrentContextOrganization organization, + Guid userId) + { + organization.Permissions = new Permissions { EditAnyCollection = true }; + + sutProvider.GetDependency().UserId.Returns(userId); + sutProvider.GetDependency().GetOrganization(resource.Collection.OrganizationId).Returns(organization); + + var context = new AuthorizationHandlerContext( + new[] { CollectionGroupOperations.Update }, new ClaimsPrincipal(), resource); + + await sutProvider.Sut.HandleAsync(context); + + Assert.True(context.HasSucceeded); + } + + [Theory, BitAutoData, CollectionCustomization] + public async Task HandleRequirementAsync_WhenMissingPermissions_NoSuccess( + SutProvider sutProvider, + CollectionGroupAccessResource resource, + CurrentContextOrganization organization, + Guid userId) + { + organization.Type = OrganizationUserType.User; + organization.Permissions = new Permissions(); + + sutProvider.GetDependency().UserId.Returns(userId); + sutProvider.GetDependency().GetOrganization(resource.Collection.OrganizationId).Returns(organization); + sutProvider.GetDependency().ProviderUserForOrgAsync(Arg.Any()).Returns(false); + + var context = new AuthorizationHandlerContext( + new[] { CollectionGroupOperations.Update }, new ClaimsPrincipal(), resource); + + await sutProvider.Sut.HandleAsync(context); + + Assert.False(context.HasSucceeded); + } + + [Theory, BitAutoData, CollectionCustomization] + public async Task HandleRequirementAsync_WhenProviderUser_Success( + SutProvider sutProvider, + CollectionGroupAccessResource resource, + CurrentContextOrganization organization, + Guid userId) + { + organization.Type = OrganizationUserType.User; + organization.Permissions = new Permissions(); + + sutProvider.GetDependency().UserId.Returns(userId); + sutProvider.GetDependency().GetOrganization(resource.Collection.OrganizationId).Returns((CurrentContextOrganization)null); + sutProvider.GetDependency().ProviderUserForOrgAsync(resource.Collection.OrganizationId).Returns(true); + + var context = new AuthorizationHandlerContext( + new[] { CollectionGroupOperations.Update }, new ClaimsPrincipal(), resource); + + await sutProvider.Sut.HandleAsync(context); + + Assert.True(context.HasSucceeded); + } + + [Theory, BitAutoData, CollectionCustomization] + public async Task HandleRequirementAsync_CalledTwiceForSameResource_OnlyQueriesRepositoryOnce( + SutProvider sutProvider, + Collection collection, + CollectionAccessDetails accessDetails, + CurrentContextOrganization organization, + Guid userId) + { + organization.Type = OrganizationUserType.User; + organization.Permissions = new Permissions(); + + sutProvider.GetDependency().UserId.Returns(userId); + sutProvider.GetDependency().GetOrganization(collection.OrganizationId).Returns(organization); + + var addResource = new CollectionGroupAccessResource(collection, accessDetails); + var removeResource = new CollectionGroupAccessResource(collection, accessDetails); + + var context1 = new AuthorizationHandlerContext( + new[] { CollectionGroupOperations.Create }, new ClaimsPrincipal(), addResource); + await sutProvider.Sut.HandleAsync(context1); + + var context2 = new AuthorizationHandlerContext( + new[] { CollectionGroupOperations.Delete }, new ClaimsPrincipal(), removeResource); + await sutProvider.Sut.HandleAsync(context2); + + await sutProvider.GetDependency().Received(1).GetManyByUserIdAsync(Arg.Any()); + } + + [Theory, BitAutoData, CollectionCustomization] + public async Task HandleRequirementAsync_MultipleResources_AllAuthorized_Success( + SutProvider sutProvider, + Guid organizationId, + CollectionAccessDetails accessDetailsA, + CollectionAccessDetails accessDetailsB, + CurrentContextOrganization organization, + Guid userId) + { + organization.Permissions = new Permissions { EditAnyCollection = true }; + sutProvider.GetDependency().UserId.Returns(userId); + sutProvider.GetDependency().GetOrganization(organizationId).Returns(organization); + + var collectionA = new Collection { Id = Guid.NewGuid(), OrganizationId = organizationId }; + var collectionB = new Collection { Id = Guid.NewGuid(), OrganizationId = organizationId }; + var resources = new[] + { + new CollectionGroupAccessResource(collectionA, accessDetailsA), + new CollectionGroupAccessResource(collectionB, accessDetailsB) + }; + + var context = new AuthorizationHandlerContext( + new[] { CollectionGroupOperations.Update }, new ClaimsPrincipal(), resources); + + await sutProvider.Sut.HandleAsync(context); + + Assert.True(context.HasSucceeded); + } + + [Theory, BitAutoData, CollectionCustomization] + public async Task HandleRequirementAsync_MultipleResources_OneUnauthorized_NoSuccess( + SutProvider sutProvider, + Guid organizationId, + CollectionAccessDetails accessDetailsA, + CollectionAccessDetails accessDetailsB, + CurrentContextOrganization organization, + Guid userId) + { + organization.Type = OrganizationUserType.User; + organization.Permissions = new Permissions(); + + var collectionA = new Collection { Id = Guid.NewGuid(), OrganizationId = organizationId }; + var collectionB = new Collection { Id = Guid.NewGuid(), OrganizationId = organizationId }; + + sutProvider.GetDependency().UserId.Returns(userId); + sutProvider.GetDependency().GetOrganization(organizationId).Returns(organization); + sutProvider.GetDependency().ProviderUserForOrgAsync(organizationId).Returns(false); + // Caller manages collectionA (directly) but not collectionB. + sutProvider.GetDependency().GetManyByUserIdAsync(userId) + .Returns(new List { new() { Id = collectionA.Id, Manage = true } }); + + var resources = new[] + { + new CollectionGroupAccessResource(collectionA, accessDetailsA), + new CollectionGroupAccessResource(collectionB, accessDetailsB) + }; + + var context = new AuthorizationHandlerContext( + new[] { CollectionGroupOperations.Update }, new ClaimsPrincipal(), resources); + + await sutProvider.Sut.HandleAsync(context); + + Assert.False(context.HasSucceeded); + } + + [Theory, BitAutoData, CollectionCustomization] + public async Task HandleRequirementAsync_MultipleResources_DifferentOrganizations_Throws( + SutProvider sutProvider, + CollectionAccessDetails accessDetailsA, + CollectionAccessDetails accessDetailsB, + Guid userId) + { + sutProvider.GetDependency().UserId.Returns(userId); + + var collectionA = new Collection { Id = Guid.NewGuid(), OrganizationId = Guid.NewGuid() }; + var collectionB = new Collection { Id = Guid.NewGuid(), OrganizationId = Guid.NewGuid() }; + var resources = new[] + { + new CollectionGroupAccessResource(collectionA, accessDetailsA), + new CollectionGroupAccessResource(collectionB, accessDetailsB) + }; + + var context = new AuthorizationHandlerContext( + new[] { CollectionGroupOperations.Update }, new ClaimsPrincipal(), resources); + + await Assert.ThrowsAsync(() => sutProvider.Sut.HandleAsync(context)); + } +} diff --git a/test/Api.Test/AdminConsole/Authorization/CollectionGroupAuthorizationRulesTests.cs b/test/Api.Test/AdminConsole/Authorization/CollectionGroupAuthorizationRulesTests.cs new file mode 100644 index 000000000000..e2143f39354f --- /dev/null +++ b/test/Api.Test/AdminConsole/Authorization/CollectionGroupAuthorizationRulesTests.cs @@ -0,0 +1,155 @@ +using Bit.Api.AdminConsole.Authorization.Collections; +using Bit.Core.Context; +using Bit.Core.Enums; +using Bit.Core.Models.Data; +using Xunit; + +namespace Bit.Api.Test.AdminConsole.Authorization; + +public class CollectionGroupAuthorizationRulesTests +{ + [Fact] + public void CanModifyGroupAccess_WithEditAnyCollectionPermission_Success() + { + var organization = Organization(OrganizationUserType.Custom, new Permissions { EditAnyCollection = true }); + + var result = CollectionGroupAuthorizationRules.CanModifyGroupAccess( + AccessDetails(anyoneManages: true), organization, + allowAdminAccessToAllCollectionItems: false, callerManagesCollection: false); + + Assert.True(result); + } + + [Theory] + [InlineData(OrganizationUserType.Owner)] + [InlineData(OrganizationUserType.Admin)] + public void CanModifyGroupAccess_WithManageUsersPermission_AllowAdminAccessTrue_Success(OrganizationUserType type) + { + var organization = Organization(type, new Permissions { ManageUsers = true }); + + var result = CollectionGroupAuthorizationRules.CanModifyGroupAccess( + AccessDetails(anyoneManages: true), organization, + allowAdminAccessToAllCollectionItems: true, callerManagesCollection: false); + + Assert.True(result); + } + + [Fact] + public void CanModifyGroupAccess_CustomUserWithManageUsersPermission_AllowAdminAccessFalse_Failure() + { + var organization = Organization(OrganizationUserType.Custom, new Permissions { ManageUsers = true }); + + var result = CollectionGroupAuthorizationRules.CanModifyGroupAccess( + AccessDetails(anyoneManages: true), organization, + allowAdminAccessToAllCollectionItems: false, callerManagesCollection: false); + + Assert.False(result); + } + + [Theory] + [InlineData(OrganizationUserType.Owner)] + [InlineData(OrganizationUserType.Admin)] + public void CanModifyGroupAccess_WhenAdminOrOwner_AllowAdminAccessTrue_Success(OrganizationUserType type) + { + var organization = Organization(type); + + var result = CollectionGroupAuthorizationRules.CanModifyGroupAccess( + AccessDetails(anyoneManages: true), organization, + allowAdminAccessToAllCollectionItems: true, callerManagesCollection: false); + + Assert.True(result); + } + + [Fact] + public void CanModifyGroupAccess_WhenCallerManagesCollection_Success() + { + var organization = Organization(OrganizationUserType.User); + + var result = CollectionGroupAuthorizationRules.CanModifyGroupAccess( + AccessDetails(anyoneManages: true), organization, + allowAdminAccessToAllCollectionItems: false, callerManagesCollection: true); + + Assert.True(result); + } + + [Theory] + [InlineData(OrganizationUserType.Owner)] + [InlineData(OrganizationUserType.Admin)] + public void CanModifyGroupAccess_WhenAdminOrOwner_AllowAdminAccessFalse_OrphanedCollection_Success(OrganizationUserType type) + { + var organization = Organization(type); + + var result = CollectionGroupAuthorizationRules.CanModifyGroupAccess( + AccessDetails(anyoneManages: false), organization, + allowAdminAccessToAllCollectionItems: false, callerManagesCollection: false); + + Assert.True(result); + } + + [Theory] + [InlineData(OrganizationUserType.Owner)] + [InlineData(OrganizationUserType.Admin)] + public void CanModifyGroupAccess_WhenAdminOrOwner_AllowAdminAccessFalse_NotOrphaned_Failure(OrganizationUserType type) + { + var organization = Organization(type); + + var result = CollectionGroupAuthorizationRules.CanModifyGroupAccess( + AccessDetails(anyoneManages: true), organization, + allowAdminAccessToAllCollectionItems: false, callerManagesCollection: false); + + Assert.False(result); + } + + [Theory] + [InlineData(OrganizationUserType.User)] + [InlineData(OrganizationUserType.Custom)] + public void CanModifyGroupAccess_WhenMissingPermissions_Failure(OrganizationUserType type) + { + var organization = Organization(type); + + var result = CollectionGroupAuthorizationRules.CanModifyGroupAccess( + AccessDetails(anyoneManages: true), organization, + allowAdminAccessToAllCollectionItems: true, callerManagesCollection: false); + + Assert.False(result); + } + + [Fact] + public void CanModifyGroupAccess_WhenMissingOrgAccess_Failure() + { + var result = CollectionGroupAuthorizationRules.CanModifyGroupAccess( + AccessDetails(anyoneManages: true), organization: null, + allowAdminAccessToAllCollectionItems: true, callerManagesCollection: false); + + Assert.False(result); + } + + [Fact] + public void CanModifyGroupAccess_OrphanedViaGroupsOnly_WhenAdminOrOwner_AllowAdminAccessFalse_Failure() + { + // A collection with only a managing group (no managing users) is not orphaned. Admins/owners must + // not gain implicit access. + var accessDetails = new CollectionAccessDetails + { + Users = Array.Empty(), + Groups = new[] { new CollectionAccessSelection { Id = Guid.NewGuid(), Manage = true } } + }; + + var result = CollectionGroupAuthorizationRules.CanModifyGroupAccess( + accessDetails, Organization(OrganizationUserType.Admin), + allowAdminAccessToAllCollectionItems: false, callerManagesCollection: false); + + Assert.False(result); + } + + private static CollectionAccessDetails AccessDetails(bool anyoneManages) => new() + { + Users = anyoneManages + ? new[] { new CollectionAccessSelection { Id = Guid.NewGuid(), Manage = true } } + : Array.Empty(), + Groups = Array.Empty() + }; + + private static CurrentContextOrganization Organization(OrganizationUserType type, Permissions permissions = null) => + new() { Type = type, Permissions = permissions ?? new Permissions() }; +} 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/util/Migrator/DbScripts/2026-08-04_00_AddCollectionGroupDeleteMany.sql b/util/Migrator/DbScripts/2026-08-04_00_AddCollectionGroupDeleteMany.sql new file mode 100644 index 000000000000..ab830fc94649 --- /dev/null +++ b/util/Migrator/DbScripts/2026-08-04_00_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