diff --git a/src/Api/Controllers/CollectionsController.cs b/src/Api/Controllers/CollectionsController.cs index 3c1fdf3c1a..ee2b5dec77 100644 --- a/src/Api/Controllers/CollectionsController.cs +++ b/src/Api/Controllers/CollectionsController.cs @@ -509,7 +509,7 @@ public class CollectionsController : Controller { // New flexible collections logic var (collection, access) = await _collectionRepository.GetByIdWithAccessAsync(id); - var authorized = (await _authorizationService.AuthorizeAsync(User, collection, BulkCollectionOperations.Read)).Succeeded; + var authorized = (await _authorizationService.AuthorizeAsync(User, collection, BulkCollectionOperations.ReadWithAccess)).Succeeded; if (!authorized) { throw new NotFoundException(); diff --git a/src/Api/Vault/AuthorizationHandlers/Collections/BulkCollectionAuthorizationHandler.cs b/src/Api/Vault/AuthorizationHandlers/Collections/BulkCollectionAuthorizationHandler.cs index c169611bf6..614567c0c1 100644 --- a/src/Api/Vault/AuthorizationHandlers/Collections/BulkCollectionAuthorizationHandler.cs +++ b/src/Api/Vault/AuthorizationHandlers/Collections/BulkCollectionAuthorizationHandler.cs @@ -74,16 +74,17 @@ public class BulkCollectionAuthorizationHandler : BulkAuthorizationHandler resources, CurrentContextOrganization? org) { // Owners, Admins, and users with EditAnyCollection or DeleteAnyCollection permission can always read a collection @@ -157,7 +157,8 @@ public class BulkCollectionAuthorizationHandler : BulkAuthorizationHandler - /// Ensures the acting user is allowed to manage access permissions for the target collections. + /// Ensures the acting user is allowed to update the target collections or manage access permissions for them. /// - private async Task CanManageCollectionAccessAsync(AuthorizationHandlerContext context, + private async Task CanUpdateCollection(AuthorizationHandlerContext context, IAuthorizationRequirement requirement, ICollection resources, CurrentContextOrganization? org) { @@ -202,7 +203,7 @@ public class BulkCollectionAuthorizationHandler : BulkAuthorizationHandler HasCollectionAccessAsync( + private async Task CanManageCollectionsAsync( ICollection targetCollections, - CurrentContextOrganization org, - bool requireManagePermission) + CurrentContextOrganization org) { // List of collection Ids the acting user has access to var manageableCollectionIds = (await _collectionRepository.GetManyByUserIdAsync(_currentContext.UserId!.Value)) .Where(c => - // If requireManagePermission is true, check Collections with Manage permission - (!requireManagePermission || c.Manage) - && c.OrganizationId == org.Id) + // Check Collections with Manage permission + c.Manage && c.OrganizationId == org.Id) .Select(c => c.Id) .ToHashSet(); diff --git a/src/Api/Vault/AuthorizationHandlers/Collections/BulkCollectionOperations.cs b/src/Api/Vault/AuthorizationHandlers/Collections/BulkCollectionOperations.cs index 7eacc37514..fc60db6e58 100644 --- a/src/Api/Vault/AuthorizationHandlers/Collections/BulkCollectionOperations.cs +++ b/src/Api/Vault/AuthorizationHandlers/Collections/BulkCollectionOperations.cs @@ -9,6 +9,7 @@ public static class BulkCollectionOperations public static readonly BulkCollectionOperationRequirement Create = new() { Name = nameof(Create) }; public static readonly BulkCollectionOperationRequirement Read = new() { Name = nameof(Read) }; public static readonly BulkCollectionOperationRequirement ReadAccess = new() { Name = nameof(ReadAccess) }; + public static readonly BulkCollectionOperationRequirement ReadWithAccess = new() { Name = nameof(ReadWithAccess) }; public static readonly BulkCollectionOperationRequirement Update = new() { Name = nameof(Update) }; /// /// The operation that represents creating, updating, or removing collection access. diff --git a/test/Api.Test/Vault/AuthorizationHandlers/BulkCollectionAuthorizationHandlerTests.cs b/test/Api.Test/Vault/AuthorizationHandlers/BulkCollectionAuthorizationHandlerTests.cs index f2fd99c71d..ca578f58da 100644 --- a/test/Api.Test/Vault/AuthorizationHandlers/BulkCollectionAuthorizationHandlerTests.cs +++ b/test/Api.Test/Vault/AuthorizationHandlers/BulkCollectionAuthorizationHandlerTests.cs @@ -158,14 +158,13 @@ public class BulkCollectionAuthorizationHandlerTests } [Theory, CollectionCustomization] - [BitAutoData(true, false, false, false, true)] - [BitAutoData(false, true, false, false, true)] - [BitAutoData(false, false, true, false, true)] - [BitAutoData(false, false, false, true, true)] - [BitAutoData(false, false, false, false, false)] + [BitAutoData(true, false, false, true)] + [BitAutoData(false, true, false, true)] + [BitAutoData(false, false, true, true)] + [BitAutoData(false, false, false, false)] public async Task CanReadAsync_WhenCustomUserWithRequiredPermissions_Success( - bool manageUsers, bool editAnyCollection, bool deleteAnyCollection, + bool editAnyCollection, bool deleteAnyCollection, bool createNewCollections, bool limitCollectionCreationDeletion, SutProvider sutProvider, ICollection collections, @@ -177,7 +176,6 @@ public class BulkCollectionAuthorizationHandlerTests organization.LimitCollectionCreationDeletion = limitCollectionCreationDeletion; organization.Permissions = new Permissions { - ManageUsers = manageUsers, EditAnyCollection = editAnyCollection, DeleteAnyCollection = deleteAnyCollection, CreateNewCollections = createNewCollections @@ -421,12 +419,10 @@ public class BulkCollectionAuthorizationHandlerTests Assert.False(context.HasSucceeded); } - // - [Theory, CollectionCustomization] [BitAutoData(OrganizationUserType.Admin)] [BitAutoData(OrganizationUserType.Owner)] - public async Task CanManageCollectionAccessAsync_WhenAdminOrOwner_Success( + public async Task CanUpdateCollection_WhenAdminOrOwner_Success( OrganizationUserType userType, Guid userId, SutProvider sutProvider, ICollection collections, @@ -461,7 +457,7 @@ public class BulkCollectionAuthorizationHandlerTests } [Theory, BitAutoData, CollectionCustomization] - public async Task CanManageCollectionAccessAsync_WithEditAnyCollectionPermission_Success( + public async Task CanUpdateCollection_WithEditAnyCollectionPermission_Success( SutProvider sutProvider, ICollection collections, CurrentContextOrganization organization) @@ -499,7 +495,7 @@ public class BulkCollectionAuthorizationHandlerTests } [Theory, BitAutoData, CollectionCustomization] - public async Task CanManageCollectionAccessAsync_WithManageCollectionPermission_Success( + public async Task CanUpdateCollection_WithManageCollectionPermission_Success( SutProvider sutProvider, ICollection collections, CurrentContextOrganization organization) @@ -542,7 +538,7 @@ public class BulkCollectionAuthorizationHandlerTests [Theory, CollectionCustomization] [BitAutoData(OrganizationUserType.User)] [BitAutoData(OrganizationUserType.Custom)] - public async Task CanManageCollectionAccessAsync_WhenMissingPermissions_NoSuccess( + public async Task CanUpdateCollection_WhenMissingPermissions_NoSuccess( OrganizationUserType userType, SutProvider sutProvider, ICollection collections, @@ -592,7 +588,7 @@ public class BulkCollectionAuthorizationHandlerTests } [Theory, BitAutoData, CollectionCustomization] - public async Task CanManageCollectionAccessAsync_WhenMissingOrgAccess_NoSuccess( + public async Task CanUpdateCollection_WhenMissingOrgAccess_NoSuccess( Guid userId, ICollection collections, SutProvider sutProvider)