Merge pull request #34 from geoadmin/feat-pb-1108-delete-user

Add DELETE endpoint for users

app/access/api.py

@@ -1,10 +1,13 @@
 from cognito.utils.user import create_cognito_user
+from cognito.utils.user import delete_cognito_user
 from ninja import Router
 from ninja.errors import HttpError
 from provider.models import Provider
 
 from django.db import transaction
+from django.http import Http404
 from django.http import HttpRequest
+from django.http import HttpResponse
 from django.shortcuts import get_object_or_404
 
 from .models import User
@@ -75,3 +78,26 @@ def create(request: HttpRequest, user_in: UserSchema) -> UserSchema:
         if not created:
             raise HttpError(500, "Internal Server Error")
     return user_to_response(user_out)
+
+
+@router.delete("users/{username}")
+def delete(request: HttpRequest, username: str) -> HttpResponse:
+    """
+    Delete the user with the given username.
+
+    Return HTTP status code
+    - 204 (No Content) if the user has been deleted
+    - 404 (Not Found) if there is no user with the given username
+    - 500 (Internal Server Error) if there is inconsistency with cognito
+    - 503 (Service Unavailable) if cognito cannot be reached
+    """
+
+    with transaction.atomic():
+        user_to_delete = User.objects.select_for_update().filter(username=username).first()
+        if not user_to_delete:
+            raise Http404("Not Found")
+        deleted = delete_cognito_user(user_to_delete)
+        if not deleted:
+            raise HttpError(500, "Internal Server Error")
+        user_to_delete.delete()
+        return HttpResponse(status=204)

app/access/tests/test_api.py

@@ -249,3 +249,47 @@ def test_post_users_returns_503_if_cognito_down(self, create_cognito_user):
         assert response.data == {'code': 503, 'description': 'Service Unavailable'}
         assert User.objects.count() == 1
         assert create_cognito_user.called
+
+    @patch('access.api.delete_cognito_user')
+    def test_delete_user_deletes_user(self, delete_cognito_user):
+        delete_cognito_user.return_value = True
+
+        response = self.client.delete("users/dude")
+
+        assert response.status_code == 204
+        assert response.content == b''
+        assert User.objects.count() == 0
+        assert delete_cognito_user.called
+
+    @patch('access.api.delete_cognito_user')
+    def test_delete_user_returns_404_if_nonexisting(self, delete_cognito_user):
+        delete_cognito_user.return_value = False
+
+        response = self.client.delete("users/lebowski")
+
+        assert response.status_code == 404
+        assert response.data == {"code": 404, "description": "Resource not found"}
+        assert User.objects.count() == 1
+        assert not delete_cognito_user.called
+
+    @patch('access.api.delete_cognito_user')
+    def test_delete_user_returns_500_if_cognito_inconsistent(self, delete_cognito_user):
+        delete_cognito_user.return_value = False
+
+        response = self.client.delete("users/dude")
+
+        assert response.status_code == 500
+        assert response.data == {"code": 500, "description": "Internal Server Error"}
+        assert User.objects.count() == 1
+        assert delete_cognito_user.called
+
+    @patch('access.api.delete_cognito_user')
+    def test_delete_user_returns_503_if_cognito_down(self, delete_cognito_user):
+        delete_cognito_user.side_effect = EndpointConnectionError(endpoint_url='http://localhost')
+
+        response = self.client.delete("users/dude")
+
+        assert response.status_code == 503
+        assert response.data == {"code": 503, "description": "Service Unavailable"}
+        assert User.objects.count() == 1
+        assert delete_cognito_user.called

app/cognito/tests/test_user_utils.py

@@ -2,7 +2,7 @@
 from unittest.mock import patch
 
 from cognito.utils.user import create_cognito_user
-from cognito.utils.user import delete_user
+from cognito.utils.user import delete_cognito_user
 from cognito.utils.user import update_user
 
 from django.test import TestCase
@@ -44,7 +44,7 @@ def test_create_user_does_not_create_existing_user(self, logger, client):
     def test_delete_user_deletes_user(self, logger, client):
         client.return_value.delete_user.return_value = True
 
-        deleted = delete_user(DummyUser('123', '[email protected]'))
+        deleted = delete_cognito_user(DummyUser('123', '[email protected]'))
 
         self.assertEqual(deleted, True)
         self.assertIn(call.info('User %s deleted', '123'), logger.mock_calls)
@@ -54,10 +54,12 @@ def test_delete_user_deletes_user(self, logger, client):
     def test_delete_user_does_not_delete_nonexisting_user(self, logger, client):
         client.return_value.delete_user.return_value = False
 
-        deleted = delete_user(DummyUser('123', '[email protected]'))
+        deleted = delete_cognito_user(DummyUser('123', '[email protected]'))
 
         self.assertEqual(deleted, False)
-        self.assertIn(call.warning('User %s does not exist, not deleted', '123'), logger.mock_calls)
+        self.assertIn(
+            call.critical('User %s does not exist, not deleted', '123'), logger.mock_calls
+        )
 
     @patch('cognito.utils.user.Client')
     @patch('cognito.utils.user.logger')

app/cognito/utils/user.py

@@ -22,7 +22,7 @@ def create_cognito_user(user: User) -> bool:
     return created
 
 
-def delete_user(user: User) -> bool:
+def delete_cognito_user(user: User) -> bool:
     """ Delete the given user from cognito.
 
     Returns True, if the user has been deleted.
@@ -33,7 +33,7 @@ def delete_user(user: User) -> bool:
     if deleted:
         logger.info("User %s deleted", user.username)
     else:
-        logger.warning("User %s does not exist, not deleted", user.username)
+        logger.critical("User %s does not exist, not deleted", user.username)
     return deleted