diff --git a/src/documents/permissions.py b/src/documents/permissions.py index 35cfbe2e6..13d9425e9 100644 --- a/src/documents/permissions.py +++ b/src/documents/permissions.py @@ -30,6 +30,7 @@ from rest_framework.permissions import BasePermission from rest_framework.permissions import DjangoObjectPermissions from documents.models import Document +from documents.versioning import get_root_document class PaperlessObjectPermissions(DjangoObjectPermissions): @@ -676,7 +677,14 @@ def has_perms_owner_aware(user, perms, obj): single-object check still has many production callers. Several callers remain across ``documents/``, ``paperless_mail/``, and ``paperless_ai/`` -- grep for this function name before removing it. + + A document version is authorized by its root document, like in + ``permitted_document_ids``, so a version's own owner and grants never + matter. Fetch the root with ``select_related("root_document__owner")`` to + avoid extra queries. """ + if isinstance(obj, Document): + obj = get_root_document(obj) checker = ObjectPermissionChecker(user) return obj.owner is None or obj.owner == user or checker.has_perm(perms, obj) diff --git a/src/documents/serialisers.py b/src/documents/serialisers.py index ec0cf2812..c2c875029 100644 --- a/src/documents/serialisers.py +++ b/src/documents/serialisers.py @@ -90,7 +90,6 @@ from documents.templating.utils import convert_format_str_to_template_format from documents.templating.workflows import validate_workflow_template from documents.validators import uri_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 sort_versions_newest_first @@ -2895,7 +2894,7 @@ class ShareLinkSerializer(OwnedObjectSerializer): and has_perms_owner_aware( self.user, "view_document", - get_root_document(document), + document, ) ): return document diff --git a/src/documents/tests/test_permission_filtering_security.py b/src/documents/tests/test_permission_filtering_security.py index 774c67803..ff7df5200 100644 --- a/src/documents/tests/test_permission_filtering_security.py +++ b/src/documents/tests/test_permission_filtering_security.py @@ -18,6 +18,7 @@ from documents.models import Correspondent from documents.models import DocumentType from documents.models import StoragePath from documents.models import Tag +from documents.permissions import has_perms_owner_aware from documents.permissions import permitted_document_ids from documents.permissions import permitted_object_ids from documents.permissions import restrict_queryset_to_visible @@ -470,6 +471,92 @@ class TestPermittedDocumentIdsVersions: ) +@pytest.mark.django_db +class TestHasPermsOwnerAwareVersions: + """ + The single-object check agrees with permitted_document_ids: a version is + authorized by its root document. + """ + + @pytest.mark.parametrize( + ("root_owner", "version_owner", "expected"), + [ + pytest.param( + "other", + "nobody", + False, + id="unowned-version-of-private-root", + ), + pytest.param("other", "user", False, id="own-version-of-private-root"), + pytest.param("user", "other", True, id="foreign-version-of-own-root"), + pytest.param("nobody", "other", True, id="private-version-of-unowned-root"), + ], + ) + def test_version_follows_root_owner( + self, + root_owner: str, + version_owner: str, + *, + expected: bool, + ) -> None: + """ + GIVEN: + - A root document and a version with differing owners + WHEN: + - The single-object check runs for the version + THEN: + - The version is allowed exactly when its root is + """ + user = UserFactory() + owners = {"user": user, "other": UserFactory(), "nobody": None} + root = DocumentFactory(owner=owners[root_owner]) + version = DocumentFactory(root_document=root, owner=owners[version_owner]) + + assert has_perms_owner_aware(user, "view_document", version) is expected + assert has_perms_owner_aware(user, "view_document", root) is expected + + def test_grant_on_root_applies_and_grant_on_version_does_not(self) -> None: + """ + GIVEN: + - A private root with a version, and a second private root with a version + - The user may change only the first root, and was granted the second + root's version directly + WHEN: + - The single-object check runs for each version + THEN: + - Only the first root's version is allowed + """ + user = UserFactory() + shared_root = DocumentFactory(owner=UserFactory()) + shared_version = DocumentFactory(root_document=shared_root, owner=UserFactory()) + private_root = DocumentFactory(owner=UserFactory()) + private_version = DocumentFactory( + root_document=private_root, + owner=UserFactory(), + ) + grant_object(user, shared_root, "change_document") + grant_object(user, private_version, "change_document") + + assert has_perms_owner_aware(user, "change_document", shared_version) + assert not has_perms_owner_aware(user, "change_document", private_version) + + def test_other_models_use_their_own_owner(self) -> None: + """ + GIVEN: + - A tag owned by someone else, and one owned by the user + WHEN: + - The single-object check runs for each + THEN: + - Only the user's own tag is allowed without a grant + """ + user = UserFactory() + mine = TagFactory(owner=user) + theirs = TagFactory(owner=UserFactory()) + + assert has_perms_owner_aware(user, "view_tag", mine) + assert not has_perms_owner_aware(user, "view_tag", theirs) + + @pytest.mark.django_db class TestAiChatAllDocumentsPermissionBoundary: """ diff --git a/src/documents/views.py b/src/documents/views.py index 0fefaa331..74ba694bd 100644 --- a/src/documents/views.py +++ b/src/documents/views.py @@ -1559,7 +1559,7 @@ class DocumentViewSet( if request.user is not None and not has_perms_owner_aware( request.user, "change_document", - get_root_document(doc), + doc, ): return HttpResponseForbidden("Insufficient permissions") @@ -1622,7 +1622,7 @@ class DocumentViewSet( if request.user is not None and not has_perms_owner_aware( request.user, "change_document", - get_root_document(doc), + doc, ): return HttpResponseForbidden("Insufficient permissions") @@ -1875,7 +1875,7 @@ class DocumentViewSet( if currentUser is not None and not has_perms_owner_aware( currentUser, "view_document", - get_root_document(doc), + doc, ): return HttpResponseForbidden("Insufficient permissions to view notes") except Document.DoesNotExist: @@ -1897,7 +1897,7 @@ class DocumentViewSet( if currentUser is not None and not has_perms_owner_aware( currentUser, "change_document", - get_root_document(doc), + doc, ): return HttpResponseForbidden( "Insufficient permissions to create notes", @@ -1940,7 +1940,7 @@ class DocumentViewSet( if currentUser is not None and not has_perms_owner_aware( currentUser, "change_document", - get_root_document(doc), + doc, ): return HttpResponseForbidden("Insufficient permissions to delete notes") @@ -1990,7 +1990,7 @@ class DocumentViewSet( if currentUser is not None and not has_perms_owner_aware( currentUser, "change_document", - get_root_document(doc), + doc, ): return HttpResponseForbidden( "Insufficient permissions to add share link", @@ -2451,7 +2451,7 @@ class ChatStreamingView(GenericAPIView[Any]): if not has_perms_owner_aware( request.user, "view_document", - get_root_document(document), + document, ): return HttpResponseForbidden("Insufficient permissions")