From cc87d9901785f252d011f486eb95ecf74898dc1a Mon Sep 17 00:00:00 2001 From: hellerve Date: Tue, 28 May 2019 16:11:49 +0200 Subject: [PATCH 1/2] all: fix error message on trying to delete protected models (references #3211) --- netbox/ipam/tests/test_api.py | 18 ++++++++++++++++++ netbox/utilities/middleware.py | 9 +++++++-- 2 files changed, 25 insertions(+), 2 deletions(-) diff --git a/netbox/ipam/tests/test_api.py b/netbox/ipam/tests/test_api.py index 3f4555b55..991f098bb 100644 --- a/netbox/ipam/tests/test_api.py +++ b/netbox/ipam/tests/test_api.py @@ -1,3 +1,5 @@ +import json + from django.urls import reverse from netaddr import IPNetwork from rest_framework import status @@ -870,6 +872,8 @@ class VLANTest(APITestCase): self.vlan2 = VLAN.objects.create(vid=2, name='Test VLAN 2') self.vlan3 = VLAN.objects.create(vid=3, name='Test VLAN 3') + self.prefix1 = Prefix.objects.create(prefix=IPNetwork('192.168.1.0/24')) + def test_get_vlan(self): url = reverse('ipam-api:vlan-detail', kwargs={'pk': self.vlan1.pk}) @@ -960,6 +964,20 @@ class VLANTest(APITestCase): self.assertHttpStatus(response, status.HTTP_204_NO_CONTENT) self.assertEqual(VLAN.objects.count(), 2) + def test_delete_vlan_with_prefix(self): + self.prefix1.vlan = self.vlan1 + self.prefix1.save() + + url = reverse('ipam-api:vlan-detail', kwargs={'pk': self.vlan1.pk}) + response = self.client.delete(url, **self.header) + + # can't use assertHttpStatus here because we don't have response.data + self.assertEqual(response.status_code, 403) + + content = json.loads(response.content.decode('utf-8')) + self.assertIn('detail', content) + self.assertTrue(content['detail'].startswith('You tried deleting a model that is protected by:')) + class ServiceTest(APITestCase): diff --git a/netbox/utilities/middleware.py b/netbox/utilities/middleware.py index 4e321ab19..6be01127c 100644 --- a/netbox/utilities/middleware.py +++ b/netbox/utilities/middleware.py @@ -1,6 +1,7 @@ from django.conf import settings from django.db import ProgrammingError -from django.http import Http404, HttpResponseRedirect +from django.http import Http404, HttpResponseRedirect, JsonResponse +from django.db.models import ProtectedError from django.urls import reverse from .views import server_error @@ -62,6 +63,11 @@ class ExceptionHandlingMiddleware(object): if isinstance(exception, Http404): return + elif isinstance(exception, ProtectedError): + models = '\n'.join('- {} ({})'.format(o, o._meta) for o in exception.protected_objects.all()) + msg = 'You tried deleting a model that is protected by:\n{}'.format(models) + return JsonResponse({'detail': msg}, status=403) + # Determine the type of exception. If it's a common issue, return a custom error page with instructions. custom_template = None if isinstance(exception, ProgrammingError): @@ -70,7 +76,6 @@ class ExceptionHandlingMiddleware(object): custom_template = 'exceptions/import_error.html' elif isinstance(exception, PermissionError): custom_template = 'exceptions/permission_error.html' - # Return a custom error message, or fall back to Django's default 500 error handling if custom_template: return server_error(request, template_name=custom_template) From 2c7bad9fff4b4741f69c68586dbb624757221805 Mon Sep 17 00:00:00 2001 From: hellerve Date: Tue, 28 May 2019 21:11:23 +0200 Subject: [PATCH 2/2] utilities: move protectederror handling to modelviewset --- netbox/ipam/tests/test_api.py | 2 +- netbox/utilities/api.py | 18 +++++++++++++++++- netbox/utilities/middleware.py | 8 +------- 3 files changed, 19 insertions(+), 9 deletions(-) diff --git a/netbox/ipam/tests/test_api.py b/netbox/ipam/tests/test_api.py index 991f098bb..0dad73550 100644 --- a/netbox/ipam/tests/test_api.py +++ b/netbox/ipam/tests/test_api.py @@ -972,7 +972,7 @@ class VLANTest(APITestCase): response = self.client.delete(url, **self.header) # can't use assertHttpStatus here because we don't have response.data - self.assertEqual(response.status_code, 403) + self.assertEqual(response.status_code, 409) content = json.loads(response.content.decode('utf-8')) self.assertIn('detail', content) diff --git a/netbox/utilities/api.py b/netbox/utilities/api.py index fbebd09ff..9960fbe42 100644 --- a/netbox/utilities/api.py +++ b/netbox/utilities/api.py @@ -4,7 +4,7 @@ import pytz from django.conf import settings from django.contrib.contenttypes.models import ContentType from django.core.exceptions import ObjectDoesNotExist -from django.db.models import ManyToManyField +from django.db.models import ManyToManyField, ProtectedError from django.http import Http404 from rest_framework.exceptions import APIException from rest_framework.permissions import BasePermission @@ -248,6 +248,22 @@ class ModelViewSet(_ModelViewSet): # Fall back to the hard-coded serializer class return self.serializer_class + def dispatch(self, request, *args, **kwargs): + try: + return super().dispatch(request, *args, **kwargs) + except ProtectedError as e: + models = '\n'.join( + '- {} ({})'.format(o, o._meta) + for o in e.protected_objects.all() + ) + msg = 'You tried deleting a model that is protected by:\n{}'.format(models) + return self.finalize_response( + request, + Response({'detail': msg}, status=409), + *args, + **kwargs + ) + class FieldChoicesViewSet(ViewSet): """ diff --git a/netbox/utilities/middleware.py b/netbox/utilities/middleware.py index 6be01127c..b2065543a 100644 --- a/netbox/utilities/middleware.py +++ b/netbox/utilities/middleware.py @@ -1,7 +1,6 @@ from django.conf import settings from django.db import ProgrammingError -from django.http import Http404, HttpResponseRedirect, JsonResponse -from django.db.models import ProtectedError +from django.http import Http404, HttpResponseRedirect from django.urls import reverse from .views import server_error @@ -63,11 +62,6 @@ class ExceptionHandlingMiddleware(object): if isinstance(exception, Http404): return - elif isinstance(exception, ProtectedError): - models = '\n'.join('- {} ({})'.format(o, o._meta) for o in exception.protected_objects.all()) - msg = 'You tried deleting a model that is protected by:\n{}'.format(models) - return JsonResponse({'detail': msg}, status=403) - # Determine the type of exception. If it's a common issue, return a custom error page with instructions. custom_template = None if isinstance(exception, ProgrammingError):