diff --git a/src/documents/matching.py b/src/documents/matching.py index af0d2d831..65f52d567 100644 --- a/src/documents/matching.py +++ b/src/documents/matching.py @@ -19,7 +19,7 @@ from documents.models import StoragePath from documents.models import Tag from documents.models import Workflow from documents.models import WorkflowTrigger -from documents.permissions import get_objects_for_user_owner_aware +from documents.permissions import permitted_object_ids from documents.regex import safe_regex_search if TYPE_CHECKING: @@ -55,10 +55,8 @@ def match_correspondents(document: Document, classifier: DocumentClassifier, use user = document.owner if user is not None: - correspondents = get_objects_for_user_owner_aware( - user, - "documents.view_correspondent", - Correspondent, + correspondents = Correspondent.objects.filter( + id__in=permitted_object_ids(user, Correspondent, "view_correspondent"), ) else: correspondents = Correspondent.objects.all() @@ -86,10 +84,8 @@ def match_document_types(document: Document, classifier: DocumentClassifier, use user = document.owner if user is not None: - document_types = get_objects_for_user_owner_aware( - user, - "documents.view_documenttype", - DocumentType, + document_types = DocumentType.objects.filter( + id__in=permitted_object_ids(user, DocumentType, "view_documenttype"), ) else: document_types = DocumentType.objects.all() @@ -116,7 +112,9 @@ def match_tags(document: Document, classifier: DocumentClassifier, user=None): user = document.owner if user is not None: - tags = get_objects_for_user_owner_aware(user, "documents.view_tag", Tag) + tags = Tag.objects.filter( + id__in=permitted_object_ids(user, Tag, "view_tag"), + ) else: tags = Tag.objects.all() @@ -145,10 +143,8 @@ def match_storage_paths(document: Document, classifier: DocumentClassifier, user user = document.owner if user is not None: - storage_paths = get_objects_for_user_owner_aware( - user, - "documents.view_storagepath", - StoragePath, + storage_paths = StoragePath.objects.filter( + id__in=permitted_object_ids(user, StoragePath, "view_storagepath"), ) else: storage_paths = StoragePath.objects.all() diff --git a/src/documents/tests/test_permission_filtering_security.py b/src/documents/tests/test_permission_filtering_security.py index aeb8be168..528885d44 100644 --- a/src/documents/tests/test_permission_filtering_security.py +++ b/src/documents/tests/test_permission_filtering_security.py @@ -12,6 +12,10 @@ from django.test import override_settings from guardian.shortcuts import assign_perm from rest_framework.test import APIClient +from documents.matching import match_correspondents +from documents.matching import match_document_types +from documents.matching import match_storage_paths +from documents.matching import match_tags from documents.models import Correspondent from documents.models import DocumentType from documents.models import StoragePath @@ -529,3 +533,90 @@ class TestPermittedObjectIdsGenericModels: expected_visible=[obj.pk], expected_hidden=[], ) + + +@pytest.mark.django_db +class TestMatchingRespectsObjectPermissions: + def test_match_tags_only_considers_tags_visible_to_user(self): + owner = User.objects.create_user(username="tag_owner") + classifying_user = User.objects.create_user(username="classifier_user") + visible_tag = TagFactory( + owner=owner, + match="invoice", + matching_algorithm=Tag.MATCH_LITERAL, + ) + hidden_tag = TagFactory( + owner=owner, + match="invoice", + matching_algorithm=Tag.MATCH_LITERAL, + ) + assign_perm("view_tag", classifying_user, visible_tag) + doc = DocumentFactory(owner=classifying_user, content="an invoice document") + + matched = match_tags(doc, classifier=None, user=classifying_user) + matched_ids = {t.pk for t in matched} + assert visible_tag.pk in matched_ids + assert hidden_tag.pk not in matched_ids + + def test_match_correspondents_only_considers_correspondents_visible_to_user(self): + owner = User.objects.create_user(username="correspondent_owner") + classifying_user = User.objects.create_user(username="classifier_user2") + visible_correspondent = CorrespondentFactory( + owner=owner, + match="invoice", + matching_algorithm=Correspondent.MATCH_LITERAL, + ) + hidden_correspondent = CorrespondentFactory( + owner=owner, + match="invoice", + matching_algorithm=Correspondent.MATCH_LITERAL, + ) + assign_perm("view_correspondent", classifying_user, visible_correspondent) + doc = DocumentFactory(owner=classifying_user, content="an invoice document") + + matched = match_correspondents(doc, classifier=None, user=classifying_user) + matched_ids = {c.pk for c in matched} + assert visible_correspondent.pk in matched_ids + assert hidden_correspondent.pk not in matched_ids + + def test_match_document_types_only_considers_document_types_visible_to_user(self): + owner = User.objects.create_user(username="document_type_owner") + classifying_user = User.objects.create_user(username="classifier_user3") + visible_document_type = DocumentTypeFactory( + owner=owner, + match="invoice", + matching_algorithm=DocumentType.MATCH_LITERAL, + ) + hidden_document_type = DocumentTypeFactory( + owner=owner, + match="invoice", + matching_algorithm=DocumentType.MATCH_LITERAL, + ) + assign_perm("view_documenttype", classifying_user, visible_document_type) + doc = DocumentFactory(owner=classifying_user, content="an invoice document") + + matched = match_document_types(doc, classifier=None, user=classifying_user) + matched_ids = {dt.pk for dt in matched} + assert visible_document_type.pk in matched_ids + assert hidden_document_type.pk not in matched_ids + + def test_match_storage_paths_only_considers_storage_paths_visible_to_user(self): + owner = User.objects.create_user(username="storage_path_owner") + classifying_user = User.objects.create_user(username="classifier_user4") + visible_storage_path = StoragePathFactory( + owner=owner, + match="invoice", + matching_algorithm=StoragePath.MATCH_LITERAL, + ) + hidden_storage_path = StoragePathFactory( + owner=owner, + match="invoice", + matching_algorithm=StoragePath.MATCH_LITERAL, + ) + assign_perm("view_storagepath", classifying_user, visible_storage_path) + doc = DocumentFactory(owner=classifying_user, content="an invoice document") + + matched = match_storage_paths(doc, classifier=None, user=classifying_user) + matched_ids = {sp.pk for sp in matched} + assert visible_storage_path.pk in matched_ids + assert hidden_storage_path.pk not in matched_ids