diff --git a/CHANGELOG.md b/CHANGELOG.md index af02f44..d75c545 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -27,6 +27,7 @@ and this project adheres to issues if the opensearch database is shared between apps. - ✨(backend) add tags - ✨(backend) adapt to conversation RAG +- ✨(backend) add deletion endpoint ## Fixed diff --git a/src/backend/core/schemas.py b/src/backend/core/schemas.py index 1d8b96d..dffc11d 100644 --- a/src/backend/core/schemas.py +++ b/src/backend/core/schemas.py @@ -117,3 +117,10 @@ class SearchQueryParametersSchema(BaseModel): order_by: Optional[Literal[enums.ORDER_BY_OPTIONS]] = Field(default=enums.RELEVANCE) order_direction: Optional[Literal["asc", "desc"]] = Field(default="desc") nb_results: Optional[conint(ge=1, le=300)] = Field(default=50) + + +class DeleteDocumentsSchema(BaseModel): + """Schema for validating the delete documents request""" + + service: str = Field(max_length=300) + document_ids: List[str] = Field(min_length=1) diff --git a/src/backend/core/tests/test_api_documents_delete.py b/src/backend/core/tests/test_api_documents_delete.py new file mode 100644 index 0000000..1a1a2b9 --- /dev/null +++ b/src/backend/core/tests/test_api_documents_delete.py @@ -0,0 +1,210 @@ +"""Tests for deleting documents from OpenSearch over the API""" + +import opensearchpy +import pytest +import responses +from rest_framework.test import APIClient + +from core import factories +from core.services.opensearch import opensearch_client +from core.utils import prepare_index + +from .utils import build_authorization_bearer, setup_oicd_resource_server + +pytestmark = pytest.mark.django_db + + +def test_api_documents_delete_anonymous(): + """Anonymous requests should not be allowed to delete documents.""" + response = APIClient().post( + "/api/v1.0/documents/delete/", + {"service": "service-name", "document_ids": ["doc1"]}, + format="json", + ) + + assert response.status_code == 401 + + +@responses.activate +def test_api_documents_delete_wrong_service_name(settings): + """Requests with a wrong service name should return 400 Bad Request.""" + setup_oicd_resource_server(responses, settings, sub="user_sub") + + response = APIClient().post( + "/api/v1.0/documents/delete/", + {"service": "wrong-service", "document_ids": ["0"]}, + format="json", + HTTP_AUTHORIZATION=f"Bearer {build_authorization_bearer()}", + ) + + assert response.status_code == 400 + assert response.json()["detail"] == "Invalid request." + + +@responses.activate +def test_api_documents_delete_success(settings): + """Authenticated users should be able to delete documents they have access to.""" + setup_oicd_resource_server(responses, settings, sub="user_sub") + + service = factories.ServiceFactory() + # Create documents user has access to + documents = factories.DocumentSchemaFactory.build_batch(3, users=["user_sub"]) + prepare_index(service.index_name, documents) + document_to_delete_ids = [doc["id"] for doc in documents[:2]] + + response = APIClient().post( + "/api/v1.0/documents/delete/", + {"service": service.name, "document_ids": document_to_delete_ids}, + format="json", + HTTP_AUTHORIZATION=f"Bearer {build_authorization_bearer()}", + ) + + assert response.status_code == 200 + assert response.json()["nb-deleted-documents"] == 2 + assert response.json()["undeleted-document-ids"] == [] + + opensearch_client_ = opensearch_client() + for document in documents: + if document["id"] in document_to_delete_ids: + with pytest.raises(opensearchpy.exceptions.NotFoundError): + opensearch_client_.get(index=service.index_name, id=document["id"]) + else: + doc = opensearch_client_.get(index=service.index_name, id=document["id"]) + assert doc["found"] + + +@responses.activate +def test_api_documents_delete_no_access(settings): + """Users should not be able to delete documents they don't have access to.""" + setup_oicd_resource_server(responses, settings, sub="user_sub") + service = factories.ServiceFactory() + # Create documents where user_sub does NOT have access + documents = factories.DocumentSchemaFactory.build_batch(2, users=["other_sub"]) + prepare_index(service.index_name, documents) + + document_ids = [doc["id"] for doc in documents] + + response = APIClient().post( + "/api/v1.0/documents/delete/", + {"service": service.name, "document_ids": document_ids}, + format="json", + HTTP_AUTHORIZATION=f"Bearer {build_authorization_bearer()}", + ) + + assert response.status_code == 200 + assert response.json()["nb-deleted-documents"] == 0 + assert set(response.json()["undeleted-document-ids"]) == set(document_ids) + + # Verify documents not deleted + opensearch_client_ = opensearch_client() + for doc_id in document_ids: + doc = opensearch_client_.get(index=service.index_name, id=doc_id) + assert doc["found"] + + +@responses.activate +def test_api_documents_delete_mixed_access(settings): + """Deleting a mix of owned and non-owned documents should only delete owned ones.""" + setup_oicd_resource_server(responses, settings, sub="user_sub") + + service = factories.ServiceFactory() + # Create documents with different access + owned_documents = factories.DocumentSchemaFactory.build_batch(2, users=["user_sub"]) + other_documents = factories.DocumentSchemaFactory.build_batch( + 2, users=["other_user"] + ) + prepare_index(service.index_name, owned_documents + other_documents) + + owned_document_ids = [doc["id"] for doc in owned_documents] + other_document_ids = [doc["id"] for doc in other_documents] + non_existing_document_ids = ["non-existent-1", "non-existent-2"] + + response = APIClient().post( + "/api/v1.0/documents/delete/", + { + "service": service.name, + "document_ids": owned_document_ids + + other_document_ids + + non_existing_document_ids, + }, + format="json", + HTTP_AUTHORIZATION=f"Bearer {build_authorization_bearer()}", + ) + + assert response.status_code == 200 + assert response.json()["nb-deleted-documents"] == 2 + assert set(response.json()["undeleted-document-ids"]) == set( + other_document_ids + non_existing_document_ids + ) + + # Verify only owned documents are deleted + opensearch_client_ = opensearch_client() + for document_id in owned_document_ids: + with pytest.raises(opensearchpy.exceptions.NotFoundError): + opensearch_client_.get(index=service.index_name, id=document_id) + + for document_id in other_document_ids: + document = opensearch_client_.get(index=service.index_name, id=document_id) + assert document["found"] + + +@responses.activate +def test_api_documents_delete_invalid_params(settings): + """Requests with invalid parameters should return 400 Bad Request.""" + setup_oicd_resource_server(responses, settings, sub="user_sub") + service = factories.ServiceFactory() + + # Missing document_ids + response = APIClient().post( + "/api/v1.0/documents/delete/", + { + "service": service.name, + }, + format="json", + HTTP_AUTHORIZATION=f"Bearer {build_authorization_bearer()}", + ) + + assert response.status_code == 400 + + # Empty document_ids + response = APIClient().post( + "/api/v1.0/documents/delete/", + {"service": service.name, "document_ids": []}, + format="json", + HTTP_AUTHORIZATION=f"Bearer {build_authorization_bearer()}", + ) + + assert response.status_code == 400 + + # Missing service + response = APIClient().post( + "/api/v1.0/documents/delete/", + {"document_ids": ["doc1"]}, + format="json", + HTTP_AUTHORIZATION=f"Bearer {build_authorization_bearer()}", + ) + + assert response.status_code == 400 + + +@responses.activate +def test_api_documents_delete_nonexistent_documents(settings): + """ + Deleting non-existent documents should not raise an error + and return the list of undeleted ids. + """ + setup_oicd_resource_server(responses, settings, sub="user_sub") + service = factories.ServiceFactory() + # Create index but with no documents + prepare_index(service.index_name, []) + + response = APIClient().post( + "/api/v1.0/documents/delete/", + {"service": service.name, "document_ids": ["non-existent-id"]}, + format="json", + HTTP_AUTHORIZATION=f"Bearer {build_authorization_bearer()}", + ) + + assert response.status_code == 200 + assert response.json()["nb-deleted-documents"] == 0 + assert response.json()["undeleted-document-ids"] == ["non-existent-id"] diff --git a/src/backend/core/tests/test_api_documents_search_access_control.py b/src/backend/core/tests/test_api_documents_search_access_control.py index 840d2c1..6c6ca99 100644 --- a/src/backend/core/tests/test_api_documents_search_access_control.py +++ b/src/backend/core/tests/test_api_documents_search_access_control.py @@ -395,7 +395,7 @@ def test_api_documents_search_access__request_services(settings): ) assert response.status_code == 400 - assert response.json() == {"detail": "Some requested services are not available"} + assert response.json() == {"detail": "Invalid request."} @responses.activate @@ -419,7 +419,7 @@ def test_api_documents_search_access__request_inactive_services(settings): ) assert response.status_code == 400 - assert response.json() == {"detail": "Service is not available"} + assert response.json() == {"detail": "Invalid request."} # Event without explicit argument, the client service from the request is not active response = APIClient().post( @@ -430,7 +430,7 @@ def test_api_documents_search_access__request_inactive_services(settings): ) assert response.status_code == 400 - assert response.json() == {"detail": "Service is not available"} + assert response.json() == {"detail": "Invalid request."} @responses.activate diff --git a/src/backend/core/urls.py b/src/backend/core/urls.py index 9461e6e..62953b2 100644 --- a/src/backend/core/urls.py +++ b/src/backend/core/urls.py @@ -2,10 +2,11 @@ from django.urls import include, path -from .views import IndexDocumentView, SearchDocumentView +from .views import DeleteDocumentsView, IndexDocumentView, SearchDocumentView urlpatterns = [ path("documents/index/", IndexDocumentView.as_view(), name="document"), path("documents/search/", SearchDocumentView.as_view(), name="document"), + path("documents/delete/", DeleteDocumentsView.as_view(), name="document"), path("", include("lasuite.oidc_resource_server.urls")), ] diff --git a/src/backend/core/views.py b/src/backend/core/views.py index bf706b8..719fa65 100644 --- a/src/backend/core/views.py +++ b/src/backend/core/views.py @@ -182,6 +182,89 @@ class IndexDocumentView(views.APIView): return Response(results, status=status.HTTP_201_CREATED) +class DeleteDocumentsView(ResourceServerMixin, views.APIView): + """ + API view for deleting documents from OpenSearch. + - Allows authenticated users to delete documents from a specified index. + - Users can only delete documents where they are listed in the 'users' field. + - Returns the count of deleted documents without revealing document existence. + """ + + authentication_classes = [ResourceServerAuthentication] + permission_classes = [IsAuthAuthenticated] + + def post(self, request, *args, **kwargs): + """ + Handle POST requests to delete documents from the specified index. + + Only documents where the authenticated user is in the 'users' field will be deleted. + + Body Parameters: + --------------- + service: str + service name to determine the index from which to delete documents. + document_ids : List[str] + A list of document IDs to delete from the index. + + Returns: + -------- + Response : rest_framework.response.Response + - 200 OK: Returns the number of documents deleted and list of undeleted document IDs. + - 400 Bad Request: If parameters are invalid or missing. + """ + params = schemas.DeleteDocumentsSchema(**request.data) + try: + index_name = get_opensearch_indices( + self._get_service_provider_audience(), services=[params.service] + )[0] + except SuspiciousOperation as e: + logger.error(e) + return Response( + {"detail": "Invalid request."}, + status=status.HTTP_400_BAD_REQUEST, + ) + + logger.info( + "Deleting %d documents from index %s", len(params.document_ids), index_name + ) + + deletable_matches = opensearch_client().search( + index=index_name, + body={ + "query": { + "bool": { + "must": [ + {"ids": {"values": params.document_ids}}, + {"term": {"users": self.request.user.sub}}, + ] + } + } + }, + ) + deletable_ids = [hit["_id"] for hit in deletable_matches["hits"]["hits"]] + + if deletable_ids: + response = opensearch_client().delete_by_query( + index=index_name, + body={"query": {"ids": {"values": deletable_ids}}}, + ) + nb_deleted = response.get("deleted", 0) + else: + nb_deleted = 0 + + return Response( + { + "nb-deleted-documents": nb_deleted, + "undeleted-document-ids": [ + document_id + for document_id in params.document_ids + if document_id not in deletable_ids + ], + }, + status=status.HTTP_200_OK, + ) + + class SearchDocumentView(ResourceServerMixin, views.APIView): """ API view for searching documents in OpenSearch. @@ -193,27 +276,6 @@ class SearchDocumentView(ResourceServerMixin, views.APIView): authentication_classes = [ResourceServerAuthentication] permission_classes = [IsAuthAuthenticated] - @staticmethod - def _get_opensearch_indices(audience, services): - # Get request user service - try: - user_service = Service.objects.get(client_id=audience, is_active=True) - except Service.DoesNotExist as e: - logger.warning("Login failed: No service %s found", audience) - raise SuspiciousOperation("Service is not available") from e - - # Find allowed sub-services for this service - allowed_services = set(user_service.services.values_list("name", flat=True)) - allowed_services.add(user_service.name) - - if services: - available = set(services).intersection(allowed_services) - - if len(available) < len(services): - raise SuspiciousOperation("Some requested services are not available") - - return [get_opensearch_index_name(name) for name in allowed_services] - def post(self, request, *args, **kwargs): """ Handle POST requests to perform a search on indexed documents with optional filtering @@ -264,11 +326,13 @@ class SearchDocumentView(ResourceServerMixin, views.APIView): # Get index list for search query try: - search_indices = self._get_opensearch_indices( - audience, services=params.services - ) + search_indices = get_opensearch_indices(audience, services=params.services) except SuspiciousOperation as e: - return Response({"detail": str(e)}, status=status.HTTP_400_BAD_REQUEST) + logger.error(e, exc_info=True) + return Response( + {"detail": "Invalid request."}, + status=status.HTTP_400_BAD_REQUEST, + ) logger.info("Search '%s' on indices %s", params.q, search_indices) result = search( @@ -287,3 +351,26 @@ class SearchDocumentView(ResourceServerMixin, views.APIView): logger.debug("results %s", result) return Response(result, status=status.HTTP_200_OK) + + +def get_opensearch_indices(audience, services): + """ + Get OpenSearch indices for the given audience and services. + """ + try: + user_service = Service.objects.get(client_id=audience, is_active=True) + except Service.DoesNotExist as e: + logger.warning("Login failed: No service %s found", audience) + raise SuspiciousOperation("Service is not available") from e + + # Find allowed sub-services for this service + allowed_services = set(user_service.services.values_list("name", flat=True)) + allowed_services.add(user_service.name) + + if services: + available_service = set(services).intersection(allowed_services) + + if len(available_service) < len(services): + raise SuspiciousOperation("Some requested services are not available") + + return [get_opensearch_index_name(service) for service in allowed_services]