diff --git a/openwisp_utils/admin.py b/openwisp_utils/admin.py index bfaba7e2..7af390c4 100644 --- a/openwisp_utils/admin.py +++ b/openwisp_utils/admin.py @@ -34,7 +34,16 @@ def has_add_permission(self, request): return False def has_delete_permission(self, request, obj=None): - return False + opts = self.model._meta + resolver_match = getattr(request, "resolver_match", None) + url_name = getattr(resolver_match, "url_name", None) + if url_name and url_name in ( + f"{opts.app_label}_{opts.model_name}_delete", + f"{opts.app_label}_{opts.model_name}_change", + f"{opts.app_label}_{opts.model_name}_changelist", + ): + return False + return super().has_delete_permission(request, obj) def save_model(self, request, obj, form, change): # pragma: nocover pass diff --git a/tests/test_project/tests/test_admin.py b/tests/test_project/tests/test_admin.py index 73ccf22f..3ac86d27 100644 --- a/tests/test_project/tests/test_admin.py +++ b/tests/test_project/tests/test_admin.py @@ -1,8 +1,7 @@ from unittest.mock import MagicMock, patch from django.contrib.admin.sites import AdminSite -from django.contrib.auth import get_user_model -from django.contrib.auth.models import Permission +from django.contrib.auth.models import Permission, User from django.core.exceptions import ImproperlyConfigured from django.test import TestCase from django.urls import reverse @@ -22,8 +21,6 @@ ) from . import AdminTestMixin, CreateMixin -User = get_user_model() - class TestAdmin(AdminTestMixin, CreateMixin, TestCase): TEST_KEY = "w1gwJxKaHcamUw62TQIPgYchwLKn3AA0" @@ -119,6 +116,62 @@ class TestReadOnlyAdmin(ReadOnlyAdmin): ["id", "session_id", "username", "start_time", "stop_time"], ) + def test_readonlyadmin_has_delete_permission(self): + modeladmin = ReadOnlyAdmin(RadiusAccounting, AdminSite()) + + with self.subTest("changelist URL returns False"): + request = self.client.get( + reverse("admin:test_project_radiusaccounting_changelist") + ).wsgi_request + self.assertFalse(modeladmin.has_delete_permission(request)) + + with self.subTest("change URL returns False"): + obj = self._create_radius_accounting(username="test", session_id="1") + request = self.client.get( + reverse("admin:test_project_radiusaccounting_change", args=[obj.pk]) + ).wsgi_request + self.assertFalse(modeladmin.has_delete_permission(request)) + + with self.subTest("delete URL returns False"): + obj = self._create_radius_accounting(username="delete-test", session_id="2") + request = self.client.get( + reverse("admin:test_project_radiusaccounting_delete", args=[obj.pk]) + ).wsgi_request + self.assertFalse(modeladmin.has_delete_permission(request)) + + with self.subTest("cascade delete from unrelated URL returns True"): + # Simulate being called from a parent model's delete + # confirmation (cascade), not from the model's own views. + request = self.client.get( + reverse("admin:test_project_radiusaccounting_changelist") + ).wsgi_request + mock_resolver = MagicMock() + mock_resolver.url_name = "index" + request.resolver_match = mock_resolver + self.assertTrue(modeladmin.has_delete_permission(request)) + + with self.subTest("no resolver_match returns True"): + request = self.client.get( + reverse("admin:test_project_radiusaccounting_changelist") + ).wsgi_request + request.resolver_match = None + self.assertTrue(modeladmin.has_delete_permission(request)) + + with self.subTest("cascade delete without child permission returns False"): + user = User.objects.create( + username="readonly-staff", + password="pass", + is_staff=True, + is_superuser=False, + ) + self.client.force_login(user) + request = self.client.get(reverse("admin:index")).wsgi_request + + mock_resolver = MagicMock() + mock_resolver.url_name = "index" + request.resolver_match = mock_resolver + self.assertFalse(modeladmin.has_delete_permission(request)) + def test_context_processor(self): url = reverse("admin:index") response = self.client.get(url)