Skip to content

Commit

Permalink
[Fixes #11995] Implement the DELETE method for the User API refactor
Browse files Browse the repository at this point in the history
  • Loading branch information
RegisSinjari committed Mar 11, 2024
1 parent 7fa4765 commit dad23ee
Show file tree
Hide file tree
Showing 5 changed files with 44 additions and 17 deletions.
12 changes: 9 additions & 3 deletions geonode/base/api/tests.py
Original file line number Diff line number Diff line change
Expand Up @@ -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="[email protected]", 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()

Expand Down
19 changes: 13 additions & 6 deletions geonode/people/tests.py
Original file line number Diff line number Diff line change
Expand Up @@ -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")
Expand All @@ -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)

Expand All @@ -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())
Expand All @@ -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")
Expand Down Expand Up @@ -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")
Expand All @@ -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])
13 changes: 8 additions & 5 deletions geonode/people/utils.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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
Expand Down Expand Up @@ -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)}")
8 changes: 5 additions & 3 deletions geonode/people/views.py
Original file line number Diff line number Diff line change
Expand Up @@ -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()

Expand Down
9 changes: 9 additions & 0 deletions geonode/settings.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down

0 comments on commit dad23ee

Please sign in to comment.