diff --git a/src/Api/Controllers/CollectionsController.cs b/src/Api/Controllers/CollectionsController.cs index 39d3c32262..7ae76ba750 100644 --- a/src/Api/Controllers/CollectionsController.cs +++ b/src/Api/Controllers/CollectionsController.cs @@ -584,11 +584,6 @@ public class CollectionsController : Controller // Filter the assigned collections to only return those where the user has Manage permission var manageableOrgCollections = assignedOrgCollections.Where(c => c.Item1.Manage).ToList(); - var readAssignedAuthorized = await _authorizationService.AuthorizeAsync(User, manageableOrgCollections.Select(c => c.Item1), BulkCollectionOperations.ReadWithAccess); - if (!readAssignedAuthorized.Succeeded) - { - throw new NotFoundException(); - } return new ListResponseModel(manageableOrgCollections.Select(c => new CollectionAccessDetailsResponseModel(c.Item1, c.Item2.Groups, c.Item2.Users) @@ -609,16 +604,8 @@ public class CollectionsController : Controller } else { - var collections = await _collectionRepository.GetManyByUserIdAsync(_currentContext.UserId.Value, FlexibleCollectionsIsEnabled); - var readAuthorized = (await _authorizationService.AuthorizeAsync(User, collections, BulkCollectionOperations.Read)).Succeeded; - if (readAuthorized) - { - orgCollections = collections.Where(c => c.OrganizationId == orgId); - } - else - { - throw new NotFoundException(); - } + var assignedCollections = await _collectionRepository.GetManyByUserIdAsync(_currentContext.UserId.Value, FlexibleCollectionsIsEnabled); + orgCollections = assignedCollections.Where(c => c.OrganizationId == orgId && c.Manage).ToList(); } var responses = orgCollections.Select(c => new CollectionResponseModel(c)); diff --git a/src/Api/Vault/AuthorizationHandlers/Collections/BulkCollectionAuthorizationHandler.cs b/src/Api/Vault/AuthorizationHandlers/Collections/BulkCollectionAuthorizationHandler.cs index db44020b05..76544e3b8d 100644 --- a/src/Api/Vault/AuthorizationHandlers/Collections/BulkCollectionAuthorizationHandler.cs +++ b/src/Api/Vault/AuthorizationHandlers/Collections/BulkCollectionAuthorizationHandler.cs @@ -131,8 +131,8 @@ public class BulkCollectionAuthorizationHandler : BulkAuthorizationHandler IsAssignedToCollectionsAsync( + private async Task CanManageCollectionsAsync( ICollection targetCollections, - CurrentContextOrganization org, - bool requireManagePermission) + CurrentContextOrganization org) { // List of collection Ids the acting user has access to var assignedCollectionIds = (await _collectionRepository.GetManyByUserIdAsync(_currentContext.UserId!.Value, useFlexibleCollections: true)) .Where(c => // Check Collections with Manage permission - (!requireManagePermission || c.Manage) && c.OrganizationId == org.Id) + c.Manage && c.OrganizationId == org.Id) .Select(c => c.Id) .ToHashSet(); diff --git a/test/Api.Test/Controllers/CollectionsControllerTests.cs b/test/Api.Test/Controllers/CollectionsControllerTests.cs index 41d877c20e..f8f3b890bb 100644 --- a/test/Api.Test/Controllers/CollectionsControllerTests.cs +++ b/test/Api.Test/Controllers/CollectionsControllerTests.cs @@ -142,7 +142,7 @@ public class CollectionsControllerTests } [Theory, BitAutoData] - public async Task GetOrganizationCollectionsWithGroups_MissingReadPermissions_ThrowsNotFound(Organization organization, Guid userId, SutProvider sutProvider) + public async Task GetOrganizationCollections_WithReadAllPermissions_GetsAllCollections(Organization organization, ICollection collections, Guid userId, SutProvider sutProvider) { sutProvider.GetDependency().UserId.Returns(userId); @@ -152,7 +152,37 @@ public class CollectionsControllerTests Arg.Any(), Arg.Is>(requirements => requirements.Cast().All(operation => - operation.Name == nameof(CollectionOperations.ReadAllWithAccess) + operation.Name == nameof(CollectionOperations.ReadAll) + && operation.OrganizationId == organization.Id))) + .Returns(AuthorizationResult.Success()); + + sutProvider.GetDependency() + .GetManyByOrganizationIdAsync(organization.Id) + .Returns(collections); + + var response = await sutProvider.Sut.Get(organization.Id); + + await sutProvider.GetDependency().Received(1).GetManyByOrganizationIdAsync(organization.Id); + + Assert.Equal(collections.Count, response.Data.Count()); + } + + [Theory, BitAutoData] + public async Task GetOrganizationCollections_MissingReadAllPermissions_GetsManageableCollections(Organization organization, ICollection collections, Guid userId, SutProvider sutProvider) + { + collections.First().OrganizationId = organization.Id; + collections.First().Manage = true; + collections.Skip(1).First().OrganizationId = organization.Id; + + sutProvider.GetDependency().UserId.Returns(userId); + + sutProvider.GetDependency() + .AuthorizeAsync( + Arg.Any(), + Arg.Any(), + Arg.Is>(requirements => + requirements.Cast().All(operation => + operation.Name == nameof(CollectionOperations.ReadAll) && operation.OrganizationId == organization.Id))) .Returns(AuthorizationResult.Failed()); @@ -162,10 +192,20 @@ public class CollectionsControllerTests Arg.Any(), Arg.Is>(requirements => requirements.Cast().All(operation => - operation.Name == nameof(BulkCollectionOperations.ReadWithAccess)))) - .Returns(AuthorizationResult.Failed()); + operation.Name == nameof(BulkCollectionOperations.Read)))) + .Returns(AuthorizationResult.Success()); - _ = await Assert.ThrowsAsync(async () => await sutProvider.Sut.GetManyWithDetails(organization.Id)); + sutProvider.GetDependency() + .GetManyByUserIdAsync(userId, true) + .Returns(collections); + + var result = await sutProvider.Sut.Get(organization.Id); + + await sutProvider.GetDependency().DidNotReceive().GetManyByOrganizationIdAsync(organization.Id); + await sutProvider.GetDependency().Received(1).GetManyByUserIdAsync(userId, true); + + Assert.Single(result.Data); + Assert.All(result.Data, c => Assert.Equal(organization.Id, c.OrganizationId)); } [Theory, BitAutoData] diff --git a/test/Api.Test/Vault/AuthorizationHandlers/BulkCollectionAuthorizationHandlerTests.cs b/test/Api.Test/Vault/AuthorizationHandlers/BulkCollectionAuthorizationHandlerTests.cs index 524533aaf6..14e863799b 100644 --- a/test/Api.Test/Vault/AuthorizationHandlers/BulkCollectionAuthorizationHandlerTests.cs +++ b/test/Api.Test/Vault/AuthorizationHandlers/BulkCollectionAuthorizationHandlerTests.cs @@ -204,13 +204,18 @@ public class BulkCollectionAuthorizationHandlerTests } [Theory, BitAutoData, CollectionCustomization] - public async Task CanReadAsync_WhenUserIsAssignedToCollections_Success( + public async Task CanReadAsync_WhenUserCanManageCollections_Success( SutProvider sutProvider, ICollection collections, CurrentContextOrganization organization) { var actingUserId = Guid.NewGuid(); + foreach (var c in collections) + { + c.Manage = true; + } + organization.Type = OrganizationUserType.User; organization.LimitCollectionCreationDeletion = false; organization.Permissions = new Permissions();