[AC-1139] Separated flexible collections logic from old logic in CollectionsController; Refactored CollectionAuthorizationHandler

src/Api/Controllers/CollectionsController.cs

@@ -5,7 +5,6 @@ using Bit.Core;
 using Bit.Core.Context;
 using Bit.Core.Entities;
 using Bit.Core.Exceptions;
-using Bit.Core.Models.Data;
 using Bit.Core.OrganizationFeatures.OrganizationCollections.Interfaces;
 using Bit.Core.Repositories;
 using Bit.Core.Services;
@@ -56,27 +55,20 @@ public class CollectionsController : Controller
     [HttpGet("{id}")]
     public async Task<CollectionResponseModel> Get(Guid orgId, Guid id)
     {
-        Collection collection;
-
         if (FlexibleCollectionsIsEnabled)
         {
-            collection = await _collectionRepository.GetByIdAsync(id, _currentContext.UserId.Value);
+            // New flexible collections logic
-            var readAuthorization = await _authorizationService.AuthorizeAsync(User, collection, CollectionOperations.Read);
+            return await Get_FC(id);
-            if (!readAuthorization.Succeeded)
-            {
-                throw new NotFoundException();
-            }
         }
-        else
-        {
-            if (!await CanViewCollectionAsync(orgId, id))
-            {
-                throw new NotFoundException();
-            }
 
-            collection = await GetCollectionAsync(id, orgId);
+        // Old pre-flexible collections logic follows
+        if (!await CanViewCollectionAsync(orgId, id))
+        {
+            throw new NotFoundException();
         }
 
+        var collection = await GetCollectionAsync(id, orgId);
+
         return new CollectionResponseModel(collection);
     }
 
@@ -85,16 +77,11 @@ public class CollectionsController : Controller
     {
         if (FlexibleCollectionsIsEnabled)
         {
-            var (collection, access) = await _collectionRepository.GetByIdWithAccessAsync(id);
+            // New flexible collections logic
-            var readAuthorization = await _authorizationService.AuthorizeAsync(User, collection, CollectionOperations.Read);
+            return await GetDetails_FC(id);
-            if (!readAuthorization.Succeeded)
-            {
-                throw new NotFoundException();
-            }
-
-            return new CollectionAccessDetailsResponseModel(collection, access.Groups, access.Users);
         }
 
+        // Old pre-flexible collections logic follows
         if (!await ViewAtLeastOneCollectionAsync(orgId) && !await _currentContext.ManageUsers(orgId))
         {
             throw new NotFoundException();
@@ -128,35 +115,18 @@ public class CollectionsController : Controller
     {
         if (FlexibleCollectionsIsEnabled)
         {
-            var readAllAuthorization = await _authorizationService.AuthorizeAsync(User, null, CollectionOperations.ReadAll(orgId));
+            // New flexible collections logic
-            if (readAllAuthorization.Succeeded)
+            return await GetManyWithDetails_FC(orgId);
-            {
-                var collections = await _collectionRepository.GetManyByOrganizationIdWithAccessAsync(orgId);
-                return new ListResponseModel<CollectionAccessDetailsResponseModel>(collections.Select(c =>
-                    new CollectionAccessDetailsResponseModel(c.Item1, c.Item2.Groups, c.Item2.Users)));
-            }
-            else
-            {
-                var collections = await _collectionRepository.GetManyByUserIdWithAccessAsync(_currentContext.UserId.Value, orgId);
-                var readAuthorization = await _authorizationService.AuthorizeAsync(User, collections.Select(c => c.Item1), CollectionOperations.Read);
-                if (!readAuthorization.Succeeded)
-                {
-                    throw new NotFoundException();
-                }
-                return new ListResponseModel<CollectionAccessDetailsResponseModel>(collections.Select(c =>
-                    new CollectionAccessDetailsResponseModel(c.Item1, c.Item2.Groups, c.Item2.Users)));
-            }
         }
 
-        if (!await ViewAtLeastOneCollectionAsync(orgId) && !await _currentContext.ManageUsers(orgId) &&
+        // Old pre-flexible collections logic follows
-            !await _currentContext.ManageGroups(orgId))
+        if (!await ViewAtLeastOneCollectionAsync(orgId) && !await _currentContext.ManageUsers(orgId) && !await _currentContext.ManageGroups(orgId))
         {
             throw new NotFoundException();
         }
 
         // We always need to know which collections the current user is assigned to
-        var assignedOrgCollections =
+        var assignedOrgCollections = await _collectionRepository.GetManyByUserIdWithAccessAsync(_currentContext.UserId.Value, orgId);
-            await _collectionRepository.GetManyByUserIdWithAccessAsync(_currentContext.UserId.Value, orgId);
 
         if (await _currentContext.ViewAllCollections(orgId) || await _currentContext.ManageUsers(orgId))
         {
@@ -183,31 +153,14 @@ public class CollectionsController : Controller
     [HttpGet("")]
     public async Task<ListResponseModel<CollectionResponseModel>> Get(Guid orgId)
     {
-        IEnumerable<Collection> orgCollections;
-
         if (FlexibleCollectionsIsEnabled)
         {
-            var readAll = (await _authorizationService.AuthorizeAsync(User, null, CollectionOperations.ReadAll(orgId))).Succeeded;
+            // New flexible collections logic
-            if (readAll)
+            return await GetByOrgId_FC(orgId);
-            {
-                orgCollections = await _collectionRepository.GetManyByOrganizationIdAsync(orgId);
-            }
-            else
-            {
-                var collections = await _collectionRepository.GetManyByUserIdAsync(_currentContext.UserId.Value);
-                orgCollections = collections.Where(c => c.OrganizationId == orgId);
-                var authorized = (await _authorizationService.AuthorizeAsync(User, orgCollections, CollectionOperations.Read)).Succeeded;
-                if (!authorized)
-                {
-                    throw new NotFoundException();
-                }
-            }
-        }
-        else
-        {
-            orgCollections = await _collectionService.GetOrganizationCollectionsAsync(orgId);
         }
 
+        // Old pre-flexible collections logic follows
+        var orgCollections = await _collectionService.GetOrganizationCollectionsAsync(orgId);
         var responses = orgCollections.Select(c => new CollectionResponseModel(c));
         return new ListResponseModel<CollectionResponseModel>(responses);
     }
@@ -224,22 +177,14 @@ public class CollectionsController : Controller
     [HttpGet("{id}/users")]
     public async Task<IEnumerable<SelectionReadOnlyResponseModel>> GetUsers(Guid orgId, Guid id)
     {
-        Collection collection;
-
         if (FlexibleCollectionsIsEnabled)
         {
-            collection = await _collectionRepository.GetByIdAsync(id);
+            // New flexible collections logic
-            var authorized = (await _authorizationService.AuthorizeAsync(User, collection, CollectionOperations.Read)).Succeeded;
+            return await GetUsers_FC(id);
-            if (!authorized)
-            {
-                throw new NotFoundException();
-            }
-        }
-        else
-        {
-            collection = await GetCollectionAsync(id, orgId);
         }
 
+        // Old pre-flexible collections logic follows
+        var collection = await GetCollectionAsync(id, orgId);
         var collectionUsers = await _collectionRepository.GetManyUsersByIdAsync(collection.Id);
         var responses = collectionUsers.Select(cu => new SelectionReadOnlyResponseModel(cu));
         return responses;
@@ -269,27 +214,19 @@ public class CollectionsController : Controller
     [HttpPost("{id}")]
     public async Task<CollectionResponseModel> Put(Guid orgId, Guid id, [FromBody] CollectionRequestModel model)
     {
-        Collection collection;
-
         if (FlexibleCollectionsIsEnabled)
         {
-            collection = await _collectionRepository.GetByIdAsync(id);
+            // New flexible collections logic
-            var authorized = (await _authorizationService.AuthorizeAsync(User, collection, CollectionOperations.Update)).Succeeded;
+            return await Put_FC(id, model);
-            if (!authorized)
-            {
-                throw new NotFoundException();
-            }
         }
-        else
+
+        // Old pre-flexible collections logic follows
+        if (!await CanEditCollectionAsync(orgId, id))
         {
-            if (!await CanEditCollectionAsync(orgId, id))
+            throw new NotFoundException();
-            {
-                throw new NotFoundException();
-            }
-
-            collection = await GetCollectionAsync(id, orgId);
         }
 
+        var collection = await GetCollectionAsync(id, orgId);
         var groups = model.Groups?.Select(g => g.ToSelectionReadOnly());
         var users = model.Users?.Select(g => g.ToSelectionReadOnly());
         await _collectionService.SaveAsync(model.ToCollection(collection), groups, users);
@@ -299,47 +236,20 @@ public class CollectionsController : Controller
     [HttpPut("{id}/users")]
     public async Task PutUsers(Guid orgId, Guid id, [FromBody] IEnumerable<SelectionReadOnlyRequestModel> model)
     {
-        Collection collection;
-        var users = model?.Select(g => g.ToSelectionReadOnly()).ToList() ??
-                    new List<CollectionAccessSelection>();
-
         if (FlexibleCollectionsIsEnabled)
         {
-            collection = await _collectionRepository.GetByIdAsync(id);
+            // New flexible collections logic
-            var authorized = (await _authorizationService.AuthorizeAsync(User, collection, CollectionOperations.ModifyAccess)).Succeeded;
+            await PutUsers_FC(id, model);
-            if (!authorized)
-            {
-                throw new NotFoundException();
-            }
         }
-        else
+
+        // Old pre-flexible collections logic follows
+        if (!await CanEditCollectionAsync(orgId, id))
         {
-            if (!await CanEditCollectionAsync(orgId, id))
+            throw new NotFoundException();
-            {
-                throw new NotFoundException();
-            }
-
-            collection = await GetCollectionAsync(id, orgId);
-
-            // If not using Flexible Collections
-            // all users with EditAnyCollection permission should have Can Manage permission for the collection
-            var organizationUsers = await _organizationUserRepository
-                .GetManyByOrganizationAsync(collection.OrganizationId, null);
-            foreach (var orgUser in organizationUsers.Where(ou => ou.GetPermissions()?.EditAnyCollection ?? false))
-            {
-                var user = users.FirstOrDefault(u => u.Id == orgUser.Id);
-                if (user != null)
-                {
-                    user.Manage = true;
-                }
-                else
-                {
-                    users.Add(new CollectionAccessSelection { Id = orgUser.Id, Manage = true });
-                }
-            }
         }
 
-        await _collectionRepository.UpdateUsersAsync(collection.Id, users);
+        var collection = await GetCollectionAsync(id, orgId);
+        await _collectionRepository.UpdateUsersAsync(collection.Id, model?.Select(g => g.ToSelectionReadOnly()));
     }
 
     [HttpPost("bulk-access")]
@@ -373,27 +283,19 @@ public class CollectionsController : Controller
     [HttpPost("{id}/delete")]
     public async Task Delete(Guid orgId, Guid id)
     {
-        Collection collection;
-
         if (FlexibleCollectionsIsEnabled)
         {
-            collection = await _collectionRepository.GetByIdAsync(id);
+            // New flexible collections logic
-            var authorized = (await _authorizationService.AuthorizeAsync(User, collection, CollectionOperations.Delete)).Succeeded;
+            await Delete_FC(id);
-            if (!authorized)
-            {
-                throw new NotFoundException();
-            }
         }
-        else
+
+        // Old pre-flexible collections logic follows
+        if (!await CanDeleteCollectionAsync(orgId, id))
         {
-            if (!await CanDeleteCollectionAsync(orgId, id))
+            throw new NotFoundException();
-            {
-                throw new NotFoundException();
-            }
-
-            collection = await GetCollectionAsync(id, orgId);
         }
 
+        var collection = await GetCollectionAsync(id, orgId);
         await _deleteCollectionCommand.DeleteAsync(collection);
     }
 
@@ -437,27 +339,14 @@ public class CollectionsController : Controller
     [HttpPost("{id}/delete-user/{orgUserId}")]
     public async Task Delete(Guid orgId, Guid id, Guid orgUserId)
     {
-        Collection collection;
-
         if (FlexibleCollectionsIsEnabled)
         {
-            collection = await _collectionRepository.GetByIdAsync(id);
+            // New flexible collections logic
-            var authorized = (await _authorizationService.AuthorizeAsync(User, collection, CollectionOperations.Delete)).Succeeded;
+            await DeleteUser_FC(id, orgUserId);
-            if (!authorized)
-            {
-                throw new NotFoundException();
-            }
-        }
-        else
-        {
-            if (!await CanDeleteCollectionAsync(orgId, id))
-            {
-                throw new NotFoundException();
-            }
-
-            collection = await GetCollectionAsync(id, orgId);
         }
 
+        // Old pre-flexible collections logic follows
+        var collection = await GetCollectionAsync(id, orgId);
         await _collectionService.DeleteUserAsync(collection, orgUserId);
     }
 
@@ -598,4 +487,143 @@ public class CollectionsController : Controller
 
         return await _currentContext.ViewAllCollections(orgId) || await _currentContext.ViewAssignedCollections(orgId);
     }
+
+    private async Task<CollectionResponseModel> Get_FC(Guid collectionId)
+    {
+        var collection = await _collectionRepository.GetByIdAsync(collectionId, _currentContext.UserId.Value);
+        var authorized = (await _authorizationService.AuthorizeAsync(User, collection, CollectionOperations.Read)).Succeeded;
+        if (!authorized)
+        {
+            throw new NotFoundException();
+        }
+
+        return new CollectionResponseModel(collection);
+    }
+
+    private async Task<CollectionAccessDetailsResponseModel> GetDetails_FC(Guid id)
+    {
+        // New flexible collections logic
+        var (collection, access) = await _collectionRepository.GetByIdWithAccessAsync(id);
+        var authorized = (await _authorizationService.AuthorizeAsync(User, collection, CollectionOperations.Read)).Succeeded;
+        if (!authorized)
+        {
+            throw new NotFoundException();
+        }
+
+        return new CollectionAccessDetailsResponseModel(collection, access.Groups, access.Users);
+    }
+
+    private async Task<ListResponseModel<CollectionAccessDetailsResponseModel>> GetManyWithDetails_FC(Guid orgId)
+    {
+        // We always need to know which collections the current user is assigned to
+        var assignedOrgCollections = await _collectionRepository
+            .GetManyByUserIdWithAccessAsync(_currentContext.UserId.Value, orgId);
+
+        var readAllAuthorized =
+            (await _authorizationService.AuthorizeAsync(User, null, CollectionOperations.ReadAll(orgId))).Succeeded;
+        if (readAllAuthorized)
+        {
+            // The user can view all collections, but they may not always be assigned to all of them
+            var allOrgCollections = await _collectionRepository.GetManyByOrganizationIdWithAccessAsync(orgId);
+
+            return new ListResponseModel<CollectionAccessDetailsResponseModel>(allOrgCollections.Select(c =>
+                new CollectionAccessDetailsResponseModel(c.Item1, c.Item2.Groups, c.Item2.Users)
+                {
+                    // Manually determine which collections they're assigned to
+                    Assigned = assignedOrgCollections.Any(ac => ac.Item1.Id == c.Item1.Id)
+                })
+            );
+        }
+
+        return new ListResponseModel<CollectionAccessDetailsResponseModel>(assignedOrgCollections.Select(c =>
+            new CollectionAccessDetailsResponseModel(c.Item1, c.Item2.Groups, c.Item2.Users)
+            {
+                Assigned = true // Mapping from assignedOrgCollections implies they're all assigned
+            })
+        );
+    }
+
+    private async Task<ListResponseModel<CollectionResponseModel>> GetByOrgId_FC(Guid orgId)
+    {
+        IEnumerable<Collection> orgCollections;
+
+        var readAllAuthorized = (await _authorizationService.AuthorizeAsync(User, null, CollectionOperations.ReadAll(orgId))).Succeeded;
+        if (readAllAuthorized)
+        {
+            orgCollections = await _collectionRepository.GetManyByOrganizationIdAsync(orgId);
+        }
+        else
+        {
+            var collections = await _collectionRepository.GetManyByUserIdAsync(_currentContext.UserId.Value);
+            orgCollections = collections.Where(c => c.OrganizationId == orgId);
+        }
+
+        var responses = orgCollections.Select(c => new CollectionResponseModel(c));
+        return new ListResponseModel<CollectionResponseModel>(responses);
+    }
+
+    private async Task<IEnumerable<SelectionReadOnlyResponseModel>> GetUsers_FC(Guid id)
+    {
+        var collection = await _collectionRepository.GetByIdAsync(id);
+        var authorized = (await _authorizationService.AuthorizeAsync(User, collection, CollectionOperations.Read)).Succeeded;
+        if (!authorized)
+        {
+            throw new NotFoundException();
+        }
+
+        var collectionUsers = await _collectionRepository.GetManyUsersByIdAsync(collection.Id);
+        var responses = collectionUsers.Select(cu => new SelectionReadOnlyResponseModel(cu));
+        return responses;
+    }
+
+    private async Task<CollectionResponseModel> Put_FC(Guid id, CollectionRequestModel model)
+    {
+        var collection = await _collectionRepository.GetByIdAsync(id);
+        var authorized = (await _authorizationService.AuthorizeAsync(User, collection, CollectionOperations.Update)).Succeeded;
+        if (!authorized)
+        {
+            throw new NotFoundException();
+        }
+
+        var groups = model.Groups?.Select(g => g.ToSelectionReadOnly());
+        var users = model.Users?.Select(g => g.ToSelectionReadOnly());
+        await _collectionService.SaveAsync(model.ToCollection(collection), groups, users);
+        return new CollectionResponseModel(collection);
+    }
+
+    private async Task PutUsers_FC(Guid id, IEnumerable<SelectionReadOnlyRequestModel> model)
+    {
+        var collection = await _collectionRepository.GetByIdAsync(id);
+        var authorized = (await _authorizationService.AuthorizeAsync(User, collection, CollectionOperations.ModifyAccess)).Succeeded;
+        if (!authorized)
+        {
+            throw new NotFoundException();
+        }
+
+        await _collectionRepository.UpdateUsersAsync(collection.Id, model?.Select(g => g.ToSelectionReadOnly()));
+    }
+
+    private async Task Delete_FC(Guid id)
+    {
+        var collection = await _collectionRepository.GetByIdAsync(id);
+        var authorized = (await _authorizationService.AuthorizeAsync(User, collection, CollectionOperations.Delete)).Succeeded;
+        if (!authorized)
+        {
+            throw new NotFoundException();
+        }
+
+        await _deleteCollectionCommand.DeleteAsync(collection);
+    }
+
+    private async Task DeleteUser_FC(Guid id, Guid orgUserId)
+    {
+        var collection = await _collectionRepository.GetByIdAsync(id);
+        var authorized = (await _authorizationService.AuthorizeAsync(User, collection, CollectionOperations.ModifyAccess)).Succeeded;
+        if (!authorized)
+        {
+            throw new NotFoundException();
+        }
+
+        await _collectionService.DeleteUserAsync(collection, orgUserId);
+    }
 }

src/Api/Vault/AuthorizationHandlers/Collections/CollectionAuthorizationHandler.cs

@@ -52,8 +52,8 @@ public class CollectionAuthorizationHandler : BulkAuthorizationHandler<Collectio
             return;
         }
 
-        var targetOrganizationId = requirement.OrganizationId != default ?
+        var targetOrganizationId = requirement.OrganizationId != default
-            requirement.OrganizationId : resources.First().OrganizationId;
+            ? requirement.OrganizationId : resources.First().OrganizationId;
 
         // Ensure all target collections belong to the same organization
         if (resources.Any(tc => tc.OrganizationId != targetOrganizationId))
@@ -72,7 +72,7 @@ public class CollectionAuthorizationHandler : BulkAuthorizationHandler<Collectio
         switch (requirement)
         {
             case not null when requirement == CollectionOperations.Create:
-                await CanCreateAsync(context, requirement, resources, org);
+                await CanCreateAsync(context, requirement, org);
                 break;
 
             case not null when requirement == CollectionOperations.Read:
@@ -91,22 +91,12 @@ public class CollectionAuthorizationHandler : BulkAuthorizationHandler<Collectio
             case not null when requirement == CollectionOperations.ModifyAccess:
                 await CanManageCollectionAccessAsync(context, requirement, resources, org);
                 break;
-
-            default:
-                context.Fail();
-                break;
         }
     }
 
     private async Task CanCreateAsync(AuthorizationHandlerContext context, CollectionOperationRequirement requirement,
-        ICollection<Collection> targetCollections, CurrentContextOrganization org)
+        CurrentContextOrganization org)
     {
-        if (targetCollections.Any(c => c.Id != default))
-        {
-            context.Fail();
-            return;
-        }
-
         // If false, all organization members are allowed to create collections
         if (!org.LimitCollectionCreationDeletion)
         {
@@ -130,12 +120,6 @@ public class CollectionAuthorizationHandler : BulkAuthorizationHandler<Collectio
     private async Task CanReadAsync(AuthorizationHandlerContext context, CollectionOperationRequirement requirement,
         ICollection<Collection> targetCollections, CurrentContextOrganization org)
     {
-        if (targetCollections.Any(c => c.Id == default))
-        {
-            context.Fail();
-            return;
-        }
-
         if (org.Type is OrganizationUserType.Owner or OrganizationUserType.Admin ||
             await _currentContext.ProviderUserForOrgAsync(org.Id))
         {
@@ -143,47 +127,27 @@ public class CollectionAuthorizationHandler : BulkAuthorizationHandler<Collectio
             return;
         }
 
-        var manageableCollectionIds =
+        await CheckCollectionPermissionsAsync(context, requirement, targetCollections, org, requireManagePermission: false);
-            (await _collectionRepository.GetManyByUserIdAsync(_currentContext.UserId!.Value))
-            .Where(c => c.OrganizationId == org.Id)
-            .Select(c => c.Id)
-            .ToHashSet();
-
-        // The acting user does not have permission to manage all target collections, fail
-        if (targetCollections.Any(c => !manageableCollectionIds.Contains(c.Id)))
-        {
-            context.Fail();
-            return;
-        }
-
-        context.Succeed(requirement);
     }
 
     private async Task CanReadAllAsync(AuthorizationHandlerContext context, CollectionOperationRequirement requirement,
         CurrentContextOrganization org)
     {
-        if (org.Type is not (OrganizationUserType.Owner or OrganizationUserType.Admin) &&
+        if (org.Type is OrganizationUserType.Owner or OrganizationUserType.Admin ||
-            !org.Permissions.ManageGroups &&
+            org.Permissions.ManageGroups ||
-            !org.Permissions.ManageUsers &&
+            org.Permissions.ManageUsers ||
-            !org.Permissions.EditAnyCollection &&
+            org.Permissions.EditAnyCollection ||
-            !org.Permissions.DeleteAnyCollection &&
+            org.Permissions.DeleteAnyCollection ||
-            !org.Permissions.AccessImportExport)
+            org.Permissions.AccessImportExport ||
+            await _currentContext.ProviderUserForOrgAsync(org.Id))
         {
-            context.Fail();
+            context.Succeed(requirement);
         }
-
-        context.Succeed(requirement);
     }
 
     private async Task CanDeleteAsync(AuthorizationHandlerContext context, CollectionOperationRequirement requirement,
         ICollection<Collection> targetCollections, CurrentContextOrganization org)
     {
-        if (targetCollections.Any(c => c.Id == default))
-        {
-            context.Fail();
-            return;
-        }
-
         // Owners, Admins, Providers, and users with DeleteAnyCollection permission can always delete collections
         if (
             org.Type is OrganizationUserType.Owner or OrganizationUserType.Admin ||
@@ -201,21 +165,7 @@ public class CollectionAuthorizationHandler : BulkAuthorizationHandler<Collectio
             return;
         }
 
-        // Other members types should have the Manage capability for all collections being deleted
+        await CheckCollectionPermissionsAsync(context, requirement, targetCollections, org, requireManagePermission: true);
-        var manageableCollectionIds =
-            (await _collectionRepository.GetManyByUserIdAsync(_currentContext.UserId!.Value))
-            .Where(c => c.Manage && c.OrganizationId == org.Id)
-            .Select(c => c.Id)
-            .ToHashSet();
-
-        // The acting user does not have permission to manage all target collections, fail
-        if (targetCollections.Any(c => !manageableCollectionIds.Contains(c.Id)))
-        {
-            context.Fail();
-            return;
-        }
-
-        context.Succeed(requirement);
     }
 
     /// <summary>
@@ -224,12 +174,6 @@ public class CollectionAuthorizationHandler : BulkAuthorizationHandler<Collectio
     private async Task CanManageCollectionAccessAsync(AuthorizationHandlerContext context,
         IAuthorizationRequirement requirement, ICollection<Collection> targetCollections, CurrentContextOrganization org)
     {
-        if (targetCollections.Any(c => c.Id == default))
-        {
-            context.Fail();
-            return;
-        }
-
         // Owners, Admins, Providers, and users with EditAnyCollection permission can always manage collection access
         if (
             org.Permissions is { EditAnyCollection: true } ||
@@ -240,14 +184,27 @@ public class CollectionAuthorizationHandler : BulkAuthorizationHandler<Collectio
             return;
         }
 
-        // List of collection Ids the acting user is allowed to manage
+        await CheckCollectionPermissionsAsync(context, requirement, targetCollections, org, requireManagePermission: true);
+    }
+
+    private async Task CheckCollectionPermissionsAsync(
+        AuthorizationHandlerContext context,
+        IAuthorizationRequirement requirement,
+        ICollection<Collection> targetCollections,
+        CurrentContextOrganization org,
+        bool requireManagePermission)
+    {
+        // List of collection Ids the acting user has access to
         var manageableCollectionIds =
             (await _collectionRepository.GetManyByUserIdAsync(_currentContext.UserId!.Value))
-            .Where(c => c.Manage && c.OrganizationId == org.Id)
+            .Where(c =>
+                // If requireManagePermission is true, check Collections with Manage permission
+                (!requireManagePermission || c.Manage)
+                && c.OrganizationId == org.Id)
             .Select(c => c.Id)
             .ToHashSet();
 
-        // The acting user does not have permission to manage all target collections, fail
+        // The acting user does not have permissions for all target collections, fail
         if (targetCollections.Any(tc => !manageableCollectionIds.Contains(tc.Id)))
         {
             context.Fail();