From 81d691a02ef8418b2369768a89041a099d2cb30b Mon Sep 17 00:00:00 2001 From: Marc Sommerhalder Date: Wed, 30 Oct 2024 07:26:17 +0100 Subject: [PATCH] PB-1108 Add delete user endpoint --- app/access/api.py | 26 ++++++++++++++++ app/access/tests/test_api.py | 44 ++++++++++++++++++++++++++++ app/cognito/tests/test_user_utils.py | 10 ++++--- app/cognito/utils/user.py | 4 +-- 4 files changed, 78 insertions(+), 6 deletions(-) diff --git a/app/access/api.py b/app/access/api.py index 971fde1..d736bbc 100644 --- a/app/access/api.py +++ b/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) diff --git a/app/access/tests/test_api.py b/app/access/tests/test_api.py index bb7e604..0d71338 100644 --- a/app/access/tests/test_api.py +++ b/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 diff --git a/app/cognito/tests/test_user_utils.py b/app/cognito/tests/test_user_utils.py index 953a616..3b7388a 100644 --- a/app/cognito/tests/test_user_utils.py +++ b/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', 'test@example.org')) + deleted = delete_cognito_user(DummyUser('123', 'test@example.org')) 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', 'test@example.org')) + deleted = delete_cognito_user(DummyUser('123', 'test@example.org')) 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') diff --git a/app/cognito/utils/user.py b/app/cognito/utils/user.py index 69e66a9..fa2ede0 100644 --- a/app/cognito/utils/user.py +++ b/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