Performance: eliminate per-document guardian permission-check causing high CPU on document lists (#13505)

* test: add permission-filtering security regression suite for Document

* test: verify document is actually soft-deleted (not hard-deleted) in soft-delete visibility test

Addresses Copilot review feedback on PR #13505: refresh_from_db() and
assert deleted_at is set before checking visibility, so this test
actually validates the deleted_at__isnull=True filtering behavior
rather than just checking the document disappeared from the queryset
for any reason.
This commit is contained in:
Trenton H
2026-08-04 08:01:23 -07:00
committed by GitHub
parent 92a1b24583
commit 3a484a3ff2
@@ -0,0 +1,127 @@
from __future__ import annotations
import pytest
from django.contrib.auth.models import AnonymousUser
from django.contrib.auth.models import Group
from django.contrib.auth.models import User
from guardian.shortcuts import assign_perm
from documents.permissions import permitted_document_ids
from documents.tests.factories import DocumentFactory
def assert_visible_document_ids(actual_ids, *, expected_visible, expected_hidden):
actual_ids = set(actual_ids)
expected_visible = set(expected_visible)
assert actual_ids == expected_visible, (
f"visible set mismatch: missing={expected_visible - actual_ids}, "
f"unexpected={actual_ids - expected_visible}"
)
for doc_id in expected_hidden:
assert doc_id not in actual_ids, (
f"document {doc_id} leaked but should be hidden"
)
@pytest.mark.django_db
class TestPermittedDocumentIdsSecurity:
def test_owner_sees_own_document(self):
user = User.objects.create_user(username="alice")
stranger = User.objects.create_user(username="mallory")
owned = DocumentFactory(owner=user)
strangers_doc = DocumentFactory(owner=stranger)
visible = permitted_document_ids(user)
assert_visible_document_ids(
visible,
expected_visible=[owned.pk],
expected_hidden=[strangers_doc.pk],
)
def test_unowned_document_visible_to_everyone(self):
user = User.objects.create_user(username="alice")
unowned = DocumentFactory(owner=None)
assert_visible_document_ids(
permitted_document_ids(user),
expected_visible=[unowned.pk],
expected_hidden=[],
)
def test_explicit_user_permission_grants_visibility(self):
grantee = User.objects.create_user(username="alice")
stranger = User.objects.create_user(username="mallory")
owner = User.objects.create_user(username="owner")
shared = DocumentFactory(owner=owner)
not_shared = DocumentFactory(owner=owner)
assign_perm("view_document", grantee, shared)
assert_visible_document_ids(
permitted_document_ids(grantee),
expected_visible=[shared.pk],
expected_hidden=[not_shared.pk],
)
assert_visible_document_ids(
permitted_document_ids(stranger),
expected_visible=[],
expected_hidden=[shared.pk, not_shared.pk],
)
def test_explicit_group_permission_grants_visibility_to_members_only(self):
owner = User.objects.create_user(username="owner")
member = User.objects.create_user(username="member")
non_member = User.objects.create_user(username="non_member")
group = Group.objects.create(name="finance")
member.groups.add(group)
shared = DocumentFactory(owner=owner)
assign_perm("view_document", group, shared)
assert_visible_document_ids(
permitted_document_ids(member),
expected_visible=[shared.pk],
expected_hidden=[],
)
assert_visible_document_ids(
permitted_document_ids(non_member),
expected_visible=[],
expected_hidden=[shared.pk],
)
def test_soft_deleted_document_excluded_by_default(self):
owner = User.objects.create_user(username="owner")
doc = DocumentFactory(owner=owner)
doc.delete() # soft delete
doc.refresh_from_db()
assert doc.deleted_at is not None, (
"document should be soft-deleted, not hard-deleted, for this "
"test to actually validate the deleted_at filtering behavior"
)
assert_visible_document_ids(
permitted_document_ids(owner),
expected_visible=[],
expected_hidden=[doc.pk],
)
def test_superuser_sees_everything_including_no_perm_documents(self):
superuser = User.objects.create_superuser(username="root")
owner = User.objects.create_user(username="owner")
doc = DocumentFactory(owner=owner)
assert_visible_document_ids(
permitted_document_ids(superuser),
expected_visible=[doc.pk],
expected_hidden=[],
)
def test_anonymous_user_sees_only_unowned_documents(self):
owner = User.objects.create_user(username="owner")
owned = DocumentFactory(owner=owner)
unowned = DocumentFactory(owner=None)
assert_visible_document_ids(
permitted_document_ids(AnonymousUser()),
expected_visible=[unowned.pk],
expected_hidden=[owned.pk],
)