mirror of
https://github.com/paperless-ngx/paperless-ngx.git
synced 2026-10-10 01:57:12 +00:00
Fix: judge versions by their root in the AI similarity permission checks
The AI classifier asked permitted_object_ids for the Document model directly, which judges a version by its own owner and grants. A version whose owner had drifted from its private root's could be offered as similar-document context to a user who cannot see the root. Use permitted_document_ids in both the vector and full-text paths, which authorizes a version by its root. Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
This commit is contained in:
1 parent
a41e06f196
commit
01e1e76f91
2 files changed
+91
-15
No files matched your search
@@ -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,
|
||||
|
||||
@@ -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")
|
||||
|
||||
Reference in new issue
Block a user