Compare commits

...
Author SHA1 Message Date
shamoon 320cf98254 documents_without_permitted_root helper used x4 2026-10-08 09:35:14 -07:00
shamoon 7588cdb952 include selectiondataview too 2026-10-08 09:28:44 -07:00
shamoon 0e27a1a327 documentfactory 2026-10-08 09:28:02 -07:00
shamoon 9821e4f73c Trash fix 2026-10-07 16:08:43 -07:00
shamoon 31776986e5 ai chat too 2026-10-07 16:03:13 -07:00
shamoon 94ae632920 Update test 2026-10-07 15:52:58 -07:00
shamoon 56d5bb7255 ai_suggestions 2026-10-07 15:52:31 -07:00
shamoon 2df81ce44e Fix: consistently use root doc for version action permissions 2026-10-07 15:37:01 -07:00
6 changed files with 320 additions and 47 deletions

No files matched your search

+24
View File
@@ -14,6 +14,7 @@ from django.db.models import QuerySet
from django.db.models import Value from django.db.models import Value
from django.db.models import When from django.db.models import When
from django.db.models.functions import Cast from django.db.models.functions import Cast
from django.db.models.functions import Coalesce
from guardian.core import ObjectPermissionChecker from guardian.core import ObjectPermissionChecker
from guardian.models import GroupObjectPermission from guardian.models import GroupObjectPermission
from guardian.models import UserObjectPermission from guardian.models import UserObjectPermission
@@ -474,6 +475,29 @@ def permitted_document_ids(
return permitted_object_ids(user, Document, perm, include_deleted=include_deleted) return permitted_object_ids(user, Document, perm, include_deleted=include_deleted)
def documents_without_permitted_root(
documents: QuerySet[Document],
user: User | None,
*,
perm: str = "view_document",
include_deleted: bool = False,
) -> QuerySet[Document]:
"""
The documents the user lacks ``perm`` on. Versions are authorized by their
root document, so a version's own owner is ignored. A single query, without
loading the documents or joining the root.
"""
return documents.annotate(
root_id=Coalesce("root_document_id", "id"),
).exclude(
root_id__in=permitted_document_ids(
user,
perm=perm,
include_deleted=include_deleted,
),
)
def get_document_count_filter_for_user(user, related_name: str = "documents"): def get_document_count_filter_for_user(user, related_name: str = "documents"):
""" """
Return the Q object used to filter document counts for the given user. Return the Q object used to filter document counts for the given user.
+2 -1
View File
@@ -90,6 +90,7 @@ from documents.templating.utils import convert_format_str_to_template_format
from documents.templating.workflows import validate_workflow_template from documents.templating.workflows import validate_workflow_template
from documents.validators import uri_validator from documents.validators import uri_validator
from documents.validators import url_validator from documents.validators import url_validator
from documents.versioning import get_root_document
from documents.versioning import has_prefetched_effective_content from documents.versioning import has_prefetched_effective_content
from documents.versioning import sort_versions_newest_first from documents.versioning import sort_versions_newest_first
@@ -2894,7 +2895,7 @@ class ShareLinkSerializer(OwnedObjectSerializer):
and has_perms_owner_aware( and has_perms_owner_aware(
self.user, self.user,
"view_document", "view_document",
document, get_root_document(document),
) )
): ):
return document return document
@@ -8,6 +8,7 @@ from auditlog.models import LogEntry # type: ignore[import-untyped]
from django.contrib.contenttypes.models import ContentType from django.contrib.contenttypes.models import ContentType
from django.core.files.uploadedfile import SimpleUploadedFile from django.core.files.uploadedfile import SimpleUploadedFile
from django.test import TestCase as DjangoTestCase from django.test import TestCase as DjangoTestCase
from django.test import override_settings
from django.utils import timezone from django.utils import timezone
from rest_framework import status from rest_framework import status
from rest_framework.test import APITestCase from rest_framework.test import APITestCase
@@ -16,13 +17,17 @@ from documents.data_models import DocumentSource
from documents.filters import EffectiveContentFilter from documents.filters import EffectiveContentFilter
from documents.filters import TitleContentFilter from documents.filters import TitleContentFilter
from documents.models import Document from documents.models import Document
from documents.models import Note
from documents.models import ShareLink
from documents.versioning import annotate_effective_content from documents.versioning import annotate_effective_content
from documents.views import DocumentSelectionMixin from documents.views import DocumentSelectionMixin
from paperless_testing.dirs import DirectoriesMixin from paperless_testing.dirs import DirectoriesMixin
from paperless_testing.factories import DocumentFactory from paperless_testing.factories import DocumentFactory
from paperless_testing.factories import UserFactory from paperless_testing.factories import UserFactory
from paperless_testing.http import read_streaming_response from paperless_testing.http import read_streaming_response
from paperless_testing.permissions import grant_all_global
from paperless_testing.permissions import grant_global from paperless_testing.permissions import grant_global
from paperless_testing.permissions import grant_object
if TYPE_CHECKING: if TYPE_CHECKING:
from pathlib import Path from pathlib import Path
@@ -1043,3 +1048,152 @@ class TestBulkSelectionExcludesVersions(DjangoTestCase):
) )
self.assertEqual(selected, [root.id]) self.assertEqual(selected, [root.id])
class TestVersionActionPermissions(DirectoriesMixin, APITestCase):
def setUp(self):
super().setUp()
self.user = UserFactory()
grant_all_global(self.user)
self.client.force_authenticate(self.user)
self.root = DocumentFactory(owner=UserFactory())
self.version = DocumentFactory(root_document=self.root, owner=None)
@override_settings(AUDIT_LOG_ENABLED=True)
def test_actions_reject_stale_version_ownership(self):
note = Note.objects.create(document=self.version, note="Version note")
for owner in (None, self.user):
self.version.owner = owner
self.version.save(update_fields=["owner"])
for action in (
"notes",
"suggestions",
"ai_suggestions",
"history",
"share_links",
):
with self.subTest(owner=owner, action=action):
response = self.client.get(
f"/api/documents/{self.version.pk}/{action}/",
)
self.assertEqual(response.status_code, 403)
response = self.client.post(
f"/api/documents/{self.version.pk}/notes/",
{"note": "New note"},
)
self.assertEqual(response.status_code, 403)
response = self.client.delete(
f"/api/documents/{self.version.pk}/notes/?id={note.pk}",
)
self.assertEqual(response.status_code, 403)
response = self.client.post(
"/api/share_links/",
{"document": self.version.pk, "file_version": "original"},
)
self.assertEqual(response.status_code, 403)
response = self.client.post(
"/api/share_link_bundles/",
{"document_ids": [self.version.pk], "file_version": "original"},
format="json",
)
self.assertEqual(response.status_code, 400)
response = self.client.post(
"/api/documents/email/",
{
"documents": [self.version.pk],
"addresses": "recipient@example.com",
"subject": "Version",
"message": "Version",
},
format="json",
)
self.assertEqual(response.status_code, 403)
with (
mock.patch("documents.views.AIConfig") as ai_config,
mock.patch("documents.views.stream_chat_with_documents") as chat,
):
ai_config.return_value.ai_enabled = True
response = self.client.post(
"/api/documents/chat/",
{"q": "Version?", "document_id": self.version.pk},
format="json",
)
self.assertEqual(response.status_code, 403)
chat.assert_not_called()
self.assertTrue(Note.objects.filter(pk=note.pk).exists())
self.assertFalse(ShareLink.objects.exists())
@mock.patch("documents.views.build_share_link_bundle.apply_async")
def test_root_permissions_allow_sharing_a_private_version(self, build_mock):
self.version.owner = UserFactory()
self.version.save(update_fields=["owner"])
grant_object(self.user, self.root, "view_document", "change_document")
note = Note.objects.create(document=self.version, note="Version note")
response = self.client.get(f"/api/documents/{self.version.pk}/notes/")
self.assertEqual(response.status_code, 200)
self.assertEqual(response.data[0]["id"], note.pk)
response = self.client.post(
"/api/share_links/",
{"document": self.version.pk, "file_version": "original"},
)
self.assertEqual(response.status_code, 201)
self.assertEqual(ShareLink.objects.get().document_id, self.version.pk)
response = self.client.get(f"/api/documents/{self.version.pk}/share_links/")
self.assertEqual(response.status_code, 200)
self.assertEqual(len(response.data), 1)
response = self.client.post(
"/api/share_link_bundles/",
{"document_ids": [self.version.pk], "file_version": "original"},
format="json",
)
self.assertEqual(response.status_code, 201)
build_mock.assert_called_once()
def test_root_view_permission_does_not_allow_note_changes(self):
grant_object(self.user, self.root, "view_document")
note = Note.objects.create(document=self.version, note="Version note")
response = self.client.get(f"/api/documents/{self.version.pk}/notes/")
self.assertEqual(response.status_code, 200)
response = self.client.post(
f"/api/documents/{self.version.pk}/notes/",
{"note": "New note"},
)
self.assertEqual(response.status_code, 403)
response = self.client.delete(
f"/api/documents/{self.version.pk}/notes/?id={note.pk}",
)
self.assertEqual(response.status_code, 403)
self.assertTrue(Note.objects.filter(pk=note.pk).exists())
@override_settings(AUDIT_LOG_ENABLED=True)
def test_history_uses_root_ownership(self):
self.root.owner = self.user
self.root.save(update_fields=["owner"])
self.version.owner = UserFactory()
self.version.save(update_fields=["owner"])
response = self.client.get(f"/api/documents/{self.version.pk}/history/")
self.assertEqual(response.status_code, 200)
def test_selection_data_rejects_stale_version_ownership(self):
for owner in (None, self.user):
self.version.owner = owner
self.version.save(update_fields=["owner"])
with self.subTest(owner=owner):
response = self.client.post(
"/api/documents/selection_data/",
{"documents": [self.version.pk]},
format="json",
)
self.assertEqual(response.status_code, 403)
def test_selection_data_allows_private_version_of_permitted_root(self):
self.version.owner = UserFactory()
self.version.save(update_fields=["owner"])
grant_object(self.user, self.root, "view_document")
other = DocumentFactory(owner=self.user)
response = self.client.post(
"/api/documents/selection_data/",
{"documents": [self.version.pk, other.pk]},
format="json",
)
self.assertEqual(response.status_code, 200)
+51
View File
@@ -6,6 +6,7 @@ from rest_framework.test import APITestCase
from documents.models import Document from documents.models import Document
from paperless_testing.dirs import DirectoriesMixin from paperless_testing.dirs import DirectoriesMixin
from paperless_testing.factories import DocumentFactory
from paperless_testing.factories import UserFactory from paperless_testing.factories import UserFactory
from paperless_testing.permissions import grant_all_global from paperless_testing.permissions import grant_all_global
@@ -279,3 +280,53 @@ class TestTrashAPI(DirectoriesMixin, APITestCase):
Document.objects.filter(root_document=root).values_list("id", flat=True), Document.objects.filter(root_document=root).values_list("id", flat=True),
[version.pk for version in versions], [version.pk for version in versions],
) )
def test_api_trash_version_follows_root_owner(self) -> None:
"""
GIVEN:
- A deleted version of user2's document, owned by nobody
- A deleted version of the user's document, owned by user2
WHEN:
- The user lists the trash and tries to restore or empty the versions
THEN:
- Only the version of the user's own document is listed
- The other version can't be restored or emptied
- The version of the user's own document can be restored
"""
user2 = UserFactory(username="user2")
other_version = DocumentFactory(
root_document=DocumentFactory(owner=user2),
version_index=1,
)
other_version.delete()
own_version = DocumentFactory(
owner=user2,
root_document=DocumentFactory(owner=self.user),
version_index=1,
)
own_version.delete()
resp = self.client.get("/api/trash/")
self.assertEqual(resp.status_code, status.HTTP_200_OK)
self.assertEqual(
[doc["id"] for doc in resp.data["results"]],
[own_version.pk],
)
for action in ("restore", "empty"):
with self.subTest(action=action):
resp = self.client.post(
"/api/trash/",
{"action": action, "documents": [other_version.pk]},
)
self.assertEqual(resp.status_code, status.HTTP_403_FORBIDDEN)
self.assertTrue(
Document.deleted_objects.filter(pk=other_version.pk).exists(),
)
resp = self.client.post(
"/api/trash/",
{"action": "restore", "documents": [own_version.pk]},
)
self.assertEqual(resp.status_code, status.HTTP_200_OK)
self.assertTrue(Document.objects.filter(pk=own_version.pk).exists())
@@ -90,10 +90,13 @@ class ShareLinkBundleAPITests(DirectoriesMixin, APITestCase):
self.assertEqual(response.status_code, status.HTTP_400_BAD_REQUEST) self.assertEqual(response.status_code, status.HTTP_400_BAD_REQUEST)
self.assertIn("document_ids", response.data) self.assertIn("document_ids", response.data)
@mock.patch("documents.views.permitted_document_ids", return_value=set()) def test_create_bundle_rejects_insufficient_permissions(self) -> None:
def test_create_bundle_rejects_insufficient_permissions(self, perms_mock) -> None: requester = UserFactory(username="bundle_creator")
grant_global(requester, "add_sharelinkbundle", "view_document")
self.client.force_authenticate(requester)
document = DocumentFactory(owner=UserFactory(username="document_owner"))
payload = { payload = {
"document_ids": [self.document.pk], "document_ids": [self.document.pk, document.pk],
"file_version": ShareLink.FileVersion.ARCHIVE, "file_version": ShareLink.FileVersion.ARCHIVE,
"expiration_days": 7, "expiration_days": 7,
} }
@@ -101,8 +104,8 @@ class ShareLinkBundleAPITests(DirectoriesMixin, APITestCase):
response = self.client.post(self.ENDPOINT, payload, format="json") response = self.client.post(self.ENDPOINT, payload, format="json")
self.assertEqual(response.status_code, status.HTTP_400_BAD_REQUEST) self.assertEqual(response.status_code, status.HTTP_400_BAD_REQUEST)
self.assertIn("document_ids", response.data) self.assertIn(str(document.pk), str(response.data["document_ids"]))
perms_mock.assert_called() self.assertFalse(ShareLinkBundle.objects.exists())
@mock.patch("documents.views.build_share_link_bundle.apply_async") @mock.patch("documents.views.build_share_link_bundle.apply_async")
def test_rebuild_bundle_resets_state(self, delay_mock) -> None: def test_rebuild_bundle_resets_state(self, delay_mock) -> None:
+81 -41
View File
@@ -174,6 +174,7 @@ from documents.permissions import TrashPermissions
from documents.permissions import ViewDocumentsPermissions from documents.permissions import ViewDocumentsPermissions
from documents.permissions import annotate_document_count_by_ids from documents.permissions import annotate_document_count_by_ids
from documents.permissions import annotate_document_count_for_related_queryset from documents.permissions import annotate_document_count_for_related_queryset
from documents.permissions import documents_without_permitted_root
from documents.permissions import get_document_count_filter_for_user from documents.permissions import get_document_count_filter_for_user
from documents.permissions import get_objects_for_user_owner_aware from documents.permissions import get_objects_for_user_owner_aware
from documents.permissions import has_global_statistics_permission from documents.permissions import has_global_statistics_permission
@@ -1550,13 +1551,16 @@ class DocumentViewSet(
) )
def suggestions(self, request, pk=None): def suggestions(self, request, pk=None):
doc = get_object_or_404( doc = get_object_or_404(
Document.objects.select_related("owner").prefetch_related("versions"), Document.objects.select_related(
"owner",
"root_document__owner",
).prefetch_related("versions"),
pk=pk, pk=pk,
) )
if request.user is not None and not has_perms_owner_aware( if request.user is not None and not has_perms_owner_aware(
request.user, request.user,
"change_document", "change_document",
doc, get_root_document(doc),
): ):
return HttpResponseForbidden("Insufficient permissions") return HttpResponseForbidden("Insufficient permissions")
@@ -1610,13 +1614,16 @@ class DocumentViewSet(
@method_decorator(cache_control(no_cache=True)) @method_decorator(cache_control(no_cache=True))
def ai_suggestions(self, request, pk=None): def ai_suggestions(self, request, pk=None):
doc = get_object_or_404( doc = get_object_or_404(
Document.objects.select_related("owner").prefetch_related("versions"), Document.objects.select_related(
"owner",
"root_document__owner",
).prefetch_related("versions"),
pk=pk, pk=pk,
) )
if request.user is not None and not has_perms_owner_aware( if request.user is not None and not has_perms_owner_aware(
request.user, request.user,
"change_document", "change_document",
doc, get_root_document(doc),
): ):
return HttpResponseForbidden("Insufficient permissions") return HttpResponseForbidden("Insufficient permissions")
@@ -1856,15 +1863,20 @@ class DocumentViewSet(
currentUser = request.user currentUser = request.user
try: try:
doc = ( doc = (
Document.objects.select_related("owner") Document.objects.select_related("owner", "root_document__owner")
.prefetch_related("notes") .prefetch_related("notes")
.only("pk", "owner__id") .only(
"pk",
"owner__id",
"root_document__id",
"root_document__owner__id",
)
.get(pk=pk) .get(pk=pk)
) )
if currentUser is not None and not has_perms_owner_aware( if currentUser is not None and not has_perms_owner_aware(
currentUser, currentUser,
"view_document", "view_document",
doc, get_root_document(doc),
): ):
return HttpResponseForbidden("Insufficient permissions to view notes") return HttpResponseForbidden("Insufficient permissions to view notes")
except Document.DoesNotExist: except Document.DoesNotExist:
@@ -1886,7 +1898,7 @@ class DocumentViewSet(
if currentUser is not None and not has_perms_owner_aware( if currentUser is not None and not has_perms_owner_aware(
currentUser, currentUser,
"change_document", "change_document",
doc, get_root_document(doc),
): ):
return HttpResponseForbidden( return HttpResponseForbidden(
"Insufficient permissions to create notes", "Insufficient permissions to create notes",
@@ -1929,7 +1941,7 @@ class DocumentViewSet(
if currentUser is not None and not has_perms_owner_aware( if currentUser is not None and not has_perms_owner_aware(
currentUser, currentUser,
"change_document", "change_document",
doc, get_root_document(doc),
): ):
return HttpResponseForbidden("Insufficient permissions to delete notes") return HttpResponseForbidden("Insufficient permissions to delete notes")
@@ -1973,11 +1985,13 @@ class DocumentViewSet(
def share_links(self, request, pk=None): def share_links(self, request, pk=None):
currentUser = request.user currentUser = request.user
try: try:
doc = Document.objects.select_related("owner").get(pk=pk) doc = Document.objects.select_related("owner", "root_document__owner").get(
pk=pk,
)
if currentUser is not None and not has_perms_owner_aware( if currentUser is not None and not has_perms_owner_aware(
currentUser, currentUser,
"change_document", "change_document",
doc, get_root_document(doc),
): ):
return HttpResponseForbidden( return HttpResponseForbidden(
"Insufficient permissions to add share link", "Insufficient permissions to add share link",
@@ -2008,10 +2022,11 @@ class DocumentViewSet(
if not settings.AUDIT_LOG_ENABLED: if not settings.AUDIT_LOG_ENABLED:
return HttpResponseBadRequest("Audit log is disabled") return HttpResponseBadRequest("Audit log is disabled")
try: try:
doc = Document.objects.get(pk=pk) doc = Document.objects.select_related("root_document__owner").get(pk=pk)
root_doc = get_root_document(doc)
if not request.user.has_perm("auditlog.view_logentry") or ( if not request.user.has_perm("auditlog.view_logentry") or (
doc.owner is not None root_doc.owner is not None
and doc.owner != request.user and root_doc.owner != request.user
and not request.user.is_superuser and not request.user.is_superuser
): ):
return HttpResponseForbidden( return HttpResponseForbidden(
@@ -2102,9 +2117,7 @@ class DocumentViewSet(
documents = Document.objects.filter(pk__in=document_ids) documents = Document.objects.filter(pk__in=document_ids)
if ( if (
request.user is not None request.user is not None
and documents.exclude( and documents_without_permitted_root(documents, request.user).exists()
pk__in=permitted_document_ids(request.user),
).exists()
): ):
return HttpResponseForbidden("Insufficient permissions") return HttpResponseForbidden("Insufficient permissions")
@@ -2430,11 +2443,17 @@ class ChatStreamingView(GenericAPIView[Any]):
if doc_id: if doc_id:
try: try:
document = Document.objects.get(id=doc_id) document = Document.objects.select_related(
"root_document__owner",
).get(id=doc_id)
except Document.DoesNotExist: except Document.DoesNotExist:
return HttpResponseBadRequest("Document not found") return HttpResponseBadRequest("Document not found")
if not has_perms_owner_aware(request.user, "view_document", document): if not has_perms_owner_aware(
request.user,
"view_document",
get_root_document(document),
):
return HttpResponseForbidden("Insufficient permissions") return HttpResponseForbidden("Insufficient permissions")
documents = Document.objects.filter(pk=document.pk) documents = Document.objects.filter(pk=document.pk)
@@ -3605,10 +3624,11 @@ class SelectionDataView(DocumentSelectionMixin, GenericAPIView[Any]):
user=request.user, user=request.user,
validated_data=serializer.validated_data, validated_data=serializer.validated_data,
) )
permitted_documents = Document.objects.filter( documents = Document.objects.filter(pk__in=ids)
id__in=permitted_document_ids(request.user), if (
) documents.count() != len(ids)
if permitted_documents.filter(pk__in=ids).count() != len(ids): or documents_without_permitted_root(documents, request.user).exists()
):
return HttpResponseForbidden("Insufficient permissions") return HttpResponseForbidden("Insufficient permissions")
correspondents = Correspondent.objects.annotate( correspondents = Correspondent.objects.annotate(
@@ -4790,19 +4810,23 @@ class ShareLinkBundleViewSet(PassUserMixin, ModelViewSet[ShareLinkBundle]):
}, },
) )
documents = list(documents_qs) denied_id = (
permitted_ids = set(permitted_document_ids(request.user)) documents_without_permitted_root(documents_qs, request.user)
for document in documents: .order_by("pk")
if document.pk not in permitted_ids: .values_list("pk", flat=True)
raise ValidationError( .first()
{ )
"document_ids": _( if denied_id is not None:
"Insufficient permissions to share document %(id)s.", raise ValidationError(
) {
% {"id": document.pk}, "document_ids": _(
}, "Insufficient permissions to share document %(id)s.",
) )
% {"id": denied_id},
},
)
documents = list(documents_qs)
document_map = {document.pk: document for document in documents} document_map = {document.pk: document for document in documents}
ordered_documents = [document_map[doc_id] for doc_id in document_ids] ordered_documents = [document_map[doc_id] for doc_id in document_ids]
@@ -5587,6 +5611,23 @@ class TrashView(ListModelMixin, PassUserMixin):
class _TrashPermittedObjectsFilter(PermittedObjectsFilter): class _TrashPermittedObjectsFilter(PermittedObjectsFilter):
include_granted = False include_granted = False
def filter_queryset(self, request, queryset, view):
if request.user.is_superuser or not request.user.is_active:
return super().filter_queryset(request, queryset, view)
# A version belongs to whoever owns its root
def owned_or_unowned(prefix: str) -> Q:
return Q(**{f"{prefix}owner": request.user}) | Q(
**{f"{prefix}owner__isnull": True},
)
return queryset.filter(
(Q(root_document__isnull=True) & owned_or_unowned(""))
| (
Q(root_document__isnull=False) & owned_or_unowned("root_document__")
),
)
filter_backends = (_TrashPermittedObjectsFilter,) filter_backends = (_TrashPermittedObjectsFilter,)
pagination_class = StandardPagination pagination_class = StandardPagination
@@ -5616,12 +5657,11 @@ class TrashView(ListModelMixin, PassUserMixin):
if doc_ids is not None if doc_ids is not None
else self.filter_queryset(self.get_queryset()).all() else self.filter_queryset(self.get_queryset()).all()
) )
if docs.exclude( if documents_without_permitted_root(
pk__in=permitted_document_ids( docs,
request.user, request.user,
perm="delete_document", perm="delete_document",
include_deleted=True, include_deleted=True,
),
).exists(): ).exists():
return HttpResponseForbidden("Insufficient permissions") return HttpResponseForbidden("Insufficient permissions")
action = serializer.validated_data.get("action") action = serializer.validated_data.get("action")