amorfo77/paperless-ngx
1
0
Fork 0
mirror of https://github.com/paperless-ngx/paperless-ngx.git synced 2026-09-20 00:18:32 +00:00
Code Issues Projects Releases Wiki Activity

feat: add perm param to permitted_document_ids for change/delete checks (#13508)

Browse Source 
* feat: add perm param to permitted_document_ids for change/delete checks

Widens permitted_document_ids(user, *, include_deleted=False) to
permitted_document_ids(user, *, perm="view_document", include_deleted=False)
so Stage 2 callers can check change_document/delete_document permissions
instead of the hardcoded view_document codename. Default is unchanged for
every existing call site.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01GRp4kf1mdn9ruv81zWAmh2

* test: strengthen delete-permission include_deleted test to discriminate from other grants

test_delete_permission_with_include_deleted_for_trash_restore only checked
an owner and a fully-ungranted stranger, so it never proved perm=
actually discriminates delete_document from other permission grants. Add
a view_only user with view_document (but not delete_document) granted on
the same doc and assert they remain excluded, mirroring the pattern in
test_change_document_permission_is_distinct_from_view.

* fix: normalize qualified permission strings in permitted_document_ids

Guardian's UserObjectPermission/GroupObjectPermission always store a bare
codename, but has_perm()-style callers commonly pass the qualified
"app_label.codename" form. Passing that qualified form here previously
matched zero rows, silently under-permitting. content_type already
disambiguates the codename, so just strip any prefix instead.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>

---------

Co-authored-by: Claude Sonnet 5 <noreply@anthropic.com>
This commit is contained in:
Trenton H
2026-08-04 08:01:24 -07:00
committed by GitHub
co-authored by Claude Sonnet 5
parent c6cecd3c4e
commit 1f1725ba56
2 changed files with 85 additions and 7 deletions
Download Patch File Download Diff File Expand all files Collapse all files
src/documents/permissions.py
+18 -7
View File
@@ -163,13 +163,18 @@ def set_permissions_for_object(
)
def permitted_document_ids(user, *, include_deleted: bool = False):
def permitted_document_ids(
user,
*,
perm: str = "view_document",
include_deleted: bool = False,
):
"""
Return a queryset of document IDs the user may view. 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 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.
"""
manager = Document.global_objects if include_deleted else Document.objects
@@ -183,9 +188,15 @@ def permitted_document_ids(user, *, include_deleted: bool = False):
if getattr(user, "is_superuser", False):
return base_docs.values_list("id", flat=True)
# Guardian's UserObjectPermission/GroupObjectPermission always store a bare
# codename, but has_perm()-style callers commonly pass the qualified
# "app_label.codename" form. content_type already disambiguates the
# codename, so just drop any prefix rather than silently under-permitting.
perm = perm.rsplit(".", 1)[-1]
document_ct = ContentType.objects.get_for_model(Document)
perm_filter = {
"permission__codename": "view_document",
"permission__codename": perm,
"permission__content_type": document_ct,
}
src/documents/tests/test_permission_filtering_security.py
+67
View File
@@ -217,3 +217,70 @@ class TestDuplicateDocumentsPermissionBoundary:
result_stranger = _get_viewable_duplicates(original, stranger)
assert {d.pk for d in result_stranger} == {dup_visible.pk}
@pytest.mark.django_db
class TestPermittedDocumentIdsArbitraryPermission:
def test_change_document_permission_is_distinct_from_view(self):
owner = User.objects.create_user(username="owner")
viewer_only = User.objects.create_user(username="viewer")
editor = User.objects.create_user(username="editor")
doc = DocumentFactory(owner=owner)
assign_perm("view_document", viewer_only, doc)
assign_perm("change_document", editor, doc)
assign_perm("view_document", editor, doc)
assert_visible_document_ids(
permitted_document_ids(editor, perm="change_document"),
expected_visible=[doc.pk],
expected_hidden=[],
)
assert_visible_document_ids(
permitted_document_ids(viewer_only, perm="change_document"),
expected_visible=[],
expected_hidden=[doc.pk],
)
def test_qualified_permission_string_is_normalized_to_codename(self):
owner = User.objects.create_user(username="owner")
editor = User.objects.create_user(username="editor")
doc = DocumentFactory(owner=owner)
assign_perm("change_document", editor, doc)
assert_visible_document_ids(
permitted_document_ids(editor, perm="documents.change_document"),
expected_visible=[doc.pk],
expected_hidden=[],
)
def test_delete_permission_with_include_deleted_for_trash_restore(self):
owner = User.objects.create_user(username="owner")
stranger = User.objects.create_user(username="mallory")
view_only = User.objects.create_user(username="viewer")
doc = DocumentFactory(owner=owner)
assign_perm("view_document", view_only, doc)
doc.delete()
assert_visible_document_ids(
permitted_document_ids(owner, perm="delete_document", include_deleted=True),
expected_visible=[doc.pk],
expected_hidden=[],
)
assert_visible_document_ids(
permitted_document_ids(
stranger,
perm="delete_document",
include_deleted=True,
),
expected_visible=[],
expected_hidden=[doc.pk],
)
assert_visible_document_ids(
permitted_document_ids(
view_only,
perm="delete_document",
include_deleted=True,
),
expected_visible=[],
expected_hidden=[doc.pk],
)