Fixes #23583: Keep honoring a local return URL ahead of get_return_url() overrides

Restore the delete view's own return_url branch, now validated with safe_for_redirect(). Subclasses which override get_return_url() without reading return_url, such as JournalEntryDeleteView, kept returning the user to the page the delete was started from only because this branch ran first.

netbox/netbox/tests/test_views.py

@@ -15,7 +15,7 @@ from core.models import DataFile, DataSource, ObjectChange, ObjectType
 from dcim.choices import DeviceStatusChoices, InterfaceTypeChoices, SiteStatusChoices
 from dcim.models import Device, DeviceRole, DeviceType, Interface, Manufacturer, Site, VirtualChassis
 from extras.events import enqueue_event
-from extras.models import ImageAttachment
+from extras.models import ImageAttachment, JournalEntry
 from extras.validators import CustomValidator
 from ipam.choices import VLANStatusChoices
 from ipam.models import VLAN, VLANGroup
@@ -661,13 +661,11 @@ class ObjectDeleteReturnURLTestCase(TestCase):
     def setUp(self):
         super().setUp()
         self.add_permissions('dcim.view_site', 'dcim.delete_site')
-        self.site = Site.objects.create(name='Site 1', slug='site-1')
-        self.url = reverse('dcim:site_delete', kwargs={'pk': self.site.pk})
 
     def test_unsafe_return_url_ignored(self):
-        for return_url in ('//example.com', '/\\example.com'):
+        for i, return_url in enumerate(('//example.com', '/\\example.com'), start=1):
             with self.subTest(return_url=return_url):
-                site = Site.objects.create(name=f'Site {return_url}', slug='site-unsafe')
+                site = Site.objects.create(name=f'Site {i}', slug=f'site-{i}')
                 url = reverse('dcim:site_delete', kwargs={'pk': site.pk})
                 response = self.client.post(url, data={'confirm': True, 'return_url': return_url})
                 self.assertHttpStatus(response, 302)
@@ -675,8 +673,24 @@ class ObjectDeleteReturnURLTestCase(TestCase):
                 self.assertFalse(Site.objects.filter(pk=site.pk).exists())
 
     def test_local_return_url_followed(self):
+        site = Site.objects.create(name='Site 1', slug='site-1')
+        url = reverse('dcim:site_delete', kwargs={'pk': site.pk})
         return_url = reverse('dcim:region_list')
-        response = self.client.post(self.url, data={'confirm': True, 'return_url': return_url})
+        response = self.client.post(url, data={'confirm': True, 'return_url': return_url})
+        self.assertHttpStatus(response, 302)
+        self.assertEqual(response['Location'], return_url)
+        self.assertFalse(Site.objects.filter(pk=site.pk).exists())
+
+    def test_local_return_url_precedes_get_return_url_override(self):
+        """
+        A local return URL must take precedence over a view which overrides get_return_url().
+        """
+        self.add_permissions('extras.delete_journalentry')
+        site = Site.objects.create(name='Site 1', slug='site-1')
+        journal_entry = JournalEntry.objects.create(assigned_object=site, created_by=self.user, comments='Entry')
+        url = reverse('extras:journalentry_delete', kwargs={'pk': journal_entry.pk})
+        return_url = reverse('extras:journalentry_list')
+        response = self.client.post(url, data={'confirm': True, 'return_url': return_url})
         self.assertHttpStatus(response, 302)
         self.assertEqual(response['Location'], return_url)
-        self.assertFalse(Site.objects.filter(pk=self.site.pk).exists())
+        self.assertFalse(JournalEntry.objects.filter(pk=journal_entry.pk).exists())

netbox/netbox/views/generic/object_views.py

@@ -510,6 +510,9 @@ class ObjectDeleteView(GetReturnURLMixin, BaseObjectView):
             logger.info(msg)
             messages.success(request, msg)
 
+            return_url = form.cleaned_data.get('return_url')
+            if return_url and safe_for_redirect(return_url):
+                return redirect(return_url)
             return redirect(self.get_return_url(request, obj))
 
         logger.debug("Form validation failed")