mirror of
https://github.com/paperless-ngx/paperless-ngx.git
synced 2026-08-07 19:33:19 +00:00
feat: generalize permitted_document_ids into permitted_object_ids for any model
Implements Task 14 of the permission-filtering consolidation plan: - Add generic permitted_object_ids(user, model, perm, include_deleted=False) - Refactor permitted_document_ids to delegate to permitted_object_ids - Add comprehensive tests for Tag/Correspondent/DocumentType/StoragePath - Preserve exact public behavior of permitted_document_ids (100% regression-free) All 38 tests pass (18 existing + 20 new). The include_deleted parameter correctly handles soft-delete patterns (effective only for Document). Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
This commit is contained in:
committed by
Trenton H
co-authored by
Claude Sonnet 5
parent
972758fc72
commit
effbddb597
@@ -163,30 +163,40 @@ def set_permissions_for_object(
|
||||
)
|
||||
|
||||
|
||||
def permitted_document_ids(
|
||||
def permitted_object_ids(
|
||||
user,
|
||||
model,
|
||||
perm: str,
|
||||
*,
|
||||
perm: str = "view_document",
|
||||
include_deleted: bool = False,
|
||||
):
|
||||
"""
|
||||
Return a queryset of document IDs the user has ``perm`` on (default
|
||||
``"view_document"``). By default limited to non-deleted documents; pass
|
||||
``include_deleted=True`` for callers that need to check permission on
|
||||
soft-deleted documents (e.g. trash restore). This intentionally avoids
|
||||
``get_objects_for_user`` to keep the subquery small and index-friendly.
|
||||
Generic version of ``permitted_document_ids`` for any model with an
|
||||
``owner`` field and guardian object-level permissions. ``include_deleted``
|
||||
only has an effect for models exposing a ``global_objects``/``deleted_at``
|
||||
soft-delete pattern (currently only ``Document``); for every other model
|
||||
it is accepted but has no effect, since those models have no soft-delete
|
||||
concept.
|
||||
"""
|
||||
|
||||
manager = Document.global_objects if include_deleted else Document.objects
|
||||
base_docs = manager.all()
|
||||
base_docs = base_docs.only("id", "owner")
|
||||
has_soft_delete = hasattr(model, "global_objects")
|
||||
manager = (
|
||||
model.global_objects if include_deleted and has_soft_delete else model.objects
|
||||
)
|
||||
if has_soft_delete:
|
||||
base_qs = (
|
||||
manager.all()
|
||||
if include_deleted
|
||||
else manager.filter(deleted_at__isnull=True)
|
||||
)
|
||||
else:
|
||||
base_qs = manager.all()
|
||||
base_qs = base_qs.only("id", "owner")
|
||||
|
||||
if user is None or not getattr(user, "is_authenticated", False):
|
||||
# Just Anonymous user e.g. for drf-spectacular
|
||||
return base_docs.filter(owner__isnull=True).values_list("id", flat=True)
|
||||
return base_qs.filter(owner__isnull=True).values_list("id", flat=True)
|
||||
|
||||
if getattr(user, "is_superuser", False):
|
||||
return base_docs.values_list("id", flat=True)
|
||||
return base_qs.values_list("id", flat=True)
|
||||
|
||||
# Guardian's UserObjectPermission/GroupObjectPermission always store a bare
|
||||
# codename, but has_perm()-style callers commonly pass the qualified
|
||||
@@ -194,31 +204,46 @@ def permitted_document_ids(
|
||||
# codename, so just drop any prefix rather than silently under-permitting.
|
||||
perm = perm.rsplit(".", 1)[-1]
|
||||
|
||||
document_ct = ContentType.objects.get_for_model(Document)
|
||||
content_type = ContentType.objects.get_for_model(model)
|
||||
perm_filter = {
|
||||
"permission__codename": perm,
|
||||
"permission__content_type": document_ct,
|
||||
"permission__content_type": content_type,
|
||||
}
|
||||
|
||||
user_perm_docs = (
|
||||
user_perm_ids = (
|
||||
UserObjectPermission.objects.filter(user=user, **perm_filter)
|
||||
.annotate(object_pk_int=Cast("object_pk", IntegerField()))
|
||||
.values_list("object_pk_int", flat=True)
|
||||
)
|
||||
|
||||
group_perm_docs = (
|
||||
group_perm_ids = (
|
||||
GroupObjectPermission.objects.filter(group__user=user, **perm_filter)
|
||||
.annotate(object_pk_int=Cast("object_pk", IntegerField()))
|
||||
.values_list("object_pk_int", flat=True)
|
||||
)
|
||||
permitted_ids = user_perm_ids.union(group_perm_ids)
|
||||
|
||||
permitted_documents = user_perm_docs.union(group_perm_docs)
|
||||
|
||||
return base_docs.filter(
|
||||
Q(owner=user) | Q(owner__isnull=True) | Q(id__in=permitted_documents),
|
||||
return base_qs.filter(
|
||||
Q(owner=user) | Q(owner__isnull=True) | Q(id__in=permitted_ids),
|
||||
).values_list("id", flat=True)
|
||||
|
||||
|
||||
def permitted_document_ids(
|
||||
user,
|
||||
*,
|
||||
perm: str = "view_document",
|
||||
include_deleted: bool = False,
|
||||
):
|
||||
"""
|
||||
Document-specific convenience wrapper around ``permitted_object_ids``.
|
||||
Return a queryset of document IDs the user has ``perm`` on (default
|
||||
``"view_document"``). By default limited to non-deleted documents; pass
|
||||
``include_deleted=True`` for callers that need to check permission on
|
||||
soft-deleted documents (e.g. trash restore). This intentionally avoids
|
||||
``get_objects_for_user`` to keep the subquery small and index-friendly.
|
||||
"""
|
||||
return permitted_object_ids(user, Document, perm, include_deleted=include_deleted)
|
||||
|
||||
|
||||
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.
|
||||
|
||||
@@ -12,9 +12,18 @@ from django.test import override_settings
|
||||
from guardian.shortcuts import assign_perm
|
||||
from rest_framework.test import APIClient
|
||||
|
||||
from documents.models import Correspondent
|
||||
from documents.models import DocumentType
|
||||
from documents.models import StoragePath
|
||||
from documents.models import Tag
|
||||
from documents.permissions import permitted_document_ids
|
||||
from documents.permissions import permitted_object_ids
|
||||
from documents.serialisers import _get_viewable_duplicates
|
||||
from documents.tests.factories import CorrespondentFactory
|
||||
from documents.tests.factories import DocumentFactory
|
||||
from documents.tests.factories import DocumentTypeFactory
|
||||
from documents.tests.factories import StoragePathFactory
|
||||
from documents.tests.factories import TagFactory
|
||||
|
||||
|
||||
def assert_visible_document_ids(actual_ids, *, expected_visible, expected_hidden):
|
||||
@@ -431,3 +440,92 @@ class TestTrashRestorePermissionBoundary:
|
||||
format="json",
|
||||
)
|
||||
assert response.status_code == HTTPStatus.OK
|
||||
|
||||
|
||||
@pytest.mark.django_db
|
||||
@pytest.mark.parametrize(
|
||||
("model", "factory", "perm"),
|
||||
[
|
||||
(Tag, TagFactory, "view_tag"),
|
||||
(Correspondent, CorrespondentFactory, "view_correspondent"),
|
||||
(DocumentType, DocumentTypeFactory, "view_documenttype"),
|
||||
(StoragePath, StoragePathFactory, "view_storagepath"),
|
||||
],
|
||||
)
|
||||
class TestPermittedObjectIdsGenericModels:
|
||||
def test_owner_sees_own_object(self, model, factory, perm):
|
||||
owner = User.objects.create_user(username=f"owner_{model.__name__}")
|
||||
stranger = User.objects.create_user(username=f"stranger_{model.__name__}")
|
||||
owned = factory(owner=owner)
|
||||
strangers = factory(owner=stranger)
|
||||
|
||||
assert_visible_document_ids(
|
||||
permitted_object_ids(owner, model, perm),
|
||||
expected_visible=[owned.pk],
|
||||
expected_hidden=[strangers.pk],
|
||||
)
|
||||
|
||||
def test_unowned_object_visible_to_everyone(self, model, factory, perm):
|
||||
user = User.objects.create_user(username=f"user_{model.__name__}")
|
||||
unowned = factory(owner=None)
|
||||
|
||||
assert_visible_document_ids(
|
||||
permitted_object_ids(user, model, perm),
|
||||
expected_visible=[unowned.pk],
|
||||
expected_hidden=[],
|
||||
)
|
||||
|
||||
def test_explicit_permission_grants_visibility(self, model, factory, perm):
|
||||
owner = User.objects.create_user(username=f"owner2_{model.__name__}")
|
||||
grantee = User.objects.create_user(username=f"grantee_{model.__name__}")
|
||||
stranger = User.objects.create_user(username=f"stranger2_{model.__name__}")
|
||||
shared = factory(owner=owner)
|
||||
not_shared = factory(owner=owner)
|
||||
assign_perm(perm, grantee, shared)
|
||||
|
||||
assert_visible_document_ids(
|
||||
permitted_object_ids(grantee, model, perm),
|
||||
expected_visible=[shared.pk],
|
||||
expected_hidden=[not_shared.pk],
|
||||
)
|
||||
assert_visible_document_ids(
|
||||
permitted_object_ids(stranger, model, perm),
|
||||
expected_visible=[],
|
||||
expected_hidden=[shared.pk, not_shared.pk],
|
||||
)
|
||||
|
||||
def test_group_permission_grants_visibility_to_members_only(
|
||||
self,
|
||||
model,
|
||||
factory,
|
||||
perm,
|
||||
):
|
||||
owner = User.objects.create_user(username=f"owner3_{model.__name__}")
|
||||
member = User.objects.create_user(username=f"member_{model.__name__}")
|
||||
non_member = User.objects.create_user(username=f"nonmember_{model.__name__}")
|
||||
group = Group.objects.create(name=f"group_{model.__name__}")
|
||||
member.groups.add(group)
|
||||
shared = factory(owner=owner)
|
||||
assign_perm(perm, group, shared)
|
||||
|
||||
assert_visible_document_ids(
|
||||
permitted_object_ids(member, model, perm),
|
||||
expected_visible=[shared.pk],
|
||||
expected_hidden=[],
|
||||
)
|
||||
assert_visible_document_ids(
|
||||
permitted_object_ids(non_member, model, perm),
|
||||
expected_visible=[],
|
||||
expected_hidden=[shared.pk],
|
||||
)
|
||||
|
||||
def test_superuser_sees_everything(self, model, factory, perm):
|
||||
superuser = User.objects.create_superuser(username=f"root_{model.__name__}")
|
||||
owner = User.objects.create_user(username=f"owner4_{model.__name__}")
|
||||
obj = factory(owner=owner)
|
||||
|
||||
assert_visible_document_ids(
|
||||
permitted_object_ids(superuser, model, perm),
|
||||
expected_visible=[obj.pk],
|
||||
expected_hidden=[],
|
||||
)
|
||||
|
||||
Reference in New Issue
Block a user