[AC-2169] Group modal - limit admin access - members tab (#3975)

* Prevent Admins from adding themselves to groups
if they cannot manage all collections and items

src/Api/AdminConsole/Controllers/GroupsController.cs

@@ -3,6 +3,7 @@ using Bit.Api.AdminConsole.Models.Response;
 using Bit.Api.Models.Response;
 using Bit.Api.Utilities;
 using Bit.Api.Vault.AuthorizationHandlers.Groups;
+using Bit.Core;
 using Bit.Core.AdminConsole.OrganizationFeatures.Groups.Interfaces;
 using Bit.Core.AdminConsole.Repositories;
 using Bit.Core.AdminConsole.Services;
@@ -28,6 +29,9 @@ public class GroupsController : Controller
     private readonly IUpdateGroupCommand _updateGroupCommand;
     private readonly IAuthorizationService _authorizationService;
     private readonly IApplicationCacheService _applicationCacheService;
+    private readonly IUserService _userService;
+    private readonly IFeatureService _featureService;
+    private readonly IOrganizationUserRepository _organizationUserRepository;
 
     public GroupsController(
         IGroupRepository groupRepository,
@@ -38,7 +42,10 @@ public class GroupsController : Controller
         IUpdateGroupCommand updateGroupCommand,
         IDeleteGroupCommand deleteGroupCommand,
         IAuthorizationService authorizationService,
-        IApplicationCacheService applicationCacheService)
+        IApplicationCacheService applicationCacheService,
+        IUserService userService,
+        IFeatureService featureService,
+        IOrganizationUserRepository organizationUserRepository)
     {
         _groupRepository = groupRepository;
         _groupService = groupService;
@@ -49,6 +56,9 @@ public class GroupsController : Controller
         _deleteGroupCommand = deleteGroupCommand;
         _authorizationService = authorizationService;
         _applicationCacheService = applicationCacheService;
+        _userService = userService;
+        _featureService = featureService;
+        _organizationUserRepository = organizationUserRepository;
     }
 
     [HttpGet("{id}")]
@@ -132,32 +142,34 @@ public class GroupsController : Controller
 
     [HttpPut("{id}")]
     [HttpPost("{id}")]
-    public async Task<GroupResponseModel> Put(string orgId, string id, [FromBody] GroupRequestModel model)
+    public async Task<GroupResponseModel> Put(Guid orgId, Guid id, [FromBody] GroupRequestModel model)
     {
-        var group = await _groupRepository.GetByIdAsync(new Guid(id));
+        var group = await _groupRepository.GetByIdAsync(id);
         if (group == null || !await _currentContext.ManageGroups(group.OrganizationId))
         {
             throw new NotFoundException();
         }
 
-        var orgIdGuid = new Guid(orgId);
+        // Flexible Collections v1 - a user may not be able to add themselves to a group
-        var organization = await _organizationRepository.GetByIdAsync(orgIdGuid);
+        var orgAbility = await _applicationCacheService.GetOrganizationAbilityAsync(orgId);
+        var flexibleCollectionsV1Enabled = _featureService.IsEnabled(FeatureFlagKeys.FlexibleCollectionsV1);
+        if (flexibleCollectionsV1Enabled && orgAbility.FlexibleCollections && !orgAbility.AllowAdminAccessToAllCollectionItems)
+        {
+            var userId = _userService.GetProperUserId(User).Value;
+            var organizationUser = await _organizationUserRepository.GetByOrganizationAsync(orgId, userId);
+            var currentGroupUsers = await _groupRepository.GetManyUserIdsByIdAsync(id);
+            if (!currentGroupUsers.Contains(organizationUser.Id) && model.Users.Contains(organizationUser.Id))
+            {
+                throw new BadRequestException("You cannot add yourself to groups.");
+            }
+        }
+
+        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);
     }
 
-    [HttpPut("{id}/users")]
-    public async Task PutUsers(string orgId, string id, [FromBody] IEnumerable<Guid> model)
-    {
-        var group = await _groupRepository.GetByIdAsync(new Guid(id));
-        if (group == null || !await _currentContext.ManageGroups(group.OrganizationId))
-        {
-            throw new NotFoundException();
-        }
-        await _groupRepository.UpdateUsersAsync(group.Id, model);
-    }
-
     [HttpDelete("{id}")]
     [HttpPost("{id}/delete")]
     public async Task Delete(string orgId, string id)

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

