From c640a9820f57981c4f476f331cb4d0c5d2a12043 Mon Sep 17 00:00:00 2001 From: UltraBot05 <170253496+UltraBot05@users.noreply.github.com> Date: Tue, 24 Feb 2026 09:10:39 +0000 Subject: [PATCH 1/2] [fix] Allow cascade deletions in ReadOnlyAdmin ReadOnlyAdmin.has_delete_permission previously returned False unconditionally, which blocked cascade deletions from parent models (like Organization). Updated the method to check request.resolver_match.url_name. It now correctly blocks direct deletes on the model's own views while delegating to the standard Django permission check for cascade deletes. --- openwisp_utils/admin.py | 11 ++++++++- tests/test_project/tests/test_admin.py | 34 ++++++++++++++++++++++++++ 2 files changed, 44 insertions(+), 1 deletion(-) 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..f2155edf 100644 --- a/tests/test_project/tests/test_admin.py +++ b/tests/test_project/tests/test_admin.py @@ -119,6 +119,40 @@ 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("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)) + def test_context_processor(self): url = reverse("admin:index") response = self.client.get(url) From de1376f9d99ac13527d4bb35aefcb16316c63a2f Mon Sep 17 00:00:00 2001 From: UltraBot05 <170253496+UltraBot05@users.noreply.github.com> Date: Sun, 8 Mar 2026 16:47:40 +0000 Subject: [PATCH 2/2] [fix] Added negative case for cascade delete permission #256 Added a subTest to verify that cascade deletes still respect child-model permissions. Fixes #256 --- tests/test_project/tests/test_admin.py | 27 ++++++++++++++++++++++---- 1 file changed, 23 insertions(+), 4 deletions(-) diff --git a/tests/test_project/tests/test_admin.py b/tests/test_project/tests/test_admin.py index f2155edf..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" @@ -135,6 +132,13 @@ def test_readonlyadmin_has_delete_permission(self): ).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. @@ -153,6 +157,21 @@ def test_readonlyadmin_has_delete_permission(self): 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)