From 5e54259db9f8b7eddd77e9a6e8fe2c2528c6523b Mon Sep 17 00:00:00 2001 From: Trenton H <797416+stumpylog@users.noreply.github.com> Date: Tue, 4 Aug 2026 08:01:23 -0700 Subject: [PATCH] feat: add include_deleted param to permitted_document_ids (#13506) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * feat: add include_deleted param to permitted_document_ids Widens permitted_document_ids to accept an include_deleted keyword-only flag (default False, preserving current behavior) so later call sites that need visibility into soft-deleted documents (e.g. trash restore) can reuse this permission check instead of duplicating it. * refactor: remove redundant deleted_at filter in permitted_document_ids Document.objects already applies filter(deleted_at__isnull=True) internally via SoftDeleteManager.get_queryset(), so the conditional filter was redundant. Simplify to just use manager.all() in both branches — manager selection alone ensures correct behavior (Document.objects excludes deleted, Document.global_objects includes all). Co-Authored-By: Claude Haiku Claude-Session: https://claude.ai/code/session --------- Co-authored-by: Claude Haiku --- src/documents/permissions.py | 14 ++++++---- .../test_permission_filtering_security.py | 26 +++++++++++++++++++ 2 files changed, 35 insertions(+), 5 deletions(-) diff --git a/src/documents/permissions.py b/src/documents/permissions.py index 006b77a81..5936f405a 100644 --- a/src/documents/permissions.py +++ b/src/documents/permissions.py @@ -163,14 +163,18 @@ def set_permissions_for_object( ) -def permitted_document_ids(user): +def permitted_document_ids(user, *, include_deleted: bool = False): """ - Return a queryset of document IDs the user may view, limited to non-deleted - documents. This intentionally avoids ``get_objects_for_user`` to keep the - subquery small and index-friendly. + 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. """ - base_docs = Document.objects.filter(deleted_at__isnull=True).only("id", "owner") + manager = Document.global_objects if include_deleted else Document.objects + base_docs = manager.all() + base_docs = base_docs.only("id", "owner") if user is None or not getattr(user, "is_authenticated", False): # Just Anonymous user e.g. for drf-spectacular diff --git a/src/documents/tests/test_permission_filtering_security.py b/src/documents/tests/test_permission_filtering_security.py index aae36f563..0db6a692a 100644 --- a/src/documents/tests/test_permission_filtering_security.py +++ b/src/documents/tests/test_permission_filtering_security.py @@ -125,3 +125,29 @@ class TestPermittedDocumentIdsSecurity: expected_visible=[unowned.pk], expected_hidden=[owned.pk], ) + + +@pytest.mark.django_db +class TestPermittedDocumentIdsIncludeDeleted: + def test_include_deleted_true_reveals_soft_deleted_owned_document(self): + owner = User.objects.create_user(username="owner") + doc = DocumentFactory(owner=owner) + doc.delete() + + assert_visible_document_ids( + permitted_document_ids(owner, include_deleted=True), + expected_visible=[doc.pk], + expected_hidden=[], + ) + + def test_include_deleted_true_still_respects_permission_boundary(self): + owner = User.objects.create_user(username="owner") + stranger = User.objects.create_user(username="mallory") + doc = DocumentFactory(owner=owner) + doc.delete() + + assert_visible_document_ids( + permitted_document_ids(stranger, include_deleted=True), + expected_visible=[], + expected_hidden=[doc.pk], + )