From 75f8e9f61131c6e746a149ef6bf65a7373f86545 Mon Sep 17 00:00:00 2001 From: stumpylog <797416+stumpylog@users.noreply.github.com> Date: Tue, 28 Jul 2026 07:26:42 -0700 Subject: [PATCH] perf: resolve permitted_document_ids(perm=change_document) once before bulk-edit loops Migrates the bulk document edit permission check in views.py and the custom-field DOCUMENTLINK validator in serialisers.py off of has_perms_owner_aware-per-document loops, resolving permitted_document_ids(user, perm="change_document") once instead. Co-Authored-By: Claude Sonnet 5 Claude-Session: https://claude.ai/code/session_01GRp4kf1mdn9ruv81zWAmh2 --- src/documents/serialisers.py | 7 ++-- .../test_permission_filtering_security.py | 34 +++++++++++++++++++ src/documents/views.py | 3 +- 3 files changed, 38 insertions(+), 6 deletions(-) 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