Compare commits

...
Author SHA1 Message Date
stumpylog 7bdc407ed9 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.
2026-10-09 14:43:34 -07:00
Trenton HandClaude Sonnet 5.5 01e1e76f91 Fix: judge versions by their root in the AI similarity permission checks
The AI classifier asked permitted_object_ids for the Document model directly,
which judges a version by its own owner and grants. A version whose owner had
drifted from its private root's could be offered as similar-document context
to a user who cannot see the root. Use permitted_document_ids in both the
vector and full-text paths, which authorizes a version by its root.

Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
2026-10-09 13:43:36 -07:00
Trenton HandClaude Sonnet 5.5 a41e06f196 Fix: authorize document versions by their root and speed up permission id sets
permitted_document_ids judged a version by its own owner and grants, so a
version whose owner had drifted from its root's was visible to the wrong
people and hidden from the right ones. Callers patched this individually by
mapping each document to its root first. The query itself was also slow on
MariaDB: the guardian grants were a UNION cast to integers and tested with
IN inside an OR with the owner checks, which MariaDB cannot materialize, so it
re-scans the user's grants for every document. At 20k documents that took
seconds for a user with a couple of hundred grants.

permitted_object_ids now casts the row key to a string, as guardian stores
object_pk, and tests it against a single uncorrelated UNION ALL of the user's
and groups' grants. No integer cast is needed and the user's groups are
matched with an IN subquery rather than a join through the membership table.
Postgres and SQLite build the grant set once, and MariaDB probes guardian's
unique indexes per row, which is cheap. A correlated EXISTS per grant also
fixed MariaDB but was up to 3x slower than before on Postgres and SQLite.

permitted_object_ids also takes an optional parent_field naming a
self-referencing foreign key whose target authorizes the row, and
permitted_document_ids passes root_document, so a version is visible exactly
when its root is. The helper that mapped documents to their roots at the call
sites is no longer needed, so the email, selection data, share link bundle,
trash, bulk download and bulk edit checks use the id set directly.

Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
2026-10-09 13:43:36 -07:00
6 changed files with 565 additions and 87 deletions

No files matched your search

