1
0
mirror of https://github.com/bitwarden/server.git synced 2025-04-05 21:18:13 -05:00

[AC-1389] [AC-1919] Only require CanManage permission when admins cannot access all items (#3530)

* move this error behind the Flexible Collections v1 flag instead of MVP
* only enforce this requirement if organization.allowAdminAccessToAllCollectionItems is false

---------

Co-authored-by: Thomas Rittson <trittson@bitwarden.com>
Co-authored-by: Thomas Rittson <31796059+eliykat@users.noreply.github.com>
This commit is contained in:
Will Martin 2024-01-04 20:56:59 -05:00 committed by GitHub
parent 061253e428
commit c553ec6aa0
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
2 changed files with 8 additions and 7 deletions

View File

@ -43,9 +43,6 @@ public class CollectionService : ICollectionService
_featureService = featureService; _featureService = featureService;
} }
private bool UseFlexibleCollections =>
_featureService.IsEnabled(FeatureFlagKeys.FlexibleCollections, _currentContext);
public async Task SaveAsync(Collection collection, IEnumerable<CollectionAccessSelection> groups = null, public async Task SaveAsync(Collection collection, IEnumerable<CollectionAccessSelection> groups = null,
IEnumerable<CollectionAccessSelection> users = null) IEnumerable<CollectionAccessSelection> users = null)
{ {
@ -59,11 +56,11 @@ public class CollectionService : ICollectionService
var usersList = users?.ToList(); var usersList = users?.ToList();
// If using Flexible Collections - a collection should always have someone with Can Manage permissions // If using Flexible Collections - a collection should always have someone with Can Manage permissions
if (UseFlexibleCollections) if (_featureService.IsEnabled(FeatureFlagKeys.FlexibleCollectionsV1, _currentContext))
{ {
var groupHasManageAccess = groupsList?.Any(g => g.Manage) ?? false; var groupHasManageAccess = groupsList?.Any(g => g.Manage) ?? false;
var userHasManageAccess = usersList?.Any(u => u.Manage) ?? false; var userHasManageAccess = usersList?.Any(u => u.Manage) ?? false;
if (!groupHasManageAccess && !userHasManageAccess) if (!groupHasManageAccess && !userHasManageAccess && !org.AllowAdminAccessToAllCollectionItems)
{ {
throw new BadRequestException( throw new BadRequestException(
"At least one member or group must have can manage permission."); "At least one member or group must have can manage permission.");
@ -125,7 +122,10 @@ public class CollectionService : ICollectionService
} }
else else
{ {
var collections = await _collectionRepository.GetManyByUserIdAsync(_currentContext.UserId.Value, UseFlexibleCollections); var collections = await _collectionRepository.GetManyByUserIdAsync(
_currentContext.UserId.Value,
_featureService.IsEnabled(FeatureFlagKeys.FlexibleCollections, _currentContext)
);
orgCollections = collections.Where(c => c.OrganizationId == organizationId); orgCollections = collections.Where(c => c.OrganizationId == organizationId);
} }

View File

@ -114,8 +114,9 @@ public class CollectionServiceTest
collection.Id = default; collection.Id = default;
sutProvider.GetDependency<IOrganizationRepository>().GetByIdAsync(organization.Id).Returns(organization); sutProvider.GetDependency<IOrganizationRepository>().GetByIdAsync(organization.Id).Returns(organization);
sutProvider.GetDependency<IFeatureService>() sutProvider.GetDependency<IFeatureService>()
.IsEnabled(FeatureFlagKeys.FlexibleCollections, Arg.Any<ICurrentContext>(), Arg.Any<bool>()) .IsEnabled(FeatureFlagKeys.FlexibleCollectionsV1, Arg.Any<ICurrentContext>(), Arg.Any<bool>())
.Returns(true); .Returns(true);
organization.AllowAdminAccessToAllCollectionItems = false;
var ex = await Assert.ThrowsAsync<BadRequestException>(() => sutProvider.Sut.SaveAsync(collection, null, users)); var ex = await Assert.ThrowsAsync<BadRequestException>(() => sutProvider.Sut.SaveAsync(collection, null, users));
Assert.Contains("At least one member or group must have can manage permission.", ex.Message); Assert.Contains("At least one member or group must have can manage permission.", ex.Message);