diff --git a/src/Api/AdminConsole/Controllers/GroupsController.cs b/src/Api/AdminConsole/Controllers/GroupsController.cs index 026e0f03f..dfe927ede 100644 --- a/src/Api/AdminConsole/Controllers/GroupsController.cs +++ b/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 Put(string orgId, string id, [FromBody] GroupRequestModel model) + public async Task 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); - var organization = await _organizationRepository.GetByIdAsync(orgIdGuid); + // Flexible Collections v1 - a user may not be able to add themselves to a group + 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 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) diff --git a/test/Api.Test/AdminConsole/Controllers/GroupsControllerTests.cs b/test/Api.Test/AdminConsole/Controllers/GroupsControllerTests.cs index 2bedef7f6..2803c5df0 100644 --- a/test/Api.Test/AdminConsole/Controllers/GroupsControllerTests.cs +++ b/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 sutProvider) + public async Task Put_AdminsCanAccessAllCollections_Success(Organization organization, Group group, GroupRequestModel groupRequestModel, SutProvider sutProvider) { group.OrganizationId = organization.Id; sutProvider.GetDependency().GetByIdAsync(organization.Id).Returns(organization); sutProvider.GetDependency().GetByIdAsync(group.Id).Returns(group); sutProvider.GetDependency().ManageGroups(organization.Id).Returns(true); + sutProvider.GetDependency().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().Received(1).ManageGroups(organization.Id); + await sutProvider.GetDependency().Received(1).UpdateGroupAsync( + Arg.Is(g => + g.OrganizationId == organization.Id && g.Name == groupRequestModel.Name && + g.AccessAll == groupRequestModel.AccessAll), + Arg.Is(o => o.Id == organization.Id), + Arg.Any>(), + Arg.Any>()); + 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 currentGroupUsers, + SutProvider 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().GetByIdAsync(organization.Id).Returns(organization); + sutProvider.GetDependency().GetByIdAsync(group.Id).Returns(group); + sutProvider.GetDependency().ManageGroups(organization.Id).Returns(true); + sutProvider.GetDependency().GetOrganizationAbilityAsync(organization.Id).Returns( + new OrganizationAbility + { + Id = organization.Id, + AllowAdminAccessToAllCollectionItems = false, + FlexibleCollections = true + }); + sutProvider.GetDependency().IsEnabled(FeatureFlagKeys.FlexibleCollectionsV1).Returns(true); + sutProvider.GetDependency() + .GetByOrganizationAsync(organization.Id, Arg.Any()) + .Returns(savingOrganizationUser); + sutProvider.GetDependency().GetProperUserId(Arg.Any()) + .Returns(savingOrganizationUser.UserId); + sutProvider.GetDependency().GetManyUserIdsByIdAsync(group.Id) + .Returns(currentGroupUsers); + + var exception = await + Assert.ThrowsAsync(() => 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 currentGroupUsers, + SutProvider 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().GetByIdAsync(organization.Id).Returns(organization); + sutProvider.GetDependency().GetByIdAsync(group.Id).Returns(group); + sutProvider.GetDependency().ManageGroups(organization.Id).Returns(true); + sutProvider.GetDependency().GetOrganizationAbilityAsync(organization.Id).Returns( + new OrganizationAbility + { + Id = organization.Id, + AllowAdminAccessToAllCollectionItems = false, + FlexibleCollections = true + }); + sutProvider.GetDependency().IsEnabled(FeatureFlagKeys.FlexibleCollectionsV1).Returns(true); + sutProvider.GetDependency() + .GetByOrganizationAsync(organization.Id, Arg.Any()) + .Returns(savingOrganizationUser); + sutProvider.GetDependency().GetProperUserId(Arg.Any()) + .Returns(savingOrganizationUser.UserId); + sutProvider.GetDependency().GetManyUserIdsByIdAsync(group.Id) + .Returns(currentGroupUsers); + + var response = await sutProvider.Sut.Put(organization.Id, group.Id, groupRequestModel); await sutProvider.GetDependency().Received(1).ManageGroups(organization.Id); await sutProvider.GetDependency().Received(1).UpdateGroupAsync(