♻️(backend) scope document search by document id instead of path

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.

Author: lunika

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
 

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)

src/backend/core/api/viewsets.py

@@ -1542,15 +1542,23 @@ def _search_using_indexer(indexer, request, params, search_type):
         """
         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 @@ def _search_using_database(self, request, validated_data, *args, **kwargs):
         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 @@ def _search_using_database(self, request, validated_data, *args, **kwargs):
 
     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"):

src/backend/core/tests/documents/test_api_documents_search.py

@@ -547,15 +547,17 @@ 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
     assert response.status_code == 200
     # 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
 
 

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