From 9ef4a15f5e7437c73ad04d2115a15dba3ffb7a76 Mon Sep 17 00:00:00 2001 From: Manuel Raynaud Date: Tue, 7 Jul 2026 09:16:32 +0200 Subject: [PATCH] =?UTF-8?q?=E2=99=BB=EF=B8=8F(backend)=20scope=20document?= =?UTF-8?q?=20search=20by=20document=20id=20instead=20of=20path?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The search in a document tree was triggered by the usage of the document path. The path is something guessable by incrementing it you can discover public documents. We decided to change this to use the document id which is not guessable and prevent discovering public documents. Thanks to @maboukerfa for discovering it. --- CHANGELOG.md | 1 + src/backend/core/api/serializers.py | 2 +- src/backend/core/api/viewsets.py | 32 ++++++++++++------- .../documents/test_api_documents_search.py | 6 ++-- .../test_api_documents_search_descendants.py | 26 +++++++-------- 5 files changed, 39 insertions(+), 28 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 34cb89ce9..6ec86b65c 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -25,6 +25,7 @@ and this project adheres to - 🚚(frontend) move Waffle to bottom left #2455 - ♿️(frontend) remove redundant aria-label on table of contents links #2459 - ♻️(core) fix typo in settings COLLABORATION_WS_NOT_CONNECTED_READY_ONLY #2481 +- ♻️(backend) scope document search by document id instead of path #2501 ### Fixed diff --git a/src/backend/core/api/serializers.py b/src/backend/core/api/serializers.py index 4e32f84b4..0b465f135 100644 --- a/src/backend/core/api/serializers.py +++ b/src/backend/core/api/serializers.py @@ -1009,4 +1009,4 @@ class SearchQueryParamDocumentSerializer(serializers.Serializer): """Serializer for fulltext search requests through Find application""" q = serializers.CharField(required=True, allow_blank=True, trim_whitespace=True) - path = serializers.CharField(required=False, allow_blank=False) + document = serializers.UUIDField(required=False) diff --git a/src/backend/core/api/viewsets.py b/src/backend/core/api/viewsets.py index a15fc6137..0b8e5f213 100644 --- a/src/backend/core/api/viewsets.py +++ b/src/backend/core/api/viewsets.py @@ -1542,15 +1542,23 @@ class DocumentViewSet( """ queryset = models.Document.objects.all() + # The indexer filters descendants by path prefix, so resolve the document + # id to its path before querying it. + path = None + document_id = params.validated_data.get("document") + if document_id: + try: + path = models.Document.objects.get(pk=document_id).values_list( + "path", flat=True + ) + except models.Document.DoesNotExist as exc: + raise drf.exceptions.NotFound("Document not found.") from exc + results = indexer.search( q=params.validated_data["q"], search_type=search_type, token=request.session.get("oidc_access_token"), - path=( - params.validated_data["path"] - if "path" in params.validated_data - else None - ), + path=path, visited=get_visited_document_ids_of(queryset, request.user), ) @@ -1618,7 +1626,7 @@ class DocumentViewSet( Only searches in the title field of documents. """ - if validated_data.get("path"): + if validated_data.get("document"): return self._list_descendants(request, validated_data) top_level_documents = self.get_queryset() @@ -1676,22 +1684,22 @@ class DocumentViewSet( def _list_descendants(self, request, validated_data): """ - List all documents whose path starts with the provided path parameter. - Includes the parent document itself. - Used internally by the search endpoint when path filtering is requested. + List all documents descending from the document identified by the provided + document id. Includes the parent document itself. + Used internally by the search endpoint when document filtering is requested. """ # Get parent document without access filtering - parent_path = validated_data["path"] + document_id = validated_data["document"] user = request.user try: parent = ( models.Document.objects.annotate_user_roles(user) .annotate_is_favorite(user) .annotate_user_has_link_trace(user) - .get(path=parent_path) + .get(pk=document_id) ) except models.Document.DoesNotExist as exc: - raise drf.exceptions.NotFound("Document not found from path.") from exc + raise drf.exceptions.NotFound("Document not found.") from exc abilities = parent.get_abilities(user) if not abilities.get("search"): diff --git a/src/backend/core/tests/documents/test_api_documents_search.py b/src/backend/core/tests/documents/test_api_documents_search.py index f20b0b514..4ccc1f00b 100644 --- a/src/backend/core/tests/documents/test_api_documents_search.py +++ b/src/backend/core/tests/documents/test_api_documents_search.py @@ -547,7 +547,7 @@ def test_api_documents_search_indexer_crashes( parent = factories.DocumentFactory(title="parent", users=[user]) q = "alpha" response = client.get( - "/api/v1.0/documents/search/", data={"q": "alpha", "path": parent.path} + "/api/v1.0/documents/search/", data={"q": "alpha", "document": parent.id} ) # the search endpoint did not crash @@ -555,7 +555,9 @@ def test_api_documents_search_indexer_crashes( # fallback on title_search assert mock_search_using_database.call_count == 1 assert mock_search_using_database.call_args[0][0].GET.get("q") == q - assert mock_search_using_database.call_args[0][0].GET.get("path") == parent.path + assert mock_search_using_database.call_args[0][0].GET.get("document") == str( + parent.id + ) assert response.json() == mocked_response diff --git a/src/backend/core/tests/documents/test_api_documents_search_descendants.py b/src/backend/core/tests/documents/test_api_documents_search_descendants.py index 8abf2984c..a542e4658 100644 --- a/src/backend/core/tests/documents/test_api_documents_search_descendants.py +++ b/src/backend/core/tests/documents/test_api_documents_search_descendants.py @@ -1,6 +1,6 @@ """ Tests for search API endpoint in impress's core app when indexer is not -available and a path param is given. +available and a document param is given. """ # pylint: disable=too-many-lines @@ -34,7 +34,7 @@ def test_api_documents_search_descendants_list_anonymous_public_standalone(): factories.UserDocumentAccessFactory(document=child1) response = APIClient().get( - "/api/v1.0/documents/search/", data={"q": "doc", "path": document.path} + "/api/v1.0/documents/search/", data={"q": "doc", "document": document.id} ) assert response.status_code == 200 @@ -248,7 +248,7 @@ def test_api_documents_search_descendants_list_anonymous_public_parent(): factories.UserDocumentAccessFactory(document=child1) response = APIClient().get( - "/api/v1.0/documents/search/", data={"q": "doc", "path": document.path} + "/api/v1.0/documents/search/", data={"q": "doc", "document": document.id} ) assert response.status_code == 200 @@ -448,7 +448,7 @@ def test_api_documents_search_descendants_list_anonymous_restricted_or_authentic _grand_child = factories.DocumentFactory(title="grand child", parent=child) response = APIClient().get( - "/api/v1.0/documents/search/", data={"q": "child", "path": document.path} + "/api/v1.0/documents/search/", data={"q": "child", "document": document.id} ) assert response.status_code == 403 @@ -478,7 +478,7 @@ def test_api_documents_search_descendants_list_authenticated_unrelated_public_or factories.UserDocumentAccessFactory(document=child1) response = client.get( - "/api/v1.0/documents/search/", data={"q": "child", "path": document.path} + "/api/v1.0/documents/search/", data={"q": "child", "document": document.id} ) assert response.status_code == 200 @@ -669,7 +669,7 @@ def test_api_documents_search_descendants_list_authenticated_public_or_authentic factories.UserDocumentAccessFactory(document=child1) response = client.get( - "/api/v1.0/documents/search/", data={"q": "child", "path": document.path} + "/api/v1.0/documents/search/", data={"q": "child", "document": document.id} ) assert response.status_code == 200 @@ -850,7 +850,7 @@ def test_api_documents_search_descendants_list_authenticated_unrelated_restricte factories.UserDocumentAccessFactory(document=child1) response = client.get( - "/api/v1.0/documents/search/", data={"q": "child", "path": document.path} + "/api/v1.0/documents/search/", data={"q": "child", "document": document.id} ) assert response.status_code == 403 @@ -881,7 +881,7 @@ def test_api_documents_search_descendants_list_authenticated_related_direct(): grand_child = factories.DocumentFactory(parent=child1, title="grand child") response = client.get( - "/api/v1.0/documents/search/", data={"q": "child", "path": document.path} + "/api/v1.0/documents/search/", data={"q": "child", "document": document.id} ) assert response.status_code == 200 assert response.json() == { @@ -1073,7 +1073,7 @@ def test_api_documents_search_descendants_list_authenticated_related_parent(): grand_child = factories.DocumentFactory(parent=child1, title="grand child") response = client.get( - "/api/v1.0/documents/search/", data={"q": "child", "path": document.path} + "/api/v1.0/documents/search/", data={"q": "child", "document": document.id} ) assert response.status_code == 200 assert response.json() == { @@ -1252,7 +1252,7 @@ def test_api_documents_search_descendants_list_authenticated_related_child(): factories.UserDocumentAccessFactory(document=document) response = client.get( - "/api/v1.0/documents/search/", data={"q": "doc", "path": document.path} + "/api/v1.0/documents/search/", data={"q": "doc", "document": document.id} ) assert response.status_code == 403 assert response.json() == { @@ -1279,7 +1279,7 @@ def test_api_documents_search_descendants_list_authenticated_related_team_none( factories.TeamDocumentAccessFactory(document=document, team="myteam") response = client.get( - "/api/v1.0/documents/search/", data={"q": "doc", "path": document.path} + "/api/v1.0/documents/search/", data={"q": "doc", "document": document.id} ) assert response.status_code == 403 @@ -1310,7 +1310,7 @@ def test_api_documents_search_descendants_list_authenticated_related_team_member access = factories.TeamDocumentAccessFactory(document=document, team="myteam") response = client.get( - "/api/v1.0/documents/search/", data={"q": "child", "path": document.path} + "/api/v1.0/documents/search/", data={"q": "child", "document": document.id} ) # pylint: disable=R0801 @@ -1509,7 +1509,7 @@ def test_api_documents_search_descendants_search_on_title(query, nb_results): # Perform the search query response = client.get( - "/api/v1.0/documents/search/", data={"q": query, "path": parent.path} + "/api/v1.0/documents/search/", data={"q": query, "document": parent.id} ) assert response.status_code == 200