diff --git a/src/paperless_ai/ai_classifier.py b/src/paperless_ai/ai_classifier.py index a1911e6c1..0559446d7 100644 --- a/src/paperless_ai/ai_classifier.py +++ b/src/paperless_ai/ai_classifier.py @@ -4,8 +4,7 @@ from django.conf import settings from django.contrib.auth.models import User from documents.models import Document -from documents.permissions import permitted_object_ids -from documents.permissions import restrict_queryset_to_visible +from documents.permissions import permitted_document_ids from documents.permissions import user_is_unrestricted from paperless.config import AIConfig from paperless_ai.base_model import ClassificationSuggestions @@ -54,8 +53,8 @@ def _fulltext_similar_documents( active superuser - see user_is_unrestricted) is normalized to ``None`` before calling, since the backend's permission filter has no superuser short-circuit of its own. Results are re-checked with - restrict_queryset_to_visible() since Tantivy's indexed permission fields - lag the DB via async reindexing. + permitted_document_ids() since Tantivy's indexed permission fields lag the + DB via async reindexing and judge a version by its own owner. """ from documents.search import get_backend @@ -69,10 +68,9 @@ def _fulltext_similar_documents( ) if not unrestricted: allowed_ids = set( - restrict_queryset_to_visible( - Document.objects.filter(pk__in=similar_ids), - user, - "view_document", + Document.objects.filter( + pk__in=similar_ids, + id__in=permitted_document_ids(user), ).values_list("pk", flat=True), ) similar_ids = [doc_id for doc_id in similar_ids if doc_id in allowed_ids] @@ -200,13 +198,13 @@ def get_taxonomy_context( # quadratic scan in the vector store at best, and past ~32,763 # documents a hard sqlite3.OperationalError (SQLite's # bound-parameter limit) at worst. - # permitted_object_ids() has its own superuser shortcut that would + # permitted_document_ids() has its own superuser shortcut that would # return every Document's id anyway, so this changes nothing about # which documents are considered -- only how we get there. visible_document_ids = ( None if user_is_unrestricted(user) - else list(permitted_object_ids(user, Document, "view_document")) + else list(permitted_document_ids(user)) ) nodes = retrieve_similar_nodes( document, diff --git a/src/paperless_ai/tests/test_ai_classifier.py b/src/paperless_ai/tests/test_ai_classifier.py index eeb8eabf7..1e018a62f 100644 --- a/src/paperless_ai/tests/test_ai_classifier.py +++ b/src/paperless_ai/tests/test_ai_classifier.py @@ -552,7 +552,7 @@ class TestGetTaxonomyContextVisibility: return_value=[], ) mock_permitted = mocker.patch( - "paperless_ai.ai_classifier.permitted_object_ids", + "paperless_ai.ai_classifier.permitted_document_ids", ) user = UserFactory.create(is_superuser=True) @@ -582,7 +582,7 @@ class TestGetTaxonomyContextVisibility: return_value=[], ) mock_permitted = mocker.patch( - "paperless_ai.ai_classifier.permitted_object_ids", + "paperless_ai.ai_classifier.permitted_document_ids", ) get_taxonomy_context(document, None) @@ -611,16 +611,55 @@ class TestGetTaxonomyContextVisibility: return_value=[], ) mock_permitted = mocker.patch( - "paperless_ai.ai_classifier.permitted_object_ids", + "paperless_ai.ai_classifier.permitted_document_ids", return_value=[1, 2, 3], ) user = UserFactory.create(is_superuser=False) get_taxonomy_context(document, user) - mock_permitted.assert_called_once_with(user, Document, "view_document") + mock_permitted.assert_called_once_with(user) assert mock_retrieve.call_args.kwargs["document_ids"] == [1, 2, 3] + @pytest.mark.django_db + @override_settings(LLM_EMBEDDING_BACKEND="huggingface") + def test_version_of_private_root_is_not_visible( + self, + mocker: pytest_mock.MockerFixture, + ) -> None: + """ + GIVEN: + - A private root document owned by someone else + - A version of it whose own owner is unset, as when the root + changed hands after the version was created + WHEN: + - get_taxonomy_context() is called for a non-superuser + THEN: + - Neither the root nor the version is in the visible ids passed to + retrieve_similar_nodes(), since a version follows its root + """ + owner = UserFactory.create() + viewer = UserFactory.create(is_superuser=False) + root = DocumentFactory.create(content="private", owner=owner) + version = DocumentFactory.create( + content="private", + owner=None, + root_document=root, + version_index=1, + ) + source = DocumentFactory.create(content="Some content", owner=viewer) + mock_retrieve = mocker.patch( + "paperless_ai.ai_classifier.retrieve_similar_nodes", + return_value=[], + ) + + get_taxonomy_context(source, viewer) + + visible = mock_retrieve.call_args.kwargs["document_ids"] + assert source.pk in visible + assert root.pk not in visible + assert version.pk not in visible + @pytest.mark.django_db class TestFulltextSimilarDocuments: @@ -803,7 +842,7 @@ class TestFulltextSimilarDocuments: - _fulltext_similar_documents() is called with that user THEN: - Only the still-permitted document is returned - the DB - re-check via restrict_queryset_to_visible() must catch the + re-check via permitted_document_ids() must catch the document Tantivy's stale index still thinks is visible """ owner = UserFactory.create() @@ -834,6 +873,45 @@ class TestFulltextSimilarDocuments: assert [s["document_id"] for s in result] == [permitted.pk] + def test_excludes_version_of_private_root_for_regular_user( + self, + fulltext_backend: TantivyBackend, + ) -> None: + """ + GIVEN: + - A regular user and a private root owned by someone else + - A version of that root with no owner of its own, which the + Tantivy index therefore treats as visible to everyone + WHEN: + - _fulltext_similar_documents() is called with that user + THEN: + - The version is not returned, since the DB re-check judges it by + its root + """ + owner = UserFactory.create() + viewer = UserFactory.create(is_superuser=False) + source = DocumentFactory.create( + content="shared content phrase", + owner=viewer, + ) + root = DocumentFactory.create( + content="shared content phrase", + owner=owner, + ) + version = DocumentFactory.create( + content="shared content phrase", + owner=None, + root_document=root, + version_index=1, + ) + fulltext_backend.add_or_update(source) + fulltext_backend.add_or_update(root) + fulltext_backend.add_or_update(version) + + result = _fulltext_similar_documents(source, user=viewer, top_k=5) + + assert version.pk not in [s["document_id"] for s in result] + @pytest.mark.django_db @override_settings(LLM_EMBEDDING_BACKEND="huggingface")