+70 -37
View File
@@ -6,7 +6,9 @@ from django.contrib.auth.models import Permission
from django.contrib.auth.models import User from django.contrib.auth.models import User
from django.contrib.contenttypes.models import ContentType from django.contrib.contenttypes.models import ContentType
from django.db.models import Case from django.db.models import Case
from django.db.models import CharField
from django.db.models import Count from django.db.models import Count
from django.db.models import F
from django.db.models import IntegerField from django.db.models import IntegerField
from django.db.models import Model from django.db.models import Model
from django.db.models import Q from django.db.models import Q
@@ -26,6 +28,7 @@ from rest_framework.permissions import BasePermission
from rest_framework.permissions import DjangoObjectPermissions from rest_framework.permissions import DjangoObjectPermissions
from documents.models import Document from documents.models import Document
from documents.versioning import get_root_document
class PaperlessObjectPermissions(DjangoObjectPermissions): class PaperlessObjectPermissions(DjangoObjectPermissions):
@@ -349,6 +352,7 @@ def permitted_object_ids(
perm: str, perm: str,
*, *,
include_deleted: bool = False, include_deleted: bool = False,
parent_field: str | None = None,
) -> QuerySet[int]: ) -> QuerySet[int]:
""" """
Generic version of ``permitted_document_ids`` for any model with an Generic version of ``permitted_document_ids`` for any model with an
@@ -357,6 +361,24 @@ def permitted_object_ids(
soft-delete pattern (currently only ``Document``); for every other model soft-delete pattern (currently only ``Document``); for every other model
it is accepted but has no effect, since those models have no soft-delete it is accepted but has no effect, since those models have no soft-delete
concept. concept.
``parent_field`` names a self-referencing foreign key whose target
authorizes the row (``Document.root_document``). A row with a parent is
visible exactly when its parent is, judged by the parent's owner and
grants, so the row's own owner and grants are ignored.
Guardian stores ``object_pk`` as a string, so the row key is cast to a
string and tested against the user's and groups' grants with a single
uncorrelated ``IN``. Postgres and SQLite build that set once. MariaDB
evaluates it as an index probe per row, which is cheap because the
lookups use guardian's unique indexes. Casting every ``object_pk`` to an
integer instead cannot use an index, and MariaDB cannot materialize it
inside the owner ``OR``, so it re-scans the user's grants for every row.
A correlated ``EXISTS`` per grant fixes MariaDB too, but Postgres and
SQLite re-run it for every row and end up slower than the original. The
user's groups are matched with an ``IN`` subquery rather than a join
through the membership table, which SQLite plans badly once the grant
tables grow.
""" """
has_soft_delete = hasattr(model, "global_objects") has_soft_delete = hasattr(model, "global_objects")
manager = ( manager = (
@@ -364,8 +386,21 @@ def permitted_object_ids(
) )
base_qs = manager.all().only("id", "owner") base_qs = manager.all().only("id", "owner")
owner_field, key_field = "owner", "pk"
if parent_field is not None:
owner_field, key_field = "authorizing_owner", "authorizing_id"
base_qs = base_qs.annotate(
authorizing_id=Coalesce(f"{parent_field}_id", "id"),
authorizing_owner=Case(
When(**{f"{parent_field}_id__isnull": True}, then=F("owner_id")),
default=F(f"{parent_field}__owner_id"),
output_field=IntegerField(),
),
)
unowned = Q(**{f"{owner_field}__isnull": True})
if user is None or not getattr(user, "is_authenticated", False): if user is None or not getattr(user, "is_authenticated", False):
return base_qs.filter(owner__isnull=True).values_list("id", flat=True) return base_qs.filter(unowned).values_list("id", flat=True)
# Deactivated users get nothing, deactivated superusers included, so this # Deactivated users get nothing, deactivated superusers included, so this
# has to come before the superuser shortcut. guardian's # has to come before the superuser shortcut. guardian's
@@ -389,21 +424,26 @@ def permitted_object_ids(
"permission__content_type": content_type, "permission__content_type": content_type,
} }
user_perm_ids = ( # Both grant sets are compared to the row key as strings, exactly as
UserObjectPermission.objects.filter(user=user, **perm_filter) # guardian stores them, and are uncorrelated, so each engine can build the
.annotate(object_pk_int=Cast("object_pk", IntegerField())) # set once instead of probing per row.
.values_list("object_pk_int", flat=True) user_keys = UserObjectPermission.objects.filter(
) user=user,
group_perm_ids = ( **perm_filter,
GroupObjectPermission.objects.filter(group__user=user, **perm_filter) ).values_list("object_pk", flat=True)
.annotate(object_pk_int=Cast("object_pk", IntegerField())) group_keys = GroupObjectPermission.objects.filter(
.values_list("object_pk_int", flat=True) group_id__in=user.groups.values("id"),
) **perm_filter,
permitted_ids = user_perm_ids.union(group_perm_ids) ).values_list("object_pk", flat=True)
permitted_keys = user_keys.union(group_keys, all=True)
return base_qs.filter( return (
Q(owner=user) | Q(owner__isnull=True) | Q(id__in=permitted_ids), base_qs.annotate(permitted_key=Cast(key_field, CharField(max_length=64)))
).values_list("id", flat=True) .filter(
Q(**{owner_field: user.pk}) | unowned | Q(permitted_key__in=permitted_keys),
)
.values_list("id", flat=True)
)
ModelT = TypeVar("ModelT", bound=Model) ModelT = TypeVar("ModelT", bound=Model)
@@ -471,30 +511,16 @@ def permitted_document_ids(
``include_deleted=True`` for callers that need to check permission on ``include_deleted=True`` for callers that need to check permission on
soft-deleted documents (e.g. trash restore). This intentionally avoids soft-deleted documents (e.g. trash restore). This intentionally avoids
``get_objects_for_user`` to keep the subquery small and index-friendly. ``get_objects_for_user`` to keep the subquery small and index-friendly.
"""
return permitted_object_ids(user, Document, perm, include_deleted=include_deleted)
A version is authorized by its root document, so a version's own owner and
def documents_without_permitted_root( grants never matter.
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 return permitted_object_ids(
root document, so a version's own owner is ignored. A single query, without user,
loading the documents or joining the root. Document,
""" perm,
return documents.annotate( include_deleted=include_deleted,
root_id=Coalesce("root_document_id", "id"), parent_field="root_document",
).exclude(
root_id__in=permitted_document_ids(
user,
perm=perm,
include_deleted=include_deleted,
),
) )
@@ -654,7 +680,14 @@ def has_perms_owner_aware(user, perms, obj):
single-object check still has many production callers. Several callers single-object check still has many production callers. Several callers
remain across ``documents/``, ``paperless_mail/``, and ``paperless_ai/`` remain across ``documents/``, ``paperless_mail/``, and ``paperless_ai/``
-- grep for this function name before removing it. -- 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) checker = ObjectPermissionChecker(user)
return obj.owner is None or obj.owner == user or checker.has_perm(perms, obj) return obj.owner is None or obj.owner == user or checker.has_perm(perms, obj)
+1 -2
View File
@@ -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.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
@@ -2895,7 +2894,7 @@ class ShareLinkSerializer(OwnedObjectSerializer):
and has_perms_owner_aware( and has_perms_owner_aware(
self.user, self.user,
"view_document", "view_document",
get_root_document(document), document,
) )
): ):
return document return document
@@ -18,6 +18,7 @@ from documents.models import Correspondent
from documents.models import DocumentType from documents.models import DocumentType
from documents.models import StoragePath from documents.models import StoragePath
from documents.models import Tag 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_document_ids
from documents.permissions import permitted_object_ids from documents.permissions import permitted_object_ids
from documents.permissions import restrict_queryset_to_visible from documents.permissions import restrict_queryset_to_visible
@@ -32,6 +33,8 @@ from paperless_testing.permissions import grant_global
from paperless_testing.permissions import grant_object from paperless_testing.permissions import grant_object
if TYPE_CHECKING: if TYPE_CHECKING:
from django.contrib.auth.models import User
from paperless_testing.dirs import PaperlessDirs from paperless_testing.dirs import PaperlessDirs
@@ -178,6 +181,382 @@ class TestPermittedDocumentIdsIncludeDeleted:
) )
@pytest.mark.django_db
class TestPermittedDocumentIdsVersions:
"""
A version is authorized by its root document: the version's own owner and
grants never matter.
"""
@pytest.mark.parametrize(
("root_owner", "version_owner", "expected_visible"),
[
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("user", "nobody", True, id="unowned-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_visible: bool,
) -> None:
"""
GIVEN:
- A root document and a version with differing owners
WHEN:
- The permitted document ids are resolved for the user
THEN:
- The version is visible 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])
visible = set(permitted_document_ids(user))
assert (version.pk in visible) is expected_visible
assert (root.pk in visible) is expected_visible
@staticmethod
def grantee(user: User, kind: str) -> User | Group:
"""The user itself, or a new group the user belongs to."""
if kind == "user":
return user
group = Group.objects.create(name="shared")
user.groups.add(group)
return group
@pytest.mark.parametrize(
"grantee_kind",
[pytest.param("user", id="user"), pytest.param("group", id="group")],
)
def test_grant_on_root_applies_to_version(self, grantee_kind: str) -> None:
"""
GIVEN:
- A private root document shared with a user or one of their groups
- A version of it owned by someone else
WHEN:
- The permitted document ids are resolved for the user
THEN:
- Both the root and the version are visible
- A user without the grant sees neither
"""
user = UserFactory()
stranger = UserFactory()
root = DocumentFactory(owner=UserFactory())
version = DocumentFactory(root_document=root, owner=UserFactory())
grant_object(self.grantee(user, grantee_kind), root, "view_document")
assert_visible_document_ids(
permitted_document_ids(user),
expected_visible=[root.pk, version.pk],
expected_hidden=[],
)
assert_visible_document_ids(
permitted_document_ids(stranger),
expected_visible=[],
expected_hidden=[root.pk, version.pk],
)
@pytest.mark.parametrize(
"grantee_kind",
[pytest.param("user", id="user"), pytest.param("group", id="group")],
)
def test_grant_on_version_is_ignored(self, grantee_kind: str) -> None:
"""
GIVEN:
- A private root document
- A version with an explicit grant for the user or one of their groups
WHEN:
- The permitted document ids are resolved for the user
THEN:
- Neither the root nor the version is visible
"""
user = UserFactory()
root = DocumentFactory(owner=UserFactory())
version = DocumentFactory(root_document=root, owner=UserFactory())
grant_object(self.grantee(user, grantee_kind), version, "view_document")
assert_visible_document_ids(
permitted_document_ids(user),
expected_visible=[],
expected_hidden=[root.pk, version.pk],
)
def test_grant_on_one_root_does_not_reach_another_roots_version(self) -> None:
"""
GIVEN:
- Two private roots, each with a version
- The user may view only the first root
WHEN:
- The permitted document ids are resolved for the user
THEN:
- Only the first root and its version are visible
"""
user = UserFactory()
first = DocumentFactory(owner=UserFactory())
first_version = DocumentFactory(root_document=first, owner=UserFactory())
second = DocumentFactory(owner=UserFactory())
second_version = DocumentFactory(root_document=second, owner=user)
grant_object(user, first, "view_document")
assert_visible_document_ids(
permitted_document_ids(user),
expected_visible=[first.pk, first_version.pk],
expected_hidden=[second.pk, second_version.pk],
)
def test_user_in_several_groups(self) -> None:
"""
GIVEN:
- A user in two groups
- Two private roots shared with one group each, and a third shared with nobody
- A version of each root
WHEN:
- The permitted document ids are resolved for the user
THEN:
- The two shared roots and their versions are visible
- The third root and its version are not
"""
user = UserFactory()
groups = [Group.objects.create(name=f"group{i}") for i in range(2)]
user.groups.add(*groups)
shared = [DocumentFactory(owner=UserFactory()) for _ in groups]
for root, group in zip(shared, groups, strict=True):
grant_object(group, root, "view_document")
unshared = DocumentFactory(owner=UserFactory())
shared_versions = [
DocumentFactory(root_document=root, owner=UserFactory()) for root in shared
]
unshared_version = DocumentFactory(root_document=unshared, owner=None)
assert_visible_document_ids(
permitted_document_ids(user),
expected_visible=[
*(root.pk for root in shared),
*(version.pk for version in shared_versions),
],
expected_hidden=[unshared.pk, unshared_version.pk],
)
def test_permission_is_resolved_through_the_root(self) -> None:
"""
GIVEN:
- A private root document where the user may view and change
WHEN:
- The permitted ids are resolved for view, change and delete
THEN:
- The version is visible for view and change only
"""
user = UserFactory()
root = DocumentFactory(owner=UserFactory())
version = DocumentFactory(root_document=root, owner=UserFactory())
grant_object(user, root, "view_document", "change_document")
assert version.pk in set(permitted_document_ids(user))
assert version.pk in set(permitted_document_ids(user, perm="change_document"))
assert version.pk in set(
permitted_document_ids(user, perm="documents.change_document"),
)
assert version.pk not in set(
permitted_document_ids(user, perm="delete_document"),
)
def test_anonymous_sees_versions_of_unowned_roots_only(self) -> None:
"""
GIVEN:
- A version owned by nobody under a private root
- A version owned by someone under an unowned root
WHEN:
- The permitted document ids are resolved for an anonymous user
THEN:
- Only the version of the unowned root is visible
"""
private_root = DocumentFactory(owner=UserFactory())
private_version = DocumentFactory(root_document=private_root, owner=None)
open_root = DocumentFactory(owner=None)
open_version = DocumentFactory(root_document=open_root, owner=UserFactory())
assert_visible_document_ids(
permitted_document_ids(AnonymousUser()),
expected_visible=[open_root.pk, open_version.pk],
expected_hidden=[private_root.pk, private_version.pk],
)
def test_deleted_versions_follow_their_deleted_root(self) -> None:
"""
GIVEN:
- A soft-deleted root document and its version, which deleting the
root soft-deletes too; the version is owned by someone else
WHEN:
- The permitted document ids are resolved with and without deleted
documents
THEN:
- Nothing is visible by default
- With deleted documents included, the version is visible to the
root's owner and not to the version's own owner
"""
owner = UserFactory()
version_owner = UserFactory()
root = DocumentFactory(owner=owner)
version = DocumentFactory(root_document=root, owner=version_owner)
root.delete()
assert not {root.pk, version.pk} & set(permitted_document_ids(owner))
assert_visible_document_ids(
permitted_document_ids(owner, include_deleted=True),
expected_visible=[root.pk, version.pk],
expected_hidden=[],
)
assert_visible_document_ids(
permitted_document_ids(version_owner, include_deleted=True),
expected_visible=[],
expected_hidden=[root.pk, version.pk],
)
@pytest.mark.parametrize(
"is_superuser",
[
pytest.param(False, id="regular-user"),
pytest.param(True, id="superuser"),
],
)
def test_inactive_user_sees_no_versions(self, *, is_superuser: bool) -> None:
"""
GIVEN:
- An inactive user, possibly a superuser, who owns a root and its version
WHEN:
- The permitted document ids are resolved for them
THEN:
- Nothing is visible
"""
user = UserFactory(is_active=False, is_superuser=is_superuser)
root = DocumentFactory(owner=user)
version = DocumentFactory(root_document=root, owner=user)
assert_visible_document_ids(
permitted_document_ids(user),
expected_visible=[],
expected_hidden=[root.pk, version.pk],
)
def test_superuser_sees_all_versions(self) -> None:
"""
GIVEN:
- A private root owned by someone else, with a version
WHEN:
- The permitted document ids are resolved for a superuser
THEN:
- Both the root and the version are visible
"""
superuser = UserFactory(superuser=True)
root = DocumentFactory(owner=UserFactory())
version = DocumentFactory(root_document=root, owner=UserFactory())
assert_visible_document_ids(
permitted_document_ids(superuser),
expected_visible=[root.pk, version.pk],
expected_hidden=[],
)
@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 @pytest.mark.django_db
class TestAiChatAllDocumentsPermissionBoundary: class TestAiChatAllDocumentsPermissionBoundary:
""" """
+24 -33
View File
@@ -174,7 +174,6 @@ 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
@@ -1560,7 +1559,7 @@ class DocumentViewSet(
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",
get_root_document(doc), doc,
): ):
return HttpResponseForbidden("Insufficient permissions") return HttpResponseForbidden("Insufficient permissions")
@@ -1623,7 +1622,7 @@ class DocumentViewSet(
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",
get_root_document(doc), doc,
): ):
return HttpResponseForbidden("Insufficient permissions") return HttpResponseForbidden("Insufficient permissions")
@@ -1876,7 +1875,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,
"view_document", "view_document",
get_root_document(doc), doc,
): ):
return HttpResponseForbidden("Insufficient permissions to view notes") return HttpResponseForbidden("Insufficient permissions to view notes")
except Document.DoesNotExist: except Document.DoesNotExist:
@@ -1898,7 +1897,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",
get_root_document(doc), doc,
): ):
return HttpResponseForbidden( return HttpResponseForbidden(
"Insufficient permissions to create notes", "Insufficient permissions to create notes",
@@ -1941,7 +1940,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",
get_root_document(doc), doc,
): ):
return HttpResponseForbidden("Insufficient permissions to delete notes") return HttpResponseForbidden("Insufficient permissions to delete notes")
@@ -1991,7 +1990,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",
get_root_document(doc), doc,
): ):
return HttpResponseForbidden( return HttpResponseForbidden(
"Insufficient permissions to add share link", "Insufficient permissions to add share link",
@@ -2117,7 +2116,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_without_permitted_root(documents, request.user).exists() and documents.exclude(id__in=permitted_document_ids(request.user)).exists()
): ):
return HttpResponseForbidden("Insufficient permissions") return HttpResponseForbidden("Insufficient permissions")
@@ -2452,7 +2451,7 @@ class ChatStreamingView(GenericAPIView[Any]):
if not has_perms_owner_aware( if not has_perms_owner_aware(
request.user, request.user,
"view_document", "view_document",
get_root_document(document), document,
): ):
return HttpResponseForbidden("Insufficient permissions") return HttpResponseForbidden("Insufficient permissions")
@@ -3009,12 +3008,8 @@ class DocumentOperationPermissionMixin(PassUserMixin, DocumentSelectionMixin):
user.has_perm( user.has_perm(
"documents.change_document", "documents.change_document",
) )
and not Document.global_objects.filter( and not Document.global_objects.filter(pk__in=documents)
pk__in=[doc.pk for doc in root_docs], .exclude(pk__in=permitted_document_ids(user, perm="change_document"))
)
.exclude(
pk__in=permitted_document_ids(user, perm="change_document"),
)
.exists() .exists()
) )
@@ -3627,7 +3622,7 @@ class SelectionDataView(DocumentSelectionMixin, GenericAPIView[Any]):
documents = Document.objects.filter(pk__in=ids) documents = Document.objects.filter(pk__in=ids)
if ( if (
documents.count() != len(ids) documents.count() != len(ids)
or documents_without_permitted_root(documents, request.user).exists() or documents.exclude(id__in=permitted_document_ids(request.user)).exists()
): ):
return HttpResponseForbidden("Insufficient permissions") return HttpResponseForbidden("Insufficient permissions")
@@ -4124,21 +4119,16 @@ class BulkDownloadView(DocumentSelectionMixin, GenericAPIView[Any]):
validated_data=serializer.validated_data, validated_data=serializer.validated_data,
) )
documents = Document.objects.filter(pk__in=ids) documents = Document.objects.filter(pk__in=ids)
versioned_documents = []
compression = serializer.validated_data.get("compression") compression = serializer.validated_data.get("compression")
content = serializer.validated_data.get("content") content = serializer.validated_data.get("content")
follow_filename_format = serializer.validated_data.get("follow_formatting") follow_filename_format = serializer.validated_data.get("follow_formatting")
permitted_ids = set(permitted_document_ids(request.user)) if documents.exclude(id__in=permitted_document_ids(request.user)).exists():
for document in documents: return HttpResponseForbidden("Insufficient permissions")
root_doc = get_root_document(document) versioned_documents = [
if root_doc.pk not in permitted_ids: get_latest_version_for_root(get_root_document(document))
return HttpResponseForbidden("Insufficient permissions") for document in documents
versioned_documents.append( ]
get_latest_version_for_root(
root_doc,
),
)
if content == "both": if content == "both":
strategy_class = OriginalAndArchiveStrategy strategy_class = OriginalAndArchiveStrategy
@@ -4811,7 +4801,7 @@ class ShareLinkBundleViewSet(PassUserMixin, ModelViewSet[ShareLinkBundle]):
) )
denied_id = ( denied_id = (
documents_without_permitted_root(documents_qs, request.user) documents_qs.exclude(id__in=permitted_document_ids(request.user))
.order_by("pk") .order_by("pk")
.values_list("pk", flat=True) .values_list("pk", flat=True)
.first() .first()
@@ -5657,11 +5647,12 @@ 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 documents_without_permitted_root( if docs.exclude(
docs, id__in=permitted_document_ids(
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")
+8 -10
View File
@@ -4,8 +4,7 @@ from django.conf import settings
from django.contrib.auth.models import User from django.contrib.auth.models import User
from documents.models import Document from documents.models import Document
from documents.permissions import permitted_object_ids from documents.permissions import permitted_document_ids
from documents.permissions import restrict_queryset_to_visible
from documents.permissions import user_is_unrestricted from documents.permissions import user_is_unrestricted
from paperless.config import AIConfig from paperless.config import AIConfig
from paperless_ai.base_model import ClassificationSuggestions from paperless_ai.base_model import ClassificationSuggestions
@@ -54,8 +53,8 @@ def _fulltext_similar_documents(
active superuser - see user_is_unrestricted) is normalized to ``None`` active superuser - see user_is_unrestricted) is normalized to ``None``
before calling, since the backend's permission filter has no superuser before calling, since the backend's permission filter has no superuser
short-circuit of its own. Results are re-checked with short-circuit of its own. Results are re-checked with
restrict_queryset_to_visible() since Tantivy's indexed permission fields permitted_document_ids() since Tantivy's indexed permission fields lag the
lag the DB via async reindexing. DB via async reindexing and judge a version by its own owner.
""" """
from documents.search import get_backend from documents.search import get_backend
@@ -69,10 +68,9 @@ def _fulltext_similar_documents(
) )
if not unrestricted: if not unrestricted:
allowed_ids = set( allowed_ids = set(
restrict_queryset_to_visible( Document.objects.filter(
Document.objects.filter(pk__in=similar_ids), pk__in=similar_ids,
user, id__in=permitted_document_ids(user),
"view_document",
).values_list("pk", flat=True), ).values_list("pk", flat=True),
) )
similar_ids = [doc_id for doc_id in similar_ids if doc_id in allowed_ids] similar_ids = [doc_id for doc_id in similar_ids if doc_id in allowed_ids]
@@ -200,13 +198,13 @@ def get_taxonomy_context(
# quadratic scan in the vector store at best, and past ~32,763 # quadratic scan in the vector store at best, and past ~32,763
# documents a hard sqlite3.OperationalError (SQLite's # documents a hard sqlite3.OperationalError (SQLite's
# bound-parameter limit) at worst. # bound-parameter limit) at worst.
# permitted_object_ids() has its own superuser shortcut that would # permitted_document_ids() has its own superuser shortcut that would
# return every Document's id anyway, so this changes nothing about # return every Document's id anyway, so this changes nothing about
# which documents are considered -- only how we get there. # which documents are considered -- only how we get there.
visible_document_ids = ( visible_document_ids = (
None None
if user_is_unrestricted(user) if user_is_unrestricted(user)
else list(permitted_object_ids(user, Document, "view_document")) else list(permitted_document_ids(user))
) )
nodes = retrieve_similar_nodes( nodes = retrieve_similar_nodes(
document, document,
+83 -5
View File
@@ -552,7 +552,7 @@ class TestGetTaxonomyContextVisibility:
return_value=[], return_value=[],
) )
mock_permitted = mocker.patch( mock_permitted = mocker.patch(
"paperless_ai.ai_classifier.permitted_object_ids", "paperless_ai.ai_classifier.permitted_document_ids",
) )
user = UserFactory.create(is_superuser=True) user = UserFactory.create(is_superuser=True)
@@ -582,7 +582,7 @@ class TestGetTaxonomyContextVisibility:
return_value=[], return_value=[],
) )
mock_permitted = mocker.patch( mock_permitted = mocker.patch(
"paperless_ai.ai_classifier.permitted_object_ids", "paperless_ai.ai_classifier.permitted_document_ids",
) )
get_taxonomy_context(document, None) get_taxonomy_context(document, None)
@@ -611,16 +611,55 @@ class TestGetTaxonomyContextVisibility:
return_value=[], return_value=[],
) )
mock_permitted = mocker.patch( mock_permitted = mocker.patch(
"paperless_ai.ai_classifier.permitted_object_ids", "paperless_ai.ai_classifier.permitted_document_ids",
return_value=[1, 2, 3], return_value=[1, 2, 3],
) )
user = UserFactory.create(is_superuser=False) user = UserFactory.create(is_superuser=False)
get_taxonomy_context(document, user) get_taxonomy_context(document, user)
mock_permitted.assert_called_once_with(user, Document, "view_document") mock_permitted.assert_called_once_with(user)
assert mock_retrieve.call_args.kwargs["document_ids"] == [1, 2, 3] assert mock_retrieve.call_args.kwargs["document_ids"] == [1, 2, 3]
@pytest.mark.django_db
@override_settings(LLM_EMBEDDING_BACKEND="huggingface")
def test_version_of_private_root_is_not_visible(
self,
mocker: pytest_mock.MockerFixture,
) -> None:
"""
GIVEN:
- A private root document owned by someone else
- A version of it whose own owner is unset, as when the root
changed hands after the version was created
WHEN:
- get_taxonomy_context() is called for a non-superuser
THEN:
- Neither the root nor the version is in the visible ids passed to
retrieve_similar_nodes(), since a version follows its root
"""
owner = UserFactory.create()
viewer = UserFactory.create(is_superuser=False)
root = DocumentFactory.create(content="private", owner=owner)
version = DocumentFactory.create(
content="private",
owner=None,
root_document=root,
version_index=1,
)
source = DocumentFactory.create(content="Some content", owner=viewer)
mock_retrieve = mocker.patch(
"paperless_ai.ai_classifier.retrieve_similar_nodes",
return_value=[],
)
get_taxonomy_context(source, viewer)
visible = mock_retrieve.call_args.kwargs["document_ids"]
assert source.pk in visible
assert root.pk not in visible
assert version.pk not in visible
@pytest.mark.django_db @pytest.mark.django_db
class TestFulltextSimilarDocuments: class TestFulltextSimilarDocuments:
@@ -803,7 +842,7 @@ class TestFulltextSimilarDocuments:
- _fulltext_similar_documents() is called with that user - _fulltext_similar_documents() is called with that user
THEN: THEN:
- Only the still-permitted document is returned - the DB - Only the still-permitted document is returned - the DB
re-check via restrict_queryset_to_visible() must catch the re-check via permitted_document_ids() must catch the
document Tantivy's stale index still thinks is visible document Tantivy's stale index still thinks is visible
""" """
owner = UserFactory.create() owner = UserFactory.create()
@@ -834,6 +873,45 @@ class TestFulltextSimilarDocuments:
assert [s["document_id"] for s in result] == [permitted.pk] assert [s["document_id"] for s in result] == [permitted.pk]
def test_excludes_version_of_private_root_for_regular_user(
self,
fulltext_backend: TantivyBackend,
) -> None:
"""
GIVEN:
- A regular user and a private root owned by someone else
- A version of that root with no owner of its own, which the
Tantivy index therefore treats as visible to everyone
WHEN:
- _fulltext_similar_documents() is called with that user
THEN:
- The version is not returned, since the DB re-check judges it by
its root
"""
owner = UserFactory.create()
viewer = UserFactory.create(is_superuser=False)
source = DocumentFactory.create(
content="shared content phrase",
owner=viewer,
)
root = DocumentFactory.create(
content="shared content phrase",
owner=owner,
)
version = DocumentFactory.create(
content="shared content phrase",
owner=None,
root_document=root,
version_index=1,
)
fulltext_backend.add_or_update(source)
fulltext_backend.add_or_update(root)
fulltext_backend.add_or_update(version)
result = _fulltext_similar_documents(source, user=viewer, top_k=5)
assert version.pk not in [s["document_id"] for s in result]
@pytest.mark.django_db @pytest.mark.django_db
@override_settings(LLM_EMBEDDING_BACKEND="huggingface") @override_settings(LLM_EMBEDDING_BACKEND="huggingface")