diff --git a/docs/release-notes/version-2.7.md b/docs/release-notes/version-2.7.md index f7460d92e..b34731403 100644 --- a/docs/release-notes/version-2.7.md +++ b/docs/release-notes/version-2.7.md @@ -9,6 +9,7 @@ ### Bug Fixes +* [#2769](https://github.com/netbox-community/netbox/issues/2769) - Improve `prefix_length` validation on available-prefixes API * [#4340](https://github.com/netbox-community/netbox/issues/4340) - Enforce unique constraints for device and virtual machine names in the API * [#4343](https://github.com/netbox-community/netbox/issues/4343) - Fix Markdown support for tables * [#4365](https://github.com/netbox-community/netbox/issues/4365) - Fix exception raised on IP address bulk add view diff --git a/netbox/extras/api/serializers.py b/netbox/extras/api/serializers.py index 8bde17912..54c0650da 100644 --- a/netbox/extras/api/serializers.py +++ b/netbox/extras/api/serializers.py @@ -13,7 +13,7 @@ from extras.constants import * from extras.models import ( ConfigContext, ExportTemplate, Graph, ImageAttachment, ObjectChange, ReportResult, Tag, ) -from extras.utils import FeatureQuerySet +from extras.utils import FeatureQuery from tenancy.api.nested_serializers import NestedTenantSerializer, NestedTenantGroupSerializer from tenancy.models import Tenant, TenantGroup from users.api.nested_serializers import NestedUserSerializer @@ -32,7 +32,7 @@ from .nested_serializers import * class GraphSerializer(ValidatedModelSerializer): type = ContentTypeField( - queryset=ContentType.objects.filter(FeatureQuerySet('graphs').get_queryset()), + queryset=ContentType.objects.filter(FeatureQuery('graphs').get_query()), ) class Meta: @@ -68,7 +68,7 @@ class RenderedGraphSerializer(serializers.ModelSerializer): class ExportTemplateSerializer(ValidatedModelSerializer): content_type = ContentTypeField( - queryset=ContentType.objects.filter(FeatureQuerySet('export_templates').get_queryset()), + queryset=ContentType.objects.filter(FeatureQuery('export_templates').get_query()), ) template_language = ChoiceField( choices=TemplateLanguageChoices, diff --git a/netbox/extras/migrations/0039_update_features_content_types.py b/netbox/extras/migrations/0039_update_features_content_types.py index c347b1198..8747fcc16 100644 --- a/netbox/extras/migrations/0039_update_features_content_types.py +++ b/netbox/extras/migrations/0039_update_features_content_types.py @@ -15,26 +15,26 @@ class Migration(migrations.Migration): migrations.AlterField( model_name='customfield', name='obj_type', - field=models.ManyToManyField(limit_choices_to=extras.utils.FeatureQuerySet('custom_fields'), related_name='custom_fields', to='contenttypes.ContentType'), + field=models.ManyToManyField(limit_choices_to=extras.utils.FeatureQuery('custom_fields'), related_name='custom_fields', to='contenttypes.ContentType'), ), migrations.AlterField( model_name='customlink', name='content_type', - field=models.ForeignKey(limit_choices_to=extras.utils.FeatureQuerySet('custom_links'), on_delete=django.db.models.deletion.CASCADE, to='contenttypes.ContentType'), + field=models.ForeignKey(limit_choices_to=extras.utils.FeatureQuery('custom_links'), on_delete=django.db.models.deletion.CASCADE, to='contenttypes.ContentType'), ), migrations.AlterField( model_name='exporttemplate', name='content_type', - field=models.ForeignKey(limit_choices_to=extras.utils.FeatureQuerySet('export_templates'), on_delete=django.db.models.deletion.CASCADE, to='contenttypes.ContentType'), + field=models.ForeignKey(limit_choices_to=extras.utils.FeatureQuery('export_templates'), on_delete=django.db.models.deletion.CASCADE, to='contenttypes.ContentType'), ), migrations.AlterField( model_name='graph', name='type', - field=models.ForeignKey(limit_choices_to=extras.utils.FeatureQuerySet('graphs'), on_delete=django.db.models.deletion.CASCADE, to='contenttypes.ContentType'), + field=models.ForeignKey(limit_choices_to=extras.utils.FeatureQuery('graphs'), on_delete=django.db.models.deletion.CASCADE, to='contenttypes.ContentType'), ), migrations.AlterField( model_name='webhook', name='obj_type', - field=models.ManyToManyField(limit_choices_to=extras.utils.FeatureQuerySet('webhooks'), related_name='webhooks', to='contenttypes.ContentType'), + field=models.ManyToManyField(limit_choices_to=extras.utils.FeatureQuery('webhooks'), related_name='webhooks', to='contenttypes.ContentType'), ), ] diff --git a/netbox/extras/models.py b/netbox/extras/models.py index c8ca1544f..488554596 100644 --- a/netbox/extras/models.py +++ b/netbox/extras/models.py @@ -22,7 +22,7 @@ from utilities.utils import deepmerge, render_jinja2 from .choices import * from .constants import * from .querysets import ConfigContextQuerySet -from .utils import FeatureQuerySet +from .utils import FeatureQuery __all__ = ( @@ -59,7 +59,7 @@ class Webhook(models.Model): to=ContentType, related_name='webhooks', verbose_name='Object types', - limit_choices_to=FeatureQuerySet('webhooks'), + limit_choices_to=FeatureQuery('webhooks'), help_text="The object(s) to which this Webhook applies." ) name = models.CharField( @@ -224,7 +224,7 @@ class CustomField(models.Model): to=ContentType, related_name='custom_fields', verbose_name='Object(s)', - limit_choices_to=FeatureQuerySet('custom_fields'), + limit_choices_to=FeatureQuery('custom_fields'), help_text='The object(s) to which this field applies.' ) type = models.CharField( @@ -471,7 +471,7 @@ class CustomLink(models.Model): content_type = models.ForeignKey( to=ContentType, on_delete=models.CASCADE, - limit_choices_to=FeatureQuerySet('custom_links') + limit_choices_to=FeatureQuery('custom_links') ) name = models.CharField( max_length=100, @@ -519,7 +519,7 @@ class Graph(models.Model): type = models.ForeignKey( to=ContentType, on_delete=models.CASCADE, - limit_choices_to=FeatureQuerySet('graphs') + limit_choices_to=FeatureQuery('graphs') ) weight = models.PositiveSmallIntegerField( default=1000 @@ -580,7 +580,7 @@ class ExportTemplate(models.Model): content_type = models.ForeignKey( to=ContentType, on_delete=models.CASCADE, - limit_choices_to=FeatureQuerySet('export_templates') + limit_choices_to=FeatureQuery('export_templates') ) name = models.CharField( max_length=100 diff --git a/netbox/extras/tests/test_api.py b/netbox/extras/tests/test_api.py index 0dbd32876..3ab9eaaf0 100644 --- a/netbox/extras/tests/test_api.py +++ b/netbox/extras/tests/test_api.py @@ -9,7 +9,7 @@ from dcim.models import Device, DeviceRole, DeviceType, Manufacturer, Platform, from extras.api.views import ScriptViewSet from extras.models import ConfigContext, Graph, ExportTemplate, Tag from extras.scripts import BooleanVar, IntegerVar, Script, StringVar -from extras.utils import FeatureQuerySet +from extras.utils import FeatureQuery from tenancy.models import Tenant, TenantGroup from utilities.testing import APITestCase diff --git a/netbox/extras/tests/test_filters.py b/netbox/extras/tests/test_filters.py index 687470bf3..25a839c39 100644 --- a/netbox/extras/tests/test_filters.py +++ b/netbox/extras/tests/test_filters.py @@ -4,7 +4,7 @@ from django.test import TestCase from dcim.models import DeviceRole, Platform, Region, Site from extras.choices import * from extras.filters import * -from extras.utils import FeatureQuerySet +from extras.utils import FeatureQuery from extras.models import ConfigContext, ExportTemplate, Graph from tenancy.models import Tenant, TenantGroup from virtualization.models import Cluster, ClusterGroup, ClusterType @@ -18,7 +18,7 @@ class GraphTestCase(TestCase): def setUpTestData(cls): # Get the first three available types - content_types = ContentType.objects.filter(FeatureQuerySet('graphs').get_queryset())[:3] + content_types = ContentType.objects.filter(FeatureQuery('graphs').get_query())[:3] graphs = ( Graph(name='Graph 1', type=content_types[0], template_language=TemplateLanguageChoices.LANGUAGE_DJANGO, source='http://example.com/1'), @@ -32,7 +32,7 @@ class GraphTestCase(TestCase): self.assertEqual(self.filterset(params, self.queryset).qs.count(), 2) def test_type(self): - content_type = ContentType.objects.filter(FeatureQuerySet('graphs').get_queryset()).first() + content_type = ContentType.objects.filter(FeatureQuery('graphs').get_query()).first() params = {'type': content_type.pk} self.assertEqual(self.filterset(params, self.queryset).qs.count(), 1) diff --git a/netbox/extras/utils.py b/netbox/extras/utils.py index 5edf3f562..5ae16bff9 100644 --- a/netbox/extras/utils.py +++ b/netbox/extras/utils.py @@ -42,7 +42,7 @@ registry = Registry() @deconstructible -class FeatureQuerySet: +class FeatureQuery: """ Helper class that delays evaluation of the registry contents for the functionaility store until it has been populated. @@ -52,9 +52,9 @@ class FeatureQuerySet: self.feature = feature def __call__(self): - return self.get_queryset() + return self.get_query() - def get_queryset(self): + def get_query(self): """ Given an extras feature, return a Q object for content type lookup """ diff --git a/netbox/extras/webhooks.py b/netbox/extras/webhooks.py index f1a3391a0..d1d5a59ab 100644 --- a/netbox/extras/webhooks.py +++ b/netbox/extras/webhooks.py @@ -8,7 +8,7 @@ from extras.models import Webhook from utilities.api import get_serializer_for_model from .choices import * from .constants import * -from .utils import FeatureQuerySet +from .utils import FeatureQuery def generate_signature(request_body, secret): @@ -30,7 +30,7 @@ def enqueue_webhooks(instance, user, request_id, action): """ obj_type = ContentType.objects.get_for_model(instance.__class__) - webhook_models = ContentType.objects.filter(FeatureQuerySet('webhooks').get_queryset()) + webhook_models = ContentType.objects.filter(FeatureQuery('webhooks').get_query()) if obj_type not in webhook_models: return diff --git a/netbox/ipam/api/serializers.py b/netbox/ipam/api/serializers.py index d654ecf70..5abe4c585 100644 --- a/netbox/ipam/api/serializers.py +++ b/netbox/ipam/api/serializers.py @@ -154,6 +154,33 @@ class PrefixSerializer(TaggitSerializer, CustomFieldModelSerializer): read_only_fields = ['family'] +class PrefixLengthSerializer(serializers.Serializer): + + prefix_length = serializers.IntegerField() + + def to_internal_value(self, data): + requested_prefix = data.get('prefix_length') + if requested_prefix is None: + raise serializers.ValidationError({ + 'prefix_length': 'this field can not be missing' + }) + if not isinstance(requested_prefix, int): + raise serializers.ValidationError({ + 'prefix_length': 'this field must be int type' + }) + + prefix = self.context.get('prefix') + if prefix.family == 4 and requested_prefix > 32: + raise serializers.ValidationError({ + 'prefix_length': 'Invalid prefix length ({}) for IPv4'.format((requested_prefix)) + }) + elif prefix.family == 6 and requested_prefix > 128: + raise serializers.ValidationError({ + 'prefix_length': 'Invalid prefix length ({}) for IPv6'.format((requested_prefix)) + }) + return data + + class AvailablePrefixSerializer(serializers.Serializer): """ Representation of a prefix which does not exist in the database. diff --git a/netbox/ipam/api/views.py b/netbox/ipam/api/views.py index 4b50ac7de..f24c71b17 100644 --- a/netbox/ipam/api/views.py +++ b/netbox/ipam/api/views.py @@ -91,45 +91,25 @@ class PrefixViewSet(CustomFieldModelViewSet): if not request.user.has_perm('ipam.add_prefix'): raise PermissionDenied() - # Normalize to a list of objects - requested_prefixes = request.data if isinstance(request.data, list) else [request.data] + # Validate Requested Prefixes' length + serializer = serializers.PrefixLengthSerializer( + data=request.data if isinstance(request.data, list) else [request.data], + many=True, + context={ + 'request': request, + 'prefix': prefix, + } + ) + if not serializer.is_valid(): + return Response( + serializer.errors, + status=status.HTTP_400_BAD_REQUEST + ) + requested_prefixes = serializer.validated_data # Allocate prefixes to the requested objects based on availability within the parent for i, requested_prefix in enumerate(requested_prefixes): - # Validate requested prefix size - prefix_length = requested_prefix.get('prefix_length') - if prefix_length is None: - return Response( - { - "detail": "Item {}: prefix_length field missing".format(i) - }, - status=status.HTTP_400_BAD_REQUEST - ) - try: - prefix_length = int(prefix_length) - except ValueError: - return Response( - { - "detail": "Item {}: Invalid prefix length ({})".format(i, prefix_length), - }, - status=status.HTTP_400_BAD_REQUEST - ) - if prefix.family == 4 and prefix_length > 32: - return Response( - { - "detail": "Item {}: Invalid prefix length ({}) for IPv4".format(i, prefix_length), - }, - status=status.HTTP_400_BAD_REQUEST - ) - elif prefix.family == 6 and prefix_length > 128: - return Response( - { - "detail": "Item {}: Invalid prefix length ({}) for IPv6".format(i, prefix_length), - }, - status=status.HTTP_400_BAD_REQUEST - ) - # Find the first available prefix equal to or larger than the requested size for available_prefix in available_prefixes.iter_cidrs(): if requested_prefix['prefix_length'] >= available_prefix.prefixlen: diff --git a/netbox/ipam/tests/test_api.py b/netbox/ipam/tests/test_api.py index 8bdf7fd06..b38daa079 100644 --- a/netbox/ipam/tests/test_api.py +++ b/netbox/ipam/tests/test_api.py @@ -586,10 +586,15 @@ class PrefixTest(APITestCase): self.assertEqual(response.data['description'], data['description']) # Try to create one more prefix - response = self.client.post(url, {'prefix_length': 30}, **self.header) + response = self.client.post(url, {'prefix_length': 30}, format='json', **self.header) self.assertHttpStatus(response, status.HTTP_204_NO_CONTENT) self.assertIn('detail', response.data) + # Try to create invalid prefix type + response = self.client.post(url, {'prefix_length': '30'}, format='json', **self.header) + self.assertHttpStatus(response, status.HTTP_400_BAD_REQUEST) + self.assertIn('prefix_length', response.data[0]) + def test_create_multiple_available_prefixes(self): prefix = Prefix.objects.create(prefix=IPNetwork('192.0.2.0/28'), is_pool=True)