From 8fd655e8480d8366314bdf09cf62ac31c0013c4b Mon Sep 17 00:00:00 2001 From: stumpylog <797416+stumpylog@users.noreply.github.com> Date: Wed, 14 Oct 2026 11:55:43 -0700 Subject: [PATCH] Chore: drop get_objects_for_user_owner_aware Drops get_objects_for_user_owner_aware. Callers now use restrict_queryset_to_visible. The same question was answered two ways, and the guardian path did not deny deactivated users the way permitted_object_ids does. --- src/documents/permissions.py | 36 ---------------- src/documents/signals/handlers.py | 16 +++---- src/documents/tests/test_consumer.py | 61 ++++++++++++++++++++++++++ src/documents/views.py | 64 ++++++++++++++-------------- src/paperless_mail/serialisers.py | 6 +-- 5 files changed, 103 insertions(+), 80 deletions(-) diff --git a/src/documents/permissions.py b/src/documents/permissions.py index 553e3ddf2..49e0f3f87 100644 --- a/src/documents/permissions.py +++ b/src/documents/permissions.py @@ -21,7 +21,6 @@ from guardian.core import ObjectPermissionChecker from guardian.models import GroupObjectPermission from guardian.models import UserObjectPermission from guardian.shortcuts import assign_perm -from guardian.shortcuts import get_objects_for_user from guardian.shortcuts import get_users_with_perms from guardian.shortcuts import remove_perm from rest_framework.permissions import BasePermission @@ -649,41 +648,6 @@ def annotate_document_count_for_related_queryset( ) -def get_objects_for_user_owner_aware( - user: User | None, - perms: str | list[str], - Model: Any, - *, - include_deleted: bool = False, -) -> QuerySet[Any]: - """ - Returns objects the user owns, are unowned, or has explicit perms. - When include_deleted is True, soft-deleted items are also included. - - Legacy slow path (guardian-backed, O(n) style permission resolution). - Most queryset-filtering call sites have migrated onto - ``PermittedObjectsFilter``/``permitted_object_ids()``, but this function - is kept because production callers still remain. Several callers remain - across ``documents/``, ``paperless_mail/``, and ``paperless_ai/`` -- - grep for this function name before removing it. - """ - manager = ( - Model.global_objects - if include_deleted and hasattr(Model, "global_objects") - else Model.objects - ) - - objects_owned = manager.filter(owner=user) - objects_unowned = manager.filter(owner__isnull=True) - objects_with_perms = get_objects_for_user( - user=user, - perms=perms, - klass=manager.all(), - accept_global_perms=False, - ) - return objects_owned | objects_unowned | objects_with_perms - - def has_perms_owner_aware(user, perms, obj): """ Legacy slow path (guardian-backed) single-object permission check. diff --git a/src/documents/signals/handlers.py b/src/documents/signals/handlers.py index 53acf75ba..409aafa41 100644 --- a/src/documents/signals/handlers.py +++ b/src/documents/signals/handlers.py @@ -52,7 +52,7 @@ from documents.models import Workflow from documents.models import WorkflowAction from documents.models import WorkflowRun from documents.models import WorkflowTrigger -from documents.permissions import get_objects_for_user_owner_aware +from documents.permissions import restrict_queryset_to_visible from documents.plugins.helpers import DocumentsStatusManager from documents.templating.utils import convert_format_str_to_template_format from documents.utils import compute_checksum @@ -81,15 +81,11 @@ DRF_DATETIME_FIELD = serializers.DateTimeField() def add_inbox_tags(sender, document: Document, logging_group=None, **kwargs) -> None: - if document.owner is not None: - tags = get_objects_for_user_owner_aware( - document.owner, - "documents.view_tag", - Tag, - ) - else: - tags = Tag.objects.all() - inbox_tags = tags.filter(is_inbox_tag=True) + inbox_tags = restrict_queryset_to_visible( + Tag.objects.filter(is_inbox_tag=True), + document.owner, + "view_tag", + ) document.add_nested_tags(inbox_tags) diff --git a/src/documents/tests/test_consumer.py b/src/documents/tests/test_consumer.py index a4e48379f..ac897563a 100644 --- a/src/documents/tests/test_consumer.py +++ b/src/documents/tests/test_consumer.py @@ -34,8 +34,10 @@ from documents.tests.utils import GetConsumerMixin from paperless_mail.models import MailRule from paperless_testing.assertions import FileSystemAssertsMixin from paperless_testing.dirs import DirectoriesMixin +from paperless_testing.factories import TagFactory from paperless_testing.factories import UserFactory from paperless_testing.fakes.progress import FakeProgressManager +from paperless_testing.permissions import grant_object class _BaseNewStyleParser: @@ -514,6 +516,65 @@ class TestConsumer( self.assertTrue(group_checker.has_perm("view_document", document)) self._assert_first_last_send_progress() + def testInboxTagsScopedToOwner(self) -> None: + """ + GIVEN: + - Inbox tags that are owned by the document owner, unowned, owned by + another user but shared with the document owner, and owned by + another user and not shared + - A non-inbox tag owned by the document owner + WHEN: + - A document is consumed with that owner + THEN: + - The document gets the owned, unowned and shared inbox tags only + """ + owner = UserFactory() + other = UserFactory() + owned = TagFactory(is_inbox_tag=True, owner=owner) + unowned = TagFactory(is_inbox_tag=True, owner=None) + shared = TagFactory(is_inbox_tag=True, owner=other) + grant_object(owner, shared, "view_tag") + TagFactory(is_inbox_tag=True, owner=other) + TagFactory(is_inbox_tag=False, owner=owner) + + with self.get_consumer( + self.get_test_file(), + DocumentMetadataOverrides(owner_id=owner.pk), + ) as consumer: + consumer.run() + + document = Document.objects.first() + assert document is not None + + self.assertCountEqual(document.tags.all(), [owned, unowned, shared]) + self._assert_first_last_send_progress() + + def testInboxTagsUnownedDocument(self) -> None: + """ + GIVEN: + - Inbox tags owned by two different users, an unowned inbox tag and + a non-inbox tag + WHEN: + - A document is consumed with no owner + THEN: + - The document gets every inbox tag + """ + inbox_tags = [ + TagFactory(is_inbox_tag=True, owner=UserFactory()), + TagFactory(is_inbox_tag=True, owner=UserFactory()), + TagFactory(is_inbox_tag=True, owner=None), + ] + TagFactory(is_inbox_tag=False) + + with self.get_consumer(self.get_test_file()) as consumer: + consumer.run() + + document = Document.objects.first() + assert document is not None + + self.assertCountEqual(document.tags.all(), inbox_tags) + self._assert_first_last_send_progress() + def testNotAFile(self) -> None: with self.assertRaisesMessage(ConsumerError, "File not found"): with self.get_consumer(Path("non-existing-file")) as consumer: diff --git a/src/documents/views.py b/src/documents/views.py index 30d790a07..abf083318 100644 --- a/src/documents/views.py +++ b/src/documents/views.py @@ -175,12 +175,12 @@ from documents.permissions import ViewDocumentsPermissions from documents.permissions import annotate_document_count_by_ids from documents.permissions import annotate_document_count_for_related_queryset from documents.permissions import get_document_count_filter_for_user -from documents.permissions import get_objects_for_user_owner_aware from documents.permissions import has_global_statistics_permission from documents.permissions import has_perms_owner_aware from documents.permissions import has_system_status_permission from documents.permissions import permitted_document_ids from documents.permissions import permitted_object_ids +from documents.permissions import restrict_queryset_to_visible from documents.permissions import set_permissions_for_objects from documents.permissions import user_is_unrestricted from documents.plugins.date_parsing import get_date_parser @@ -3836,49 +3836,51 @@ class GlobalSearchView(PassUserMixin): if doc_id in docs_by_id ][:OBJECT_LIMIT] saved_views = ( - get_objects_for_user_owner_aware( + restrict_queryset_to_visible( + SavedView.objects.filter(name__icontains=query), request.user, "view_savedview", - SavedView, - ).filter(name__icontains=query) + ) if request.user.has_perm("documents.view_savedview") else [] ) saved_views = saved_views[:OBJECT_LIMIT] tags = ( - get_objects_for_user_owner_aware(request.user, "view_tag", Tag).filter( - name__icontains=query, + restrict_queryset_to_visible( + Tag.objects.filter(name__icontains=query), + request.user, + "view_tag", ) if request.user.has_perm("documents.view_tag") else [] ) tags = tags[:OBJECT_LIMIT] correspondents = ( - get_objects_for_user_owner_aware( + restrict_queryset_to_visible( + Correspondent.objects.filter(name__icontains=query), request.user, "view_correspondent", - Correspondent, - ).filter(name__icontains=query) + ) if request.user.has_perm("documents.view_correspondent") else [] ) correspondents = correspondents[:OBJECT_LIMIT] document_types = ( - get_objects_for_user_owner_aware( + restrict_queryset_to_visible( + DocumentType.objects.filter(name__icontains=query), request.user, "view_documenttype", - DocumentType, - ).filter(name__icontains=query) + ) if request.user.has_perm("documents.view_documenttype") else [] ) document_types = document_types[:OBJECT_LIMIT] storage_paths = ( - get_objects_for_user_owner_aware( + restrict_queryset_to_visible( + StoragePath.objects.filter(name__icontains=query), request.user, "view_storagepath", - StoragePath, - ).filter(name__icontains=query) + ) if request.user.has_perm("documents.view_storagepath") else [] ) @@ -3896,21 +3898,21 @@ class GlobalSearchView(PassUserMixin): ) groups = groups[:OBJECT_LIMIT] mail_rules = ( - get_objects_for_user_owner_aware( + restrict_queryset_to_visible( + MailRule.objects.filter(name__icontains=query), request.user, "view_mailrule", - MailRule, - ).filter(name__icontains=query) + ) if request.user.has_perm("paperless_mail.view_mailrule") else [] ) mail_rules = mail_rules[:OBJECT_LIMIT] mail_accounts = ( - get_objects_for_user_owner_aware( + restrict_queryset_to_visible( + MailAccount.objects.filter(name__icontains=query), request.user, "view_mailaccount", - MailAccount, - ).filter(name__icontains=query) + ) if request.user.has_perm("paperless_mail.view_mailaccount") else [] ) @@ -4026,33 +4028,33 @@ class StatisticsView(GenericAPIView[Any]): tags = ( Tag.objects.all() if can_view_global_stats - else get_objects_for_user_owner_aware(user, "documents.view_tag", Tag) + else restrict_queryset_to_visible(Tag.objects.all(), user, "view_tag") ).only("id", "is_inbox_tag") correspondent_count = ( Correspondent.objects.count() if can_view_global_stats - else get_objects_for_user_owner_aware( + else restrict_queryset_to_visible( + Correspondent.objects.all(), user, - "documents.view_correspondent", - Correspondent, + "view_correspondent", ).count() ) document_type_count = ( DocumentType.objects.count() if can_view_global_stats - else get_objects_for_user_owner_aware( + else restrict_queryset_to_visible( + DocumentType.objects.all(), user, - "documents.view_documenttype", - DocumentType, + "view_documenttype", ).count() ) storage_path_count = ( StoragePath.objects.count() if can_view_global_stats - else get_objects_for_user_owner_aware( + else restrict_queryset_to_visible( + StoragePath.objects.all(), user, - "documents.view_storagepath", - StoragePath, + "view_storagepath", ).count() ) diff --git a/src/paperless_mail/serialisers.py b/src/paperless_mail/serialisers.py index d78c220aa..541b02bbb 100644 --- a/src/paperless_mail/serialisers.py +++ b/src/paperless_mail/serialisers.py @@ -2,8 +2,8 @@ from django.utils.translation import gettext as _ from rest_framework import serializers from rest_framework.exceptions import PermissionDenied -from documents.permissions import get_objects_for_user_owner_aware from documents.permissions import has_perms_owner_aware +from documents.permissions import restrict_queryset_to_visible from documents.serialisers import CorrespondentField from documents.serialisers import DocumentTypeField from documents.serialisers import OwnedObjectSerializer @@ -68,10 +68,10 @@ class AccountField(serializers.PrimaryKeyRelatedField[MailAccount]): if user is None: return MailAccount.objects.none() - return get_objects_for_user_owner_aware( + return restrict_queryset_to_visible( + MailAccount.objects.all(), user, "change_mailaccount", - MailAccount, ).order_by("-id")