[AC-2170] Group modal - limit admin access - collections tab (#3998)

* Update GroupsController POST and PUT to respect collection management settings

src/Api/AdminConsole/Controllers/GroupsController.cs

@@ -2,6 +2,7 @@
 using Bit.Api.AdminConsole.Models.Response;
 using Bit.Api.Models.Response;
 using Bit.Api.Utilities;
+using Bit.Api.Vault.AuthorizationHandlers.Collections;
 using Bit.Api.Vault.AuthorizationHandlers.Groups;
 using Bit.Core;
 using Bit.Core.AdminConsole.OrganizationFeatures.Groups.Interfaces;
@@ -32,6 +33,7 @@ public class GroupsController : Controller
     private readonly IUserService _userService;
     private readonly IFeatureService _featureService;
     private readonly IOrganizationUserRepository _organizationUserRepository;
+    private readonly ICollectionRepository _collectionRepository;
 
     public GroupsController(
         IGroupRepository groupRepository,
@@ -45,7 +47,8 @@ public class GroupsController : Controller
         IApplicationCacheService applicationCacheService,
         IUserService userService,
         IFeatureService featureService,
-        IOrganizationUserRepository organizationUserRepository)
+        IOrganizationUserRepository organizationUserRepository,
+        ICollectionRepository collectionRepository)
     {
         _groupRepository = groupRepository;
         _groupService = groupService;
@@ -59,6 +62,7 @@ public class GroupsController : Controller
         _userService = userService;
         _featureService = featureService;
         _organizationUserRepository = organizationUserRepository;
+        _collectionRepository = collectionRepository;
     }
 
     [HttpGet("{id}")]
@@ -125,16 +129,28 @@ public class GroupsController : Controller
     }
 
     [HttpPost("")]
