From 7dbf8bdd4aff6ceb4eedd8a4aba7652978d1dc74 Mon Sep 17 00:00:00 2001 From: shamoon <4887959+shamoon@users.noreply.github.com> Date: Sat, 21 Mar 2026 00:44:28 -0700 Subject: [PATCH 1/4] Fix: enforce permissions when attaching accounts to mail rules --- src/paperless_mail/serialisers.py | 15 ++++ src/paperless_mail/tests/test_api.py | 108 +++++++++++++++++++++++++++ 2 files changed, 123 insertions(+) diff --git a/src/paperless_mail/serialisers.py b/src/paperless_mail/serialisers.py index b38c8e78c..aff3e75da 100644 --- a/src/paperless_mail/serialisers.py +++ b/src/paperless_mail/serialisers.py @@ -1,5 +1,8 @@ +from django.utils.translation import gettext as _ from rest_framework import serializers +from rest_framework.exceptions import PermissionDenied +from documents.permissions import has_perms_owner_aware from documents.serialisers import CorrespondentField from documents.serialisers import DocumentTypeField from documents.serialisers import OwnedObjectSerializer @@ -127,6 +130,18 @@ class MailRuleSerializer(OwnedObjectSerializer): return attrs + def validate_account(self, account): + if self.user is not None and has_perms_owner_aware( + self.user, + "change_mailaccount", + account, + ): + return account + + raise PermissionDenied( + _("Insufficient permissions."), + ) + def validate_maximum_age(self, value): if value > 36500: # ~100 years raise serializers.ValidationError("Maximum mail age is unreasonably large.") diff --git a/src/paperless_mail/tests/test_api.py b/src/paperless_mail/tests/test_api.py index cbfe0f9a4..905509ec1 100644 --- a/src/paperless_mail/tests/test_api.py +++ b/src/paperless_mail/tests/test_api.py @@ -632,6 +632,114 @@ class TestAPIMailRules(DirectoriesMixin, APITestCase): self.assertEqual(returned_rule1.name, "Updated Name 1") self.assertEqual(returned_rule1.action, MailRule.MailAction.DELETE) + def test_create_mail_rule_forbidden_for_unpermitted_account(self): + other_user = User.objects.create_user(username="mail-owner") + foreign_account = MailAccount.objects.create( + name="ForeignEmail", + username="username1", + password="password1", + imap_server="server.example.com", + imap_port=443, + imap_security=MailAccount.ImapSecurity.SSL, + character_set="UTF-8", + owner=other_user, + ) + + response = self.client.post( + self.ENDPOINT, + data={ + "name": "Rule1", + "account": foreign_account.pk, + "folder": "INBOX", + "filter_from": "from@example.com", + "maximum_age": 30, + "action": MailRule.MailAction.MARK_READ, + "assign_title_from": MailRule.TitleSource.FROM_SUBJECT, + "assign_correspondent_from": MailRule.CorrespondentSource.FROM_NOTHING, + "order": 0, + "attachment_type": MailRule.AttachmentProcessing.ATTACHMENTS_ONLY, + }, + ) + + self.assertEqual(response.status_code, status.HTTP_403_FORBIDDEN) + self.assertEqual(MailRule.objects.count(), 0) + + def test_create_mail_rule_allowed_for_granted_account_change_permission(self): + other_user = User.objects.create_user(username="mail-owner") + foreign_account = MailAccount.objects.create( + name="ForeignEmail", + username="username1", + password="password1", + imap_server="server.example.com", + imap_port=443, + imap_security=MailAccount.ImapSecurity.SSL, + character_set="UTF-8", + owner=other_user, + ) + assign_perm("change_mailaccount", self.user, foreign_account) + + response = self.client.post( + self.ENDPOINT, + data={ + "name": "Rule1", + "account": foreign_account.pk, + "folder": "INBOX", + "filter_from": "from@example.com", + "maximum_age": 30, + "action": MailRule.MailAction.MARK_READ, + "assign_title_from": MailRule.TitleSource.FROM_SUBJECT, + "assign_correspondent_from": MailRule.CorrespondentSource.FROM_NOTHING, + "order": 0, + "attachment_type": MailRule.AttachmentProcessing.ATTACHMENTS_ONLY, + }, + ) + + self.assertEqual(response.status_code, status.HTTP_201_CREATED) + self.assertEqual(MailRule.objects.get().account, foreign_account) + + def test_update_mail_rule_forbidden_for_unpermitted_account(self): + own_account = MailAccount.objects.create( + name="Email1", + username="username1", + password="password1", + imap_server="server.example.com", + imap_port=443, + imap_security=MailAccount.ImapSecurity.SSL, + character_set="UTF-8", + ) + other_user = User.objects.create_user(username="mail-owner") + foreign_account = MailAccount.objects.create( + name="ForeignEmail", + username="username2", + password="password2", + imap_server="server.example.com", + imap_port=443, + imap_security=MailAccount.ImapSecurity.SSL, + character_set="UTF-8", + owner=other_user, + ) + rule1 = MailRule.objects.create( + name="Rule1", + account=own_account, + folder="INBOX", + filter_from="from@example.com", + maximum_age=30, + action=MailRule.MailAction.MARK_READ, + assign_title_from=MailRule.TitleSource.FROM_SUBJECT, + assign_correspondent_from=MailRule.CorrespondentSource.FROM_NOTHING, + order=0, + attachment_type=MailRule.AttachmentProcessing.ATTACHMENTS_ONLY, + ) + + response = self.client.patch( + f"{self.ENDPOINT}{rule1.pk}/", + data={"account": foreign_account.pk}, + ) + + self.assertEqual(response.status_code, status.HTTP_403_FORBIDDEN) + rule1.refresh_from_db() + self.assertEqual(rule1.account, own_account) + def test_get_mail_rules_owner_aware(self): """ GIVEN: From f84e0097e5b3a5383c662759ae0a780f5d63d416 Mon Sep 17 00:00:00 2001 From: shamoon <4887959+shamoon@users.noreply.github.com> Date: Sat, 21 Mar 2026 00:55:36 -0700 Subject: [PATCH 2/4] Fix validate document link targets --- src/documents/serialisers.py | 43 +++++++-- src/documents/tests/test_api_bulk_edit.py | 44 +++++++++ src/documents/tests/test_api_custom_fields.py | 95 +++++++++++++++++++ 3 files changed, 176 insertions(+), 6 deletions(-) diff --git a/src/documents/serialisers.py b/src/documents/serialisers.py index 647e0e8b5..ea8cc70b6 100644 --- a/src/documents/serialisers.py +++ b/src/documents/serialisers.py @@ -853,6 +853,25 @@ class ReadWriteSerializerMethodField(serializers.SerializerMethodField): return {self.field_name: data} +def validate_documentlink_targets(user, doc_ids): + if Document.objects.filter(id__in=doc_ids).count() != len(doc_ids): + raise serializers.ValidationError( + "Some documents in value don't exist or were specified twice.", + ) + + 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 + ): + raise PermissionDenied( + _("Insufficient permissions."), + ) + + class CustomFieldInstanceSerializer(serializers.ModelSerializer): field = serializers.PrimaryKeyRelatedField(queryset=CustomField.objects.all()) value = ReadWriteSerializerMethodField(allow_null=True) @@ -943,12 +962,11 @@ class CustomFieldInstanceSerializer(serializers.ModelSerializer): "Value must be a list", ) doc_ids = data["value"] - if Document.objects.filter(id__in=doc_ids).count() != len( - data["value"], - ): - raise serializers.ValidationError( - "Some documents in value don't exist or were specified twice.", - ) + request = self.context.get("request") + validate_documentlink_targets( + getattr(request, "user", None) if request is not None else None, + doc_ids, + ) return data @@ -1498,6 +1516,19 @@ class BulkEditSerializer( f"Some custom fields in {name} don't exist or were specified twice.", ) + if isinstance(custom_fields, dict): + custom_field_map = CustomField.objects.in_bulk(ids) + for raw_field_id, value in custom_fields.items(): + field = custom_field_map.get(int(raw_field_id)) + if ( + field is not None + and field.data_type == CustomField.FieldDataType.DOCUMENTLINK + and value is not None + ): + if not isinstance(value, list): + raise serializers.ValidationError("Value must be a list") + validate_documentlink_targets(self.user, value) + def validate_method(self, method): if method == "set_correspondent": return bulk_edit.set_correspondent diff --git a/src/documents/tests/test_api_bulk_edit.py b/src/documents/tests/test_api_bulk_edit.py index be7a81cd4..8400372a7 100644 --- a/src/documents/tests/test_api_bulk_edit.py +++ b/src/documents/tests/test_api_bulk_edit.py @@ -262,6 +262,50 @@ class TestBulkEditAPI(DirectoriesMixin, APITestCase): self.assertEqual(kwargs["add_custom_fields"], [self.cf1.id]) self.assertEqual(kwargs["remove_custom_fields"], [self.cf2.id]) + @mock.patch("documents.serialisers.bulk_edit.modify_custom_fields") + def test_api_modify_custom_fields_documentlink_forbidden_for_unpermitted_target( + self, + m, + ): + self.setup_mock(m, "modify_custom_fields") + user = User.objects.create_user(username="doc-owner") + user.user_permissions.add(Permission.objects.get(codename="change_document")) + other_user = User.objects.create_user(username="other-user") + source_doc = Document.objects.create( + checksum="source", + title="Source", + owner=user, + ) + target_doc = Document.objects.create( + checksum="target", + title="Target", + owner=other_user, + ) + doclink_field = CustomField.objects.create( + name="doclink", + data_type=CustomField.FieldDataType.DOCUMENTLINK, + ) + + self.client.force_authenticate(user=user) + + response = self.client.post( + "/api/documents/bulk_edit/", + json.dumps( + { + "documents": [source_doc.id], + "method": "modify_custom_fields", + "parameters": { + "add_custom_fields": {doclink_field.id: [target_doc.id]}, + "remove_custom_fields": [], + }, + }, + ), + content_type="application/json", + ) + + self.assertEqual(response.status_code, status.HTTP_403_FORBIDDEN) + m.assert_not_called() + @mock.patch("documents.serialisers.bulk_edit.modify_custom_fields") def test_api_modify_custom_fields_with_values(self, m): self.setup_mock(m, "modify_custom_fields") diff --git a/src/documents/tests/test_api_custom_fields.py b/src/documents/tests/test_api_custom_fields.py index 8cc8f2cb2..998bc445a 100644 --- a/src/documents/tests/test_api_custom_fields.py +++ b/src/documents/tests/test_api_custom_fields.py @@ -6,6 +6,7 @@ from unittest.mock import ANY from django.contrib.auth.models import Permission from django.contrib.auth.models import User from django.test import override_settings +from guardian.shortcuts import assign_perm from rest_framework import status from rest_framework.test import APITestCase @@ -1247,6 +1248,100 @@ class TestCustomFieldsAPI(DirectoriesMixin, APITestCase): self.assertEqual(resp.status_code, status.HTTP_200_OK) self.assertEqual(doc5.custom_fields.first().value, [1]) + def test_documentlink_patch_requires_change_permission_on_target_documents(self): + source_owner = User.objects.create_user(username="source-owner") + source_owner.user_permissions.add( + Permission.objects.get(codename="change_document"), + ) + other_user = User.objects.create_user(username="other-user") + + source_doc = Document.objects.create( + title="Source", + checksum="source", + mime_type="application/pdf", + owner=source_owner, + ) + target_doc = Document.objects.create( + title="Target", + checksum="target", + mime_type="application/pdf", + owner=other_user, + ) + custom_field_doclink = CustomField.objects.create( + name="Test Custom Field Doc Link", + data_type=CustomField.FieldDataType.DOCUMENTLINK, + ) + + self.client.force_authenticate(user=source_owner) + + resp = self.client.patch( + f"/api/documents/{source_doc.id}/", + data={ + "custom_fields": [ + { + "field": custom_field_doclink.id, + "value": [target_doc.id], + }, + ], + }, + format="json", + ) + + self.assertEqual(resp.status_code, status.HTTP_403_FORBIDDEN) + self.assertEqual( + CustomFieldInstance.objects.filter(field=custom_field_doclink).count(), + 0, + ) + + def test_documentlink_patch_allowed_with_change_permission_on_target_documents( + self, + ): + source_owner = User.objects.create_user(username="source-owner") + source_owner.user_permissions.add( + Permission.objects.get(codename="change_document"), + ) + other_user = User.objects.create_user(username="other-user") + + source_doc = Document.objects.create( + title="Source", + checksum="source", + mime_type="application/pdf", + owner=source_owner, + ) + target_doc = Document.objects.create( + title="Target", + checksum="target", + mime_type="application/pdf", + owner=other_user, + ) + custom_field_doclink = CustomField.objects.create( + name="Test Custom Field Doc Link", + data_type=CustomField.FieldDataType.DOCUMENTLINK, + ) + + assign_perm("change_document", source_owner, target_doc) + self.client.force_authenticate(user=source_owner) + + resp = self.client.patch( + f"/api/documents/{source_doc.id}/", + data={ + "custom_fields": [ + { + "field": custom_field_doclink.id, + "value": [target_doc.id], + }, + ], + }, + format="json", + ) + + self.assertEqual(resp.status_code, status.HTTP_200_OK) + target_doc.refresh_from_db() + self.assertEqual( + target_doc.custom_fields.get(field=custom_field_doclink).value, + [source_doc.id], + ) + def test_custom_field_filters(self): custom_field_string = CustomField.objects.create( name="Test Custom Field String", From 3cbdf5d0b7a4aa59aa00cdba274f7734e43a02f7 Mon Sep 17 00:00:00 2001 From: shamoon <4887959+shamoon@users.noreply.github.com> Date: Sat, 21 Mar 2026 01:20:59 -0700 Subject: [PATCH 3/4] Fix: require view permission for more-like search --- src/documents/tests/test_api_search.py | 52 ++++++++++++++++++++++++++ src/documents/views.py | 25 ++++++++++++- 2 files changed, 75 insertions(+), 2 deletions(-) diff --git a/src/documents/tests/test_api_search.py b/src/documents/tests/test_api_search.py index d671fe546..79fae018a 100644 --- a/src/documents/tests/test_api_search.py +++ b/src/documents/tests/test_api_search.py @@ -772,6 +772,58 @@ class TestDocumentSearchApi(DirectoriesMixin, APITestCase): self.assertEqual(results[0]["id"], d3.id) self.assertEqual(results[1]["id"], d1.id) + def test_search_more_like_requires_view_permission_on_seed_document(self): + """ + GIVEN: + - A user can search documents they own + - Another user's private document exists with similar content + WHEN: + - The user requests more-like-this for the private seed document + THEN: + - The request is rejected + """ + owner = User.objects.create_user("owner") + attacker = User.objects.create_user("attacker") + attacker.user_permissions.add( + Permission.objects.get(codename="view_document"), + ) + + private_seed = Document.objects.create( + title="private bank statement", + content="quarterly treasury bank statement wire transfer", + checksum="seed", + owner=owner, + pk=10, + ) + visible_doc = Document.objects.create( + title="attacker-visible match", + content="quarterly treasury bank statement wire transfer summary", + checksum="visible", + owner=attacker, + pk=11, + ) + other_doc = Document.objects.create( + title="unrelated", + content="completely different topic", + checksum="other", + owner=attacker, + pk=12, + ) + + with AsyncWriter(index.open_index()) as writer: + index.update_document(writer, private_seed) + index.update_document(writer, visible_doc) + index.update_document(writer, other_doc) + + self.client.force_authenticate(user=attacker) + + response = self.client.get( + f"/api/documents/?more_like_id={private_seed.id}", + ) + + self.assertEqual(response.status_code, status.HTTP_403_FORBIDDEN) + self.assertEqual(response.content, b"Insufficient permissions.") + def test_search_filtering(self): t = Tag.objects.create(name="tag") t2 = Tag.objects.create(name="tag2") diff --git a/src/documents/views.py b/src/documents/views.py index e8a929859..5985c17a8 100644 --- a/src/documents/views.py +++ b/src/documents/views.py @@ -49,6 +49,7 @@ from django.utils import timezone from django.utils.decorators import method_decorator from django.utils.timezone import make_aware from django.utils.translation import get_language +from django.utils.translation import gettext_lazy as _ from django.views import View from django.views.decorators.cache import cache_control from django.views.decorators.http import condition @@ -70,6 +71,7 @@ from rest_framework import parsers from rest_framework import serializers from rest_framework.decorators import action from rest_framework.exceptions import NotFound +from rest_framework.exceptions import PermissionDenied from rest_framework.exceptions import ValidationError from rest_framework.filters import OrderingFilter from rest_framework.filters import SearchFilter @@ -1369,11 +1371,28 @@ class UnifiedSearchViewSet(DocumentViewSet): filtered_queryset = super().filter_queryset(queryset) if self._is_search_request(): - from documents import index - if "query" in self.request.query_params: + from documents import index + query_class = index.DelayedFullTextQuery elif "more_like_id" in self.request.query_params: + try: + more_like_doc_id = int(self.request.query_params["more_like_id"]) + more_like_doc = Document.objects.select_related("owner").get( + pk=more_like_doc_id, + ) + except (TypeError, ValueError, Document.DoesNotExist): + raise PermissionDenied(_("Invalid more_like_id")) + + if not has_perms_owner_aware( + self.request.user, + "view_document", + more_like_doc, + ): + raise PermissionDenied(_("Insufficient permissions.")) + + from documents import index + query_class = index.DelayedMoreLikeThisQuery else: raise ValueError @@ -1409,6 +1428,8 @@ class UnifiedSearchViewSet(DocumentViewSet): return response except NotFound: raise + except PermissionDenied as e: + return HttpResponseForbidden(str(e.detail)) except Exception as e: logger.warning(f"An error occurred listing search results: {e!s}") return HttpResponseBadRequest( From cc71aad058bded68f5cbe31b4be379f1644c4f2c Mon Sep 17 00:00:00 2001 From: shamoon <4887959+shamoon@users.noreply.github.com> Date: Sat, 21 Mar 2026 01:24:23 -0700 Subject: [PATCH 4/4] Fix: suggest corrections only if visible results --- src/documents/index.py | 9 ++++++- src/documents/tests/test_api_search.py | 34 ++++++++++++++++++++++++++ 2 files changed, 42 insertions(+), 1 deletion(-) diff --git a/src/documents/index.py b/src/documents/index.py index 8afc31fe9..f6e0821db 100644 --- a/src/documents/index.py +++ b/src/documents/index.py @@ -470,7 +470,14 @@ class DelayedFullTextQuery(DelayedQuery): try: corrected = self.searcher.correct_query(q, q_str) if corrected.string != q_str: - suggested_correction = corrected.string + corrected_results = self.searcher.search( + corrected.query, + limit=1, + filter=MappedDocIdSet(self.filter_queryset, self.searcher.ixreader), + scored=False, + ) + if len(corrected_results) > 0: + suggested_correction = corrected.string except Exception as e: logger.info( "Error while correcting query %s: %s", diff --git a/src/documents/tests/test_api_search.py b/src/documents/tests/test_api_search.py index 79fae018a..190e0c431 100644 --- a/src/documents/tests/test_api_search.py +++ b/src/documents/tests/test_api_search.py @@ -702,6 +702,40 @@ class TestDocumentSearchApi(DirectoriesMixin, APITestCase): self.assertEqual(correction, None) + def test_search_spelling_suggestion_suppressed_for_private_terms(self): + owner = User.objects.create_user("owner") + attacker = User.objects.create_user("attacker") + attacker.user_permissions.add( + Permission.objects.get(codename="view_document"), + ) + + with AsyncWriter(index.open_index()) as writer: + for i in range(55): + private_doc = Document.objects.create( + checksum=f"p{i}", + pk=100 + i, + title=f"Private Document {i + 1}", + content=f"treasury document {i + 1}", + owner=owner, + ) + visible_doc = Document.objects.create( + checksum=f"v{i}", + pk=200 + i, + title=f"Visible Document {i + 1}", + content=f"public ledger {i + 1}", + owner=attacker, + ) + index.update_document(writer, private_doc) + index.update_document(writer, visible_doc) + + self.client.force_authenticate(user=attacker) + + response = self.client.get("/api/documents/?query=treasurx") + + self.assertEqual(response.status_code, status.HTTP_200_OK) + self.assertEqual(response.data["count"], 0) + self.assertIsNone(response.data["corrected_query"]) + @mock.patch( "whoosh.searching.Searcher.correct_query", side_effect=Exception("Test error"),