@@ -1,11 +1,17 @@
-using Bit.Api.AdminConsole.Controllers;
+using System.Security.Claims;
+using Bit.Api.AdminConsole.Controllers;
 using Bit.Api.AdminConsole.Models.Request;
+using Bit.Core;
 using Bit.Core.AdminConsole.Entities;
 using Bit.Core.AdminConsole.OrganizationFeatures.Groups.Interfaces;
 using Bit.Core.AdminConsole.Repositories;
 using Bit.Core.Context;
+using Bit.Core.Entities;
+using Bit.Core.Exceptions;
 using Bit.Core.Models.Data;
+using Bit.Core.Models.Data.Organizations;
 using Bit.Core.Repositories;
+using Bit.Core.Services;
 using Bit.Test.Common.AutoFixture;
 using Bit.Test.Common.AutoFixture.Attributes;
 using NSubstitute;
@@ -41,15 +47,105 @@ public class GroupsControllerTests
 
     [Theory]
     [BitAutoData]
-    public async Task Put_Success(Organization organization, Group group, GroupRequestModel groupRequestModel, SutProvider<GroupsController> sutProvider)
+    public async Task Put_AdminsCanAccessAllCollections_Success(Organization organization, Group group, GroupRequestModel groupRequestModel, SutProvider<GroupsController> sutProvider)
     {
         group.OrganizationId = organization.Id;
 
         sutProvider.GetDependency<IOrganizationRepository>().GetByIdAsync(organization.Id).Returns(organization);
         sutProvider.GetDependency<IGroupRepository>().GetByIdAsync(group.Id).Returns(group);
         sutProvider.GetDependency<ICurrentContext>().ManageGroups(organization.Id).Returns(true);
+        sutProvider.GetDependency<IApplicationCacheService>().GetOrganizationAbilityAsync(organization.Id).Returns(
+            new OrganizationAbility { Id = organization.Id, AllowAdminAccessToAllCollectionItems = true });
 
-        var response = await sutProvider.Sut.Put(organization.Id.ToString(), group.Id.ToString(), groupRequestModel);
+        var response = await sutProvider.Sut.Put(organization.Id, group.Id, groupRequestModel);
+
+        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.Any<ICollection<CollectionAccessSelection>>(),
+            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_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);
+        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);
+        sutProvider.GetDependency<IUserService>().GetProperUserId(Arg.Any<ClaimsPrincipal>())
+            .Returns(savingOrganizationUser.UserId);
+        sutProvider.GetDependency<IGroupRepository>().GetManyUserIdsByIdAsync(group.Id)
+            .Returns(currentGroupUsers);
+
+        var exception = await
+            Assert.ThrowsAsync<BadRequestException>(() => sutProvider.Sut.Put(organization.Id, group.Id, groupRequestModel));
+
+        Assert.Contains("You cannot add yourself to groups", exception.Message);
+    }
+
+    [Theory]
+    [BitAutoData]
+    public async Task Put_AdminsCannotAccessAllCollections_Success(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;
+
+        // But! they are already a member of the group
+        currentGroupUsers.Add(savingOrganizationUser.Id);
+
+        sutProvider.GetDependency<IOrganizationRepository>().GetByIdAsync(organization.Id).Returns(organization);
+        sutProvider.GetDependency<IGroupRepository>().GetByIdAsync(group.Id).Returns(group);
+        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);
+        sutProvider.GetDependency<IUserService>().GetProperUserId(Arg.Any<ClaimsPrincipal>())
+            .Returns(savingOrganizationUser.UserId);
+        sutProvider.GetDependency<IGroupRepository>().GetManyUserIdsByIdAsync(group.Id)
+            .Returns(currentGroupUsers);
+
+        var response = await sutProvider.Sut.Put(organization.Id, group.Id, groupRequestModel);
 
         await sutProvider.GetDependency<ICurrentContext>().Received(1).ManageGroups(organization.Id);
         await sutProvider.GetDependency<IUpdateGroupCommand>().Received(1).UpdateGroupAsync(