-    public async Task<GroupResponseModel> Post(string orgId, [FromBody] GroupRequestModel model)
+    public async Task<GroupResponseModel> Post(Guid orgId, [FromBody] GroupRequestModel model)
     {
-        var orgIdGuid = new Guid(orgId);
-        if (!await _currentContext.ManageGroups(orgIdGuid))
+        if (!await _currentContext.ManageGroups(orgId))
         {
             throw new NotFoundException();
         }
 
-        var organization = await _organizationRepository.GetByIdAsync(orgIdGuid);
-        var group = model.ToGroup(orgIdGuid);
+        // Flexible Collections - check the user has permission to grant access to the collections for the new group
+        if (await FlexibleCollectionsIsEnabledAsync(orgId) && _featureService.IsEnabled(FeatureFlagKeys.FlexibleCollectionsV1))
+        {
+            var collections = await _collectionRepository.GetManyByManyIdsAsync(model.Collections.Select(a => a.Id));
+            var authorized =
+                (await _authorizationService.AuthorizeAsync(User, collections, BulkCollectionOperations.ModifyGroupAccess))
+                .Succeeded;
+            if (!authorized)
+            {
+                throw new NotFoundException("You are not authorized to grant access to these collections.");
+            }
+        }
+
+        var organization = await _organizationRepository.GetByIdAsync(orgId);
+        var group = model.ToGroup(orgId);
         await _createGroupCommand.CreateGroupAsync(group, organization, model.Collections?.Select(c => c.ToSelectionReadOnly()).ToList(), model.Users);
 
         return new GroupResponseModel(group);
@@ -144,16 +160,40 @@ public class GroupsController : Controller
     [HttpPost("{id}")]
     public async Task<GroupResponseModel> Put(Guid orgId, Guid id, [FromBody] GroupRequestModel model)
     {
+        if (await FlexibleCollectionsIsEnabledAsync(orgId) && _featureService.IsEnabled(FeatureFlagKeys.FlexibleCollectionsV1))
+        {
+            // Use new Flexible Collections v1 logic
+            return await Put_vNext(orgId, id, model);
+        }
+
+        // Pre-Flexible Collections v1 logic follows
         var group = await _groupRepository.GetByIdAsync(id);
         if (group == null || !await _currentContext.ManageGroups(group.OrganizationId))
         {
             throw new NotFoundException();
         }
 
-        // Flexible Collections v1 - a user may not be able to add themselves to a group
+        var organization = await _organizationRepository.GetByIdAsync(orgId);
+
+        await _updateGroupCommand.UpdateGroupAsync(model.ToGroup(group), organization,
+            model.Collections.Select(c => c.ToSelectionReadOnly()).ToList(), model.Users);
+        return new GroupResponseModel(group);
+    }
+
+    /// <summary>
+    /// Put logic for Flexible Collections v1
+    /// </summary>
+    private async Task<GroupResponseModel> Put_vNext(Guid orgId, Guid id, [FromBody] GroupRequestModel model)
+    {
+        var (group, currentAccess) = await _groupRepository.GetByIdWithCollectionsAsync(id);
+        if (group == null || !await _currentContext.ManageGroups(group.OrganizationId))
+        {
+            throw new NotFoundException();
+        }
+
+        // Check whether the user is permitted to add themselves to the group
         var orgAbility = await _applicationCacheService.GetOrganizationAbilityAsync(orgId);
-        var flexibleCollectionsV1Enabled = _featureService.IsEnabled(FeatureFlagKeys.FlexibleCollectionsV1);
-        if (flexibleCollectionsV1Enabled && orgAbility.FlexibleCollections && !orgAbility.AllowAdminAccessToAllCollectionItems)
+        if (!orgAbility.AllowAdminAccessToAllCollectionItems)
         {
             var userId = _userService.GetProperUserId(User).Value;
             var organizationUser = await _organizationUserRepository.GetByOrganizationAsync(orgId, userId);
@@ -164,9 +204,38 @@ public class GroupsController : Controller
             }
         }
 
+        // The client only sends collections that the saving user has permissions to edit.
+        // On the server side, we need to (1) confirm this and (2) concat these with the collections that the user
+        // can't edit before saving to the database.
+        var currentCollections = await _collectionRepository
+            .GetManyByManyIdsAsync(currentAccess.Select(cas => cas.Id));
+
+        var readonlyCollectionIds = new HashSet<Guid>();
+        foreach (var collection in currentCollections)
+        {
+            if (!(await _authorizationService.AuthorizeAsync(User, collection, BulkCollectionOperations.ModifyGroupAccess))
+                .Succeeded)
+            {
+                readonlyCollectionIds.Add(collection.Id);
+            }
+        }
+
+        if (model.Collections.Any(c => readonlyCollectionIds.Contains(c.Id)))
+        {
+            throw new BadRequestException("You must have Can Manage permissions to edit a collection's membership");
+        }
+
+        var editedCollectionAccess = model.Collections
+            .Select(c => c.ToSelectionReadOnly());
+        var readonlyCollectionAccess = currentAccess
+            .Where(ca => readonlyCollectionIds.Contains(ca.Id));
+        var collectionsToSave = editedCollectionAccess
+            .Concat(readonlyCollectionAccess)
+            .ToList();
+
         var organization = await _organizationRepository.GetByIdAsync(orgId);
 
-        await _updateGroupCommand.UpdateGroupAsync(model.ToGroup(group), organization, model.Collections?.Select(c => c.ToSelectionReadOnly()).ToList(), model.Users);
+        await _updateGroupCommand.UpdateGroupAsync(model.ToGroup(group), organization, collectionsToSave, model.Users);
         return new GroupResponseModel(group);
     }
 

src/Api/Vault/AuthorizationHandlers/Collections/CollectionAuthorizationHandler.cs

@@ -85,7 +85,8 @@ public class CollectionAuthorizationHandler : AuthorizationHandler<CollectionOpe
         { Type: OrganizationUserType.Owner or OrganizationUserType.Admin } or
         { Permissions.EditAnyCollection: true } or
         { Permissions.DeleteAnyCollection: true } or
-        { Permissions.ManageUsers: true })
+        { Permissions.ManageUsers: true } or
+        { Permissions.ManageGroups: true })
         {
             context.Succeed(requirement);
             return;

test/Api.Test/AdminConsole/Controllers/GroupsControllerTests.cs

@@ -1,6 +1,8 @@
 using System.Security.Claims;
 using Bit.Api.AdminConsole.Controllers;
 using Bit.Api.AdminConsole.Models.Request;
+using Bit.Api.Models.Request;
+using Bit.Api.Vault.AuthorizationHandlers.Collections;
 using Bit.Core;
 using Bit.Core.AdminConsole.Entities;
 using Bit.Core.AdminConsole.OrganizationFeatures.Groups.Interfaces;
@@ -12,8 +14,10 @@ using Bit.Core.Models.Data;
 using Bit.Core.Models.Data.Organizations;
 using Bit.Core.Repositories;
 using Bit.Core.Services;
+using Bit.Core.Utilities;
 using Bit.Test.Common.AutoFixture;
 using Bit.Test.Common.AutoFixture.Attributes;
+using Microsoft.AspNetCore.Authorization;
 using NSubstitute;
 using Xunit;
 
@@ -25,12 +29,12 @@ public class GroupsControllerTests
 {
     [Theory]
     [BitAutoData]
-    public async Task Post_Success(Organization organization, GroupRequestModel groupRequestModel, SutProvider<GroupsController> sutProvider)
+    public async Task Post_PreFCv1_Success(Organization organization, GroupRequestModel groupRequestModel, SutProvider<GroupsController> sutProvider)
     {
         sutProvider.GetDependency<IOrganizationRepository>().GetByIdAsync(organization.Id).Returns(organization);
         sutProvider.GetDependency<ICurrentContext>().ManageGroups(organization.Id).Returns(true);
 
-        var response = await sutProvider.Sut.Post(organization.Id.ToString(), groupRequestModel);
+        var response = await sutProvider.Sut.Post(organization.Id, groupRequestModel);
 
         await sutProvider.GetDependency<ICurrentContext>().Received(1).ManageGroups(organization.Id);
         await sutProvider.GetDependency<ICreateGroupCommand>().Received(1).CreateGroupAsync(
@@ -47,15 +51,100 @@ public class GroupsControllerTests
 
     [Theory]
     [BitAutoData]
-    public async Task Put_AdminsCanAccessAllCollections_Success(Organization organization, Group group, GroupRequestModel groupRequestModel, SutProvider<GroupsController> sutProvider)
+    public async Task Post_AuthorizedToGiveAccessToCollections_Success(Organization organization,
+        GroupRequestModel groupRequestModel, SutProvider<GroupsController> sutProvider)
     {
-        group.OrganizationId = organization.Id;
+        // Enable FC and v1
+        sutProvider.GetDependency<IApplicationCacheService>().GetOrganizationAbilityAsync(organization.Id).Returns(
+            new OrganizationAbility { Id = organization.Id, FlexibleCollections = true, AllowAdminAccessToAllCollectionItems = false });
+        sutProvider.GetDependency<IFeatureService>().IsEnabled(FeatureFlagKeys.FlexibleCollectionsV1).Returns(true);
+
+        sutProvider.GetDependency<IAuthorizationService>()
+            .AuthorizeAsync(Arg.Any<ClaimsPrincipal>(),
+                 Arg.Any<IEnumerable<Collection>>(),
+                 Arg.Is<IEnumerable<IAuthorizationRequirement>>(reqs => reqs.Contains(BulkCollectionOperations.ModifyGroupAccess)))
+             .Returns(AuthorizationResult.Success());
 
         sutProvider.GetDependency<IOrganizationRepository>().GetByIdAsync(organization.Id).Returns(organization);
-        sutProvider.GetDependency<IGroupRepository>().GetByIdAsync(group.Id).Returns(group);
         sutProvider.GetDependency<ICurrentContext>().ManageGroups(organization.Id).Returns(true);
+
+        var response = await sutProvider.Sut.Post(organization.Id, groupRequestModel);
+
+        var requestModelCollectionIds = groupRequestModel.Collections.Select(c => c.Id).ToHashSet();
+
+        // Assert that it checked permissions
+        await sutProvider.GetDependency<ICurrentContext>().Received(1).ManageGroups(organization.Id);
+        await sutProvider.GetDependency<IAuthorizationService>()
+            .Received(1)
+            .AuthorizeAsync(Arg.Any<ClaimsPrincipal>(),
+                Arg.Is<IEnumerable<Collection>>(collections =>
+                    collections.All(c => requestModelCollectionIds.Contains(c.Id))),
+                Arg.Is<IEnumerable<IAuthorizationRequirement>>(reqs =>
+                    reqs.Single() == BulkCollectionOperations.ModifyGroupAccess));
+
+        // Assert that it saved the data
+        await sutProvider.GetDependency<ICreateGroupCommand>().Received(1).CreateGroupAsync(
+            Arg.Is<Group>(g =>
+                g.OrganizationId == organization.Id && g.Name == groupRequestModel.Name &&
+                g.AccessAll == groupRequestModel.AccessAll),
+            organization,
+            Arg.Is<ICollection<CollectionAccessSelection>>(access =>
+                access.All(c => requestModelCollectionIds.Contains(c.Id))),
+            Arg.Any<IEnumerable<Guid>>());
+        Assert.Equal(groupRequestModel.Name, response.Name);
+        Assert.Equal(organization.Id, response.OrganizationId);
+        Assert.Equal(groupRequestModel.AccessAll, response.AccessAll);
+    }
+
+    [Theory]
+    [BitAutoData]
+    public async Task Post_NotAuthorizedToGiveAccessToCollections_Throws(Organization organization, GroupRequestModel groupRequestModel, SutProvider<GroupsController> sutProvider)
+    {
+        // Enable FC and v1
         sutProvider.GetDependency<IApplicationCacheService>().GetOrganizationAbilityAsync(organization.Id).Returns(
-            new OrganizationAbility { Id = organization.Id, AllowAdminAccessToAllCollectionItems = true });
+            new OrganizationAbility { Id = organization.Id, FlexibleCollections = true, AllowAdminAccessToAllCollectionItems = false });
+        sutProvider.GetDependency<IFeatureService>().IsEnabled(FeatureFlagKeys.FlexibleCollectionsV1).Returns(true);
+
+        sutProvider.GetDependency<IOrganizationRepository>().GetByIdAsync(organization.Id).Returns(organization);
+        sutProvider.GetDependency<ICurrentContext>().ManageGroups(organization.Id).Returns(true);
+
+        var requestModelCollectionIds = groupRequestModel.Collections.Select(c => c.Id).ToHashSet();
+        sutProvider.GetDependency<IAuthorizationService>()
+           .AuthorizeAsync(Arg.Any<ClaimsPrincipal>(),
+                Arg.Is<IEnumerable<Collection>>(collections => collections.All(c => requestModelCollectionIds.Contains(c.Id))),
+                Arg.Is<IEnumerable<IAuthorizationRequirement>>(reqs => reqs.Contains(BulkCollectionOperations.ModifyGroupAccess)))
+            .Returns(AuthorizationResult.Failed());
+
+        var exception = await Assert.ThrowsAsync<NotFoundException>(() => sutProvider.Sut.Post(organization.Id, groupRequestModel));
+
+        Assert.Contains("You are not authorized to grant access to these collections.", exception.Message);
+
+        await sutProvider.GetDependency<ICreateGroupCommand>().DidNotReceiveWithAnyArgs()
+            .CreateGroupAsync(default, default, default, default);
+    }
+
+    [Theory]
+    [BitAutoData]
+    public async Task Put_AdminsCanAccessAllCollections_Success(Organization organization, Group group,
+        GroupRequestModel groupRequestModel, List<CollectionAccessSelection> existingCollectionAccess,
+        SutProvider<GroupsController> sutProvider)
+    {
+        group.OrganizationId = organization.Id;
+
+        // Enable FC and v1, set Collection Management Setting
+        sutProvider.GetDependency<IApplicationCacheService>().GetOrganizationAbilityAsync(organization.Id).Returns(
+            new OrganizationAbility { Id = organization.Id, AllowAdminAccessToAllCollectionItems = true, FlexibleCollections = true });
+        sutProvider.GetDependency<IFeatureService>().IsEnabled(FeatureFlagKeys.FlexibleCollectionsV1).Returns(true);
+
+        sutProvider.GetDependency<IOrganizationRepository>().GetByIdAsync(organization.Id).Returns(organization);
+        sutProvider.GetDependency<IGroupRepository>().GetByIdWithCollectionsAsync(group.Id)
+            .Returns(new Tuple<Group, ICollection<CollectionAccessSelection>>(group, existingCollectionAccess));
+        sutProvider.GetDependency<ICollectionRepository>()
+            .GetManyByManyIdsAsync(existingCollectionAccess.Select(c => c.Id))
+            .Returns(existingCollectionAccess.Select(c => new Collection { Id = c.Id }).ToList());
+        sutProvider.GetDependency<ICurrentContext>().ManageGroups(organization.Id).Returns(true);
+
+        var requestModelCollectionIds = groupRequestModel.Collections.Select(c => c.Id).ToHashSet();
 
         var response = await sutProvider.Sut.Put(organization.Id, group.Id, groupRequestModel);
 
@@ -65,7 +154,9 @@ public class GroupsControllerTests
                 g.OrganizationId == organization.Id && g.Name == groupRequestModel.Name &&
                 g.AccessAll == groupRequestModel.AccessAll),
             Arg.Is<Organization>(o => o.Id == organization.Id),
-            Arg.Any<ICollection<CollectionAccessSelection>>(),
+            // Should overwrite any existing collections
+            Arg.Is<ICollection<CollectionAccessSelection>>(access =>
+                access.All(c => requestModelCollectionIds.Contains(c.Id))),
             Arg.Any<IEnumerable<Guid>>());
         Assert.Equal(groupRequestModel.Name, response.Name);
         Assert.Equal(organization.Id, response.OrganizationId);
@@ -74,20 +165,13 @@ public class GroupsControllerTests
 
     [Theory]
     [BitAutoData]
-    public async Task Put_AdminsCannotAccessAllCollections_CannotAddSelfToGroup(Organization organization, Group group,
+    public async Task Put_UpdateMembers_AdminsCannotAccessAllCollections_CannotAddSelfToGroup(Organization organization, Group group,
         GroupRequestModel groupRequestModel, OrganizationUser savingOrganizationUser, List<Guid> currentGroupUsers,
         SutProvider<GroupsController> sutProvider)
     {
         group.OrganizationId = organization.Id;
 
-        // Saving user is trying to add themselves to the group
-        var updatedUsers = groupRequestModel.Users.ToList();
-        updatedUsers.Add(savingOrganizationUser.Id);
-        groupRequestModel.Users = updatedUsers;
-
-        sutProvider.GetDependency<IOrganizationRepository>().GetByIdAsync(organization.Id).Returns(organization);
-        sutProvider.GetDependency<IGroupRepository>().GetByIdAsync(group.Id).Returns(group);
-        sutProvider.GetDependency<ICurrentContext>().ManageGroups(organization.Id).Returns(true);
+        // Enable FC and v1
         sutProvider.GetDependency<IApplicationCacheService>().GetOrganizationAbilityAsync(organization.Id).Returns(
             new OrganizationAbility
             {
@@ -96,6 +180,16 @@ public class GroupsControllerTests
                 FlexibleCollections = true
             });
         sutProvider.GetDependency<IFeatureService>().IsEnabled(FeatureFlagKeys.FlexibleCollectionsV1).Returns(true);
+
+        // Saving user is trying to add themselves to the group
+        var updatedUsers = groupRequestModel.Users.ToList();
+        updatedUsers.Add(savingOrganizationUser.Id);
+        groupRequestModel.Users = updatedUsers;
+
+        sutProvider.GetDependency<IOrganizationRepository>().GetByIdAsync(organization.Id).Returns(organization);
+        sutProvider.GetDependency<IGroupRepository>().GetByIdWithCollectionsAsync(group.Id)
+            .Returns(new Tuple<Group, ICollection<CollectionAccessSelection>>(group, new List<CollectionAccessSelection>()));
+        sutProvider.GetDependency<ICurrentContext>().ManageGroups(organization.Id).Returns(true);
         sutProvider.GetDependency<IOrganizationUserRepository>()
             .GetByOrganizationAsync(organization.Id, Arg.Any<Guid>())
                 .Returns(savingOrganizationUser);
@@ -112,12 +206,22 @@ public class GroupsControllerTests
 
     [Theory]
     [BitAutoData]
-    public async Task Put_AdminsCannotAccessAllCollections_Success(Organization organization, Group group,
+    public async Task Put_UpdateMembers_AdminsCannotAccessAllCollections_AlreadyInGroup_Success(Organization organization, Group group,
         GroupRequestModel groupRequestModel, OrganizationUser savingOrganizationUser, List<Guid> currentGroupUsers,
         SutProvider<GroupsController> sutProvider)
     {
         group.OrganizationId = organization.Id;
 
+        // Enable FC and v1
+        sutProvider.GetDependency<IApplicationCacheService>().GetOrganizationAbilityAsync(organization.Id).Returns(
+            new OrganizationAbility
+            {
+                Id = organization.Id,
+                AllowAdminAccessToAllCollectionItems = false,
+                FlexibleCollections = true
+            });
+        sutProvider.GetDependency<IFeatureService>().IsEnabled(FeatureFlagKeys.FlexibleCollectionsV1).Returns(true);
+
         // Saving user is trying to add themselves to the group
         var updatedUsers = groupRequestModel.Users.ToList();
         updatedUsers.Add(savingOrganizationUser.Id);
@@ -127,16 +231,9 @@ public class GroupsControllerTests
         currentGroupUsers.Add(savingOrganizationUser.Id);
 
         sutProvider.GetDependency<IOrganizationRepository>().GetByIdAsync(organization.Id).Returns(organization);
-        sutProvider.GetDependency<IGroupRepository>().GetByIdAsync(group.Id).Returns(group);
+        sutProvider.GetDependency<IGroupRepository>().GetByIdWithCollectionsAsync(group.Id)
+            .Returns(new Tuple<Group, ICollection<CollectionAccessSelection>>(group, new List<CollectionAccessSelection>()));
         sutProvider.GetDependency<ICurrentContext>().ManageGroups(organization.Id).Returns(true);
-        sutProvider.GetDependency<IApplicationCacheService>().GetOrganizationAbilityAsync(organization.Id).Returns(
-            new OrganizationAbility
-            {
-                Id = organization.Id,
-                AllowAdminAccessToAllCollectionItems = false,
-                FlexibleCollections = true
-            });
-        sutProvider.GetDependency<IFeatureService>().IsEnabled(FeatureFlagKeys.FlexibleCollectionsV1).Returns(true);
         sutProvider.GetDependency<IOrganizationUserRepository>()
             .GetByOrganizationAsync(organization.Id, Arg.Any<Guid>())
                 .Returns(savingOrganizationUser);
@@ -145,6 +242,9 @@ public class GroupsControllerTests
         sutProvider.GetDependency<IGroupRepository>().GetManyUserIdsByIdAsync(group.Id)
             .Returns(currentGroupUsers);
 
+        // Make collection authorization pass, it's not being tested here
+        groupRequestModel.Collections = Array.Empty<SelectionReadOnlyRequestModel>();
+
         var response = await sutProvider.Sut.Put(organization.Id, group.Id, groupRequestModel);
 
         await sutProvider.GetDependency<ICurrentContext>().Received(1).ManageGroups(organization.Id);
@@ -159,4 +259,143 @@ public class GroupsControllerTests
         Assert.Equal(organization.Id, response.OrganizationId);
         Assert.Equal(groupRequestModel.AccessAll, response.AccessAll);
     }
+
+    [Theory]
+    [BitAutoData]
+    public async Task Put_UpdateCollections_OnlyUpdatesCollectionsTheSavingUserCanUpdate(GroupRequestModel groupRequestModel,
+        Group group, Organization organization,
+        SutProvider<GroupsController> sutProvider, Guid savingUserId)
+    {
+        organization.FlexibleCollections = true;
+        sutProvider.GetDependency<IFeatureService>().IsEnabled(FeatureFlagKeys.FlexibleCollectionsV1).Returns(true);
+        Put_Setup(sutProvider, organization, group, savingUserId);
+
+        var editedCollectionId = CoreHelpers.GenerateComb();
+        var readonlyCollectionId1 = CoreHelpers.GenerateComb();
+        var readonlyCollectionId2 = CoreHelpers.GenerateComb();
+
+        var currentCollectionAccess = new List<CollectionAccessSelection>
+        {
+            new()
+            {
+                Id = editedCollectionId,
+                HidePasswords = true,
+                Manage = false,
+                ReadOnly = true
+            },
+            new()
+            {
+                Id = readonlyCollectionId1,
+                HidePasswords = false,
+                Manage = true,
+                ReadOnly = false
+            },
+            new()
+            {
+                Id = readonlyCollectionId2,
+                HidePasswords = false,
+                Manage = false,
+                ReadOnly = false
+            },
+        };
+
+        // User is upgrading editedCollectionId to manage
+        groupRequestModel.Collections = new List<SelectionReadOnlyRequestModel>
+        {
+            new() { Id = editedCollectionId, HidePasswords = false, Manage = true, ReadOnly = false }
+        };
+
+        sutProvider.GetDependency<IGroupRepository>()
+            .GetByIdWithCollectionsAsync(group.Id)
+            .Returns(new Tuple<Group, ICollection<CollectionAccessSelection>>(group,
+                currentCollectionAccess));
+
+        var currentCollections = currentCollectionAccess
+            .Select(cas => new Collection { Id = cas.Id }).ToList();
+        sutProvider.GetDependency<ICollectionRepository>()
+            .GetManyByManyIdsAsync(Arg.Any<IEnumerable<Guid>>())
+            .Returns(currentCollections);
+
+        // Authorize the editedCollection
+        sutProvider.GetDependency<IAuthorizationService>()
+            .AuthorizeAsync(Arg.Any<ClaimsPrincipal>(), Arg.Is<Collection>(c => c.Id == editedCollectionId),
+                Arg.Is<IEnumerable<IAuthorizationRequirement>>(reqs => reqs.Contains(BulkCollectionOperations.ModifyGroupAccess)))
+            .Returns(AuthorizationResult.Success());
+
+        // Do not authorize the readonly collections
+        sutProvider.GetDependency<IAuthorizationService>()
+            .AuthorizeAsync(Arg.Any<ClaimsPrincipal>(), Arg.Is<Collection>(c => c.Id == readonlyCollectionId1 || c.Id == readonlyCollectionId2),
+                Arg.Is<IEnumerable<IAuthorizationRequirement>>(reqs => reqs.Contains(BulkCollectionOperations.ModifyGroupAccess)))
+            .Returns(AuthorizationResult.Failed());
+
+        var response = await sutProvider.Sut.Put(organization.Id, group.Id, groupRequestModel);
+
+        // Expect all collection access (modified and unmodified) to be saved
+        await sutProvider.GetDependency<ICurrentContext>().Received(1).ManageGroups(organization.Id);
+        await sutProvider.GetDependency<IUpdateGroupCommand>().Received(1).UpdateGroupAsync(
+            Arg.Is<Group>(g =>
+                g.OrganizationId == organization.Id && g.Name == groupRequestModel.Name &&
+                g.AccessAll == groupRequestModel.AccessAll),
+            Arg.Is<Organization>(o => o.Id == organization.Id),
+            Arg.Is<List<CollectionAccessSelection>>(cas =>
+                cas.Select(c => c.Id).SequenceEqual(currentCollectionAccess.Select(c => c.Id)) &&
+                cas.First(c => c.Id == editedCollectionId).Manage == true &&
+                cas.First(c => c.Id == editedCollectionId).ReadOnly == false &&
+                cas.First(c => c.Id == editedCollectionId).HidePasswords == false),
+            Arg.Any<IEnumerable<Guid>>());
+        Assert.Equal(groupRequestModel.Name, response.Name);
+        Assert.Equal(organization.Id, response.OrganizationId);
+        Assert.Equal(groupRequestModel.AccessAll, response.AccessAll);
+    }
+
+    [Theory]
+    [BitAutoData]
+    public async Task Put_UpdateCollections_ThrowsIfSavingUserCannotUpdateCollections(GroupRequestModel groupRequestModel,
+        Group group, Organization organization,
+        SutProvider<GroupsController> sutProvider, Guid savingUserId)
+    {
+        organization.FlexibleCollections = true;
+        sutProvider.GetDependency<IFeatureService>().IsEnabled(FeatureFlagKeys.FlexibleCollectionsV1).Returns(true);
+        Put_Setup(sutProvider, organization, group, savingUserId);
+
+        sutProvider.GetDependency<IGroupRepository>()
+            .GetByIdWithCollectionsAsync(group.Id)
+            .Returns(new Tuple<Group, ICollection<CollectionAccessSelection>>(group,
+                groupRequestModel.Collections.Select(cas => cas.ToSelectionReadOnly()).ToList()));
+        var collections = groupRequestModel.Collections.Select(cas => new Collection { Id = cas.Id }).ToList();
+        sutProvider.GetDependency<ICollectionRepository>()
+            .GetManyByManyIdsAsync(Arg.Is<IEnumerable<Guid>>(guids => guids.SequenceEqual(collections.Select(c => c.Id))))
+            .Returns(collections);
+
+        sutProvider.GetDependency<IAuthorizationService>()
+            .AuthorizeAsync(Arg.Any<ClaimsPrincipal>(), Arg.Is<Collection>(c => collections.Contains(c)),
+                Arg.Is<IEnumerable<IAuthorizationRequirement>>(reqs => reqs.Contains(BulkCollectionOperations.ModifyGroupAccess)))
+            .Returns(AuthorizationResult.Failed());
+
+        var exception = await Assert.ThrowsAsync<BadRequestException>(() => sutProvider.Sut.Put(organization.Id, group.Id, groupRequestModel));
+        Assert.Contains("You must have Can Manage permission", exception.Message);
+    }
+
+    private void Put_Setup(SutProvider<GroupsController> sutProvider, Organization organization,
+        Group group, Guid savingUserId)
+    {
+        var orgId = organization.Id = group.OrganizationId;
+
+        sutProvider.GetDependency<ICurrentContext>().ManageGroups(orgId).Returns(true);
+        sutProvider.GetDependency<IApplicationCacheService>().GetOrganizationAbilityAsync(orgId)
+            .Returns(new OrganizationAbility
+            {
+                Id = organization.Id,
+                FlexibleCollections = true,
+                AllowAdminAccessToAllCollectionItems = false
+            });
+
+        sutProvider.GetDependency<IGroupRepository>().GetManyUserIdsByIdAsync(group.Id).Returns(new List<Guid>());
+        sutProvider.GetDependency<IUserService>().GetProperUserId(Arg.Any<ClaimsPrincipal>()).Returns(savingUserId);
+        sutProvider.GetDependency<IOrganizationUserRepository>().GetByOrganizationAsync(orgId, savingUserId).Returns(new OrganizationUser
+        {
+            Id = savingUserId
+        });
+        sutProvider.GetDependency<IOrganizationRepository>().GetByIdAsync(organization.Id).Returns(organization);
+    }
 }