diff --git a/src/documents/serialisers.py b/src/documents/serialisers.py index 8499cf359..c1115a72e 100644 --- a/src/documents/serialisers.py +++ b/src/documents/serialisers.py @@ -864,11 +864,8 @@ def validate_documentlink_targets(user, doc_ids): if user is None: return - target_documents = Document.objects.filter(id__in=doc_ids).select_related("owner") - if not all( - has_perms_owner_aware(user, "change_document", document) - for document in target_documents - ): + permitted_change_ids = set(permitted_document_ids(user, perm="change_document")) + if not set(doc_ids) <= permitted_change_ids: raise PermissionDenied( _("Insufficient permissions."), ) diff --git a/src/documents/tests/test_permission_filtering_security.py b/src/documents/tests/test_permission_filtering_security.py index 7b28bcb05..172bfb197 100644 --- a/src/documents/tests/test_permission_filtering_security.py +++ b/src/documents/tests/test_permission_filtering_security.py @@ -280,3 +280,37 @@ class TestEmailDocumentPermissionBoundary: format="json", ) assert response.status_code == 403 + + +@pytest.mark.django_db +class TestBulkEditChangePermissionBoundary: + def test_bulk_edit_rejects_document_without_change_permission( + self, + rest_api_client, + ): + owner = User.objects.create_user(username="owner") + requester = User.objects.create_user(username="requester") + # grant the global change_document permission so the object-level + # check (not the global has_perm check) is what's under test + requester.user_permissions.add( + Permission.objects.get(codename="change_document"), + ) + rest_api_client.force_authenticate(user=requester) + assign_perm( + "view_document", + requester, + DocumentFactory(owner=owner), + ) # unrelated grant + target = DocumentFactory(owner=owner) + assign_perm("view_document", requester, target) # view only, NOT change + + response = rest_api_client.post( + "/api/documents/bulk_edit/", + { + "documents": [target.pk], + "method": "modify_tags", + "parameters": {"add_tags": [], "remove_tags": []}, + }, + format="json", + ) + assert response.status_code == 403 diff --git a/src/documents/views.py b/src/documents/views.py index 46eff8504..333b04049 100644 --- a/src/documents/views.py +++ b/src/documents/views.py @@ -2782,8 +2782,9 @@ class DocumentOperationPermissionMixin(PassUserMixin, DocumentSelectionMixin): ) # check global and object permissions for all documents + permitted_change_ids = set(permitted_document_ids(user, perm="change_document")) has_perms = user.has_perm("documents.change_document") and all( - has_perms_owner_aware(user, "change_document", doc) for doc in document_objs + doc.pk in permitted_change_ids for doc in document_objs ) # check ownership for methods that change original document