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:
stumpylog
2026-08-06 08:35:03 -07:00
co-authored by Claude Sonnet 5
parent 52a0484f74
commit 0193dd5b99
2 changed files with 146 additions and 23 deletions
+48 -23
View File
@@ -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=[],
)