From dad23ee2bbcc792d140e2c895973450319baa548 Mon Sep 17 00:00:00 2001 From: RegisSinjari Date: Mon, 11 Mar 2024 12:02:11 +0100 Subject: [PATCH] [Fixes #11995] Implement the DELETE method for the User API refactor --- geonode/base/api/tests.py | 12 +++++++++--- geonode/people/tests.py | 19 +++++++++++++------ geonode/people/utils.py | 13 ++++++++----- geonode/people/views.py | 8 +++++--- geonode/settings.py | 9 +++++++++ 5 files changed, 44 insertions(+), 17 deletions(-) diff --git a/geonode/base/api/tests.py b/geonode/base/api/tests.py index bc9dc434770..88970a91bbc 100644 --- a/geonode/base/api/tests.py +++ b/geonode/base/api/tests.py @@ -465,14 +465,20 @@ def test_delete_user_profile(self): self.assertTrue(self.client.login(username="bobby", password="bob")) response = self.client.delete(url, format="json") self.assertEqual(response.status_code, 403) - # User can not delete self profile + # User can delete self profile self.assertTrue(self.client.login(username="user_test_delete", password="user")) response = self.client.delete(url, format="json") - self.assertEqual(response.status_code, 403) + self.assertEqual(response.status_code, 200) + self.assertEqual(get_user_model().objects.filter(username="user_test_delete").first(), None) + # recreate user that was deleted + user = get_user_model().objects.create_user( + username="user_test_delete", email="user_test_delete@geonode.org", password="user" + ) + url = reverse("users-detail", kwargs={"pk": user.pk}) # Admin can delete user self.assertTrue(self.client.login(username="admin", password="admin")) response = self.client.delete(url, format="json") - self.assertEqual(response.status_code, 204) + self.assertEqual(response.status_code, 200) finally: user.delete() diff --git a/geonode/people/tests.py b/geonode/people/tests.py index 7efd83415c9..dc89fd3c3e1 100644 --- a/geonode/people/tests.py +++ b/geonode/people/tests.py @@ -790,7 +790,9 @@ def test_users_api_patch_username(self): self.assertEqual(response.status_code, 400) self.assertTrue("username cannot be updated" in response.json()["errors"]) - @override_settings(USER_DELETION_RULES=["geonode.people.utils.has_resources", "geonode.people.utils.is_manager"]) + @override_settings( + USER_DELETION_RULES=["geonode.people.utils.user_has_resources", "geonode.people.utils.user_is_manager"] + ) def test_valid_delete(self): # create a new user tim = get_user_model().objects.create(username="tim") @@ -811,7 +813,7 @@ def test_valid_delete(self): response = self.client.delete(url, content_type="application/json") # admin is permitted to delete - self.assertEqual(response.status_code, 204) + self.assertEqual(response.status_code, 200) # tim has been deleted self.assertEqual(get_user_model().objects.filter(username="tim").first(), None) @@ -837,7 +839,7 @@ def test_delete_without_validators(self): # Ensure norman is now a member self.assertTrue(self.bar.user_is_member(norman)) - # p romote norman to a manager + # promote norman to a manager self.bar.promote(norman) # Ensure norman is in the managers queryset self.assertTrue(norman in self.bar.get_managers()) @@ -846,10 +848,12 @@ def test_delete_without_validators(self): response = self.client.delete(url, content_type="application/json") # norman can be deleted because validator rules are not applied - self.assertEqual(response.status_code, 204) + self.assertEqual(response.status_code, 200) self.assertEqual(get_user_model().objects.filter(username="norman").first(), None) - @override_settings(USER_DELETION_RULES=["geonode.people.utils.has_resources", "geonode.people.utils.is_manager"]) + @override_settings( + USER_DELETION_RULES=["geonode.people.utils.user_has_resources", "geonode.people.utils.user_is_manager"] + ) def test_delete_a_manger(self): norman = get_user_model().objects.get(username="norman") admin = get_user_model().objects.get(username="admin") @@ -880,8 +884,10 @@ def test_delete_a_manger(self): # norman cant be deleted self.assertEqual(response.status_code, 403) self.assertNotEqual(get_user_model().objects.filter(username="norman").first(), None) + # + self.assertTrue("user_is_manager" in response.json()["errors"][0]) - @override_settings(USER_DELETION_RULES=["geonode.people.utils.has_resources", "geonode.people.utils.is_manager"]) + @override_settings(USER_DELETION_RULES=["geonode.people.utils.user_has_resources"]) def test_delete_a_user_with_resource(self): # create a new user bobby = get_user_model().objects.get(username="bobby") @@ -904,3 +910,4 @@ def test_delete_a_user_with_resource(self): self.assertEqual(response.status_code, 403) # bobby cant be deleted self.assertNotEqual(get_user_model().objects.filter(username="bobby").first(), None) + self.assertTrue("user_has_resources" in response.json()["errors"][0]) diff --git a/geonode/people/utils.py b/geonode/people/utils.py index 09b6c0be55e..9df70ff0de6 100644 --- a/geonode/people/utils.py +++ b/geonode/people/utils.py @@ -148,7 +148,7 @@ def get_available_users(user): return get_user_model().objects.filter(id__in=member_ids) -def has_resources(profile) -> bool: +def user_has_resources(profile) -> bool: """ checks if user has any resource in ownership @@ -161,7 +161,7 @@ def has_resources(profile) -> bool: return ResourceBase.objects.filter(owner_id=profile.pk).exists() -def is_manager(profile) -> bool: +def user_is_manager(profile) -> bool: """ Checks if user is the manager of any group @@ -190,6 +190,9 @@ def call_user_deletion_rules(profile) -> None: if not globals().get("user_deletion_modules"): rule_path = settings.USER_DELETION_RULES if hasattr(settings, "USER_DELETION_RULES") else [] globals()["user_deletion_modules"] = [import_string(deletion_rule) for deletion_rule in rule_path] - for not_valid in globals().get("user_deletion_modules", []): - if not_valid(profile): - raise PermissionDenied() + error_list = [] + for not_deletable in globals().get("user_deletion_modules", []): + if not_deletable(profile): + error_list.append(not_deletable.__name__) + if error_list: + raise PermissionDenied(f"Deletion rule Violated: {', '.join(error_list)}") diff --git a/geonode/people/views.py b/geonode/people/views.py index 5ca39c4f1d5..184f6a28b60 100644 --- a/geonode/people/views.py +++ b/geonode/people/views.py @@ -205,10 +205,12 @@ def update(self, request, *args, **kwargs): serializer.save() return Response(serializer.data) + def destroy(self, request, *args, **kwargs): + instance = self.get_object() + self.perform_destroy(instance) + return Response("User deleted sucessfully", status=200) + def perform_destroy(self, instance): - # self delete check - if self.request.user.pk == int(self.kwargs["pk"]): - raise PermissionDenied() call_user_deletion_rules(instance) instance.delete() diff --git a/geonode/settings.py b/geonode/settings.py index bd597c2ac93..b0bbe92c19e 100644 --- a/geonode/settings.py +++ b/geonode/settings.py @@ -2220,6 +2220,15 @@ def get_geonode_catalogue_service(): ] +""" +List of modules that implement the deletion rules for a user +""" +USER_DELETION_RULES = [ + "geonode.people.utils.user_has_resources" + # ,"geonode.people.utils.user_is_manager" +] + + """ Define the URLs patterns used by the SizeRestrictedFileUploadHandler to evaluate if the file is greater than the limit size defined