mirror of
https://github.com/paperless-ngx/paperless-ngx.git
synced 2026-10-09 17:47:12 +00:00
Fix: authorize document versions by their root in the single-object permission check
has_perms_owner_aware judged a document by its own owner and grants, so each endpoint that fetches a document itself had to remember to map a version to its root document first, and one that forgot, like the more-like-this search filter, authorized by a stale version owner. The check now maps a Document to its root before looking at the owner and the guardian grants, matching what permitted_document_ids does for id sets. The eight call sites that mapped the document themselves pass it straight through. The DRF object permission class needs no change because the document viewset only ever serves root documents.
This commit is contained in:
1 parent
ece8769f59
commit
8a96190359
4 files changed
+103
-9
No files matched your search
@@ -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)
|
||||
|
||||
|
||||
@@ -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
|
||||
|
||||
@@ -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:
|
||||
"""
|
||||
|
||||
@@ -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")
|
||||
|
||||
|
||||
Reference in new issue
Block a user