From 4a8a1ce45cb9f94000bbaeabb363f829b5d92b30 Mon Sep 17 00:00:00 2001 From: Jeremy Stretch Date: Thu, 12 Nov 2020 12:18:31 -0500 Subject: [PATCH] Check for extraneous custom field data on clean() --- netbox/dcim/models/cables.py | 2 ++ netbox/dcim/models/devices.py | 3 +- netbox/dcim/models/power.py | 2 ++ netbox/dcim/models/racks.py | 2 ++ netbox/extras/models/customfields.py | 10 ++++++ netbox/extras/tests/test_customfields.py | 44 +++++++++++++++++++++++- netbox/ipam/models.py | 5 +++ netbox/secrets/models.py | 5 ++- netbox/virtualization/models.py | 2 +- 9 files changed, 69 insertions(+), 6 deletions(-) diff --git a/netbox/dcim/models/cables.py b/netbox/dcim/models/cables.py index 03656533c..9794ef8fa 100644 --- a/netbox/dcim/models/cables.py +++ b/netbox/dcim/models/cables.py @@ -155,6 +155,8 @@ class Cable(ChangeLoggedModel, CustomFieldModel): def clean(self): from circuits.models import CircuitTermination + super().clean() + # Validate that termination A exists if not hasattr(self, 'termination_a_type'): raise ValidationError('Termination A type has not been specified') diff --git a/netbox/dcim/models/devices.py b/netbox/dcim/models/devices.py index 662308d28..5b685057e 100644 --- a/netbox/dcim/models/devices.py +++ b/netbox/dcim/models/devices.py @@ -254,6 +254,7 @@ class DeviceType(ChangeLoggedModel, CustomFieldModel): return yaml.dump(dict(data), sort_keys=False) def clean(self): + super().clean() # If editing an existing DeviceType to have a larger u_height, first validate that *all* instances of it have # room to expand within their racks. This validation will impose a very high performance penalty when there are @@ -634,7 +635,6 @@ class Device(ChangeLoggedModel, ConfigContextModel, CustomFieldModel): super().validate_unique(exclude) def clean(self): - super().clean() # Validate site/rack combination @@ -917,6 +917,7 @@ class VirtualChassis(ChangeLoggedModel, CustomFieldModel): return reverse('dcim:virtualchassis', kwargs={'pk': self.pk}) def clean(self): + super().clean() # Verify that the selected master device has been assigned to this VirtualChassis. (Skip when creating a new # VirtualChassis.) diff --git a/netbox/dcim/models/power.py b/netbox/dcim/models/power.py index 8dae9074a..1215ced4c 100644 --- a/netbox/dcim/models/power.py +++ b/netbox/dcim/models/power.py @@ -64,6 +64,7 @@ class PowerPanel(ChangeLoggedModel, CustomFieldModel): ) def clean(self): + super().clean() # RackGroup must belong to assigned Site if self.rack_group and self.rack_group.site != self.site: @@ -172,6 +173,7 @@ class PowerFeed(ChangeLoggedModel, PathEndpoint, CableTermination, CustomFieldMo ) def clean(self): + super().clean() # Rack must belong to same Site as PowerPanel if self.rack and self.rack.site != self.power_panel.site: diff --git a/netbox/dcim/models/racks.py b/netbox/dcim/models/racks.py index b5dd92069..cad20241b 100644 --- a/netbox/dcim/models/racks.py +++ b/netbox/dcim/models/racks.py @@ -296,6 +296,7 @@ class Rack(ChangeLoggedModel, CustomFieldModel): return reverse('dcim:rack', args=[self.pk]) def clean(self): + super().clean() # Validate outer dimensions and unit if (self.outer_width is not None or self.outer_depth is not None) and not self.outer_unit: @@ -602,6 +603,7 @@ class RackReservation(ChangeLoggedModel, CustomFieldModel): return reverse('dcim:rackreservation', args=[self.pk]) def clean(self): + super().clean() if hasattr(self, 'rack') and self.units: diff --git a/netbox/extras/models/customfields.py b/netbox/extras/models/customfields.py index 203d58cd3..fa4f55fa6 100644 --- a/netbox/extras/models/customfields.py +++ b/netbox/extras/models/customfields.py @@ -43,6 +43,16 @@ class CustomFieldModel(models.Model): (field, self.custom_field_data.get(field.name)) for field in fields ]) + def clean(self): + + # Validate custom field data + custom_field_names = CustomField.objects.get_for_model(self).values_list('name', flat=True) + for field_name in self.custom_field_data: + if field_name not in custom_field_names: + raise ValidationError({ + 'custom_field_data': f'Unknown custom field: {field_name}' + }) + class CustomFieldManager(models.Manager): use_in_migrations = True diff --git a/netbox/extras/tests/test_customfields.py b/netbox/extras/tests/test_customfields.py index ca33c4697..9286acf1e 100644 --- a/netbox/extras/tests/test_customfields.py +++ b/netbox/extras/tests/test_customfields.py @@ -1,9 +1,10 @@ from django.contrib.contenttypes.models import ContentType +from django.core.exceptions import ValidationError from django.urls import reverse from rest_framework import status from dcim.forms import SiteCSVForm -from dcim.models import Site +from dcim.models import Site, Rack from extras.choices import * from extras.models import CustomField from utilities.testing import APITestCase, TestCase @@ -534,3 +535,44 @@ class CustomFieldImportTest(TestCase): form = SiteCSVForm(data=form_data) self.assertFalse(form.is_valid()) self.assertIn('cf_select', form.errors) + + +class CustomFieldModelTest(TestCase): + + @classmethod + def setUpTestData(cls): + cf1 = CustomField(type=CustomFieldTypeChoices.TYPE_TEXT, name='foo') + cf1.save() + cf1.content_types.set([ContentType.objects.get_for_model(Site)]) + + cf2 = CustomField(type=CustomFieldTypeChoices.TYPE_TEXT, name='bar') + cf2.save() + cf2.content_types.set([ContentType.objects.get_for_model(Rack)]) + + def test_cf_data(self): + site = Site(name='Test Site', slug='test-site') + + # Check custom field data on new instance + site.cf['foo'] = 'abc' + self.assertEqual(site.cf['foo'], 'abc') + + # Check custom field data from database + site.save() + site = Site.objects.get(name='Test Site') + self.assertEqual(site.cf['foo'], 'abc') + + def test_invalid_data(self): + """ + Setting custom field data for a non-applicable (or non-existent) CustomField should raise a ValidationError. + """ + site = Site(name='Test Site', slug='test-site') + + # Set custom field data + site.cf['foo'] = 'abc' + site.cf['bar'] = 'def' + + with self.assertRaises(ValidationError): + site.clean() + + del(site.cf['bar']) + site.clean() diff --git a/netbox/ipam/models.py b/netbox/ipam/models.py index 493df9d9a..0ae9db7b2 100644 --- a/netbox/ipam/models.py +++ b/netbox/ipam/models.py @@ -256,6 +256,7 @@ class Aggregate(ChangeLoggedModel, CustomFieldModel): return reverse('ipam:aggregate', args=[self.pk]) def clean(self): + super().clean() if self.prefix: @@ -442,6 +443,7 @@ class Prefix(ChangeLoggedModel, CustomFieldModel): return reverse('ipam:prefix', args=[self.pk]) def clean(self): + super().clean() if self.prefix: @@ -721,6 +723,7 @@ class IPAddress(ChangeLoggedModel, CustomFieldModel): ).exclude(pk=self.pk) def clean(self): + super().clean() if self.address: @@ -970,6 +973,7 @@ class VLAN(ChangeLoggedModel, CustomFieldModel): return reverse('ipam:vlan', args=[self.pk]) def clean(self): + super().clean() # Validate VLAN group if self.group and self.group.site != self.site: @@ -1078,6 +1082,7 @@ class Service(ChangeLoggedModel, CustomFieldModel): return self.device or self.virtual_machine def clean(self): + super().clean() # A Service must belong to a Device *or* to a VirtualMachine if self.device and self.virtual_machine: diff --git a/netbox/secrets/models.py b/netbox/secrets/models.py index 862fd4f2d..0fa6fd04e 100644 --- a/netbox/secrets/models.py +++ b/netbox/secrets/models.py @@ -74,7 +74,8 @@ class UserKey(models.Model): def __str__(self): return self.user.username - def clean(self, *args, **kwargs): + def clean(self): + super().clean() if self.public_key: @@ -105,8 +106,6 @@ class UserKey(models.Model): ) }) - super().clean() - def save(self, *args, **kwargs): # Check whether public_key has been modified. If so, nullify the initial master_key_cipher. diff --git a/netbox/virtualization/models.py b/netbox/virtualization/models.py index 544a14cd0..1edfe872b 100644 --- a/netbox/virtualization/models.py +++ b/netbox/virtualization/models.py @@ -172,6 +172,7 @@ class Cluster(ChangeLoggedModel, CustomFieldModel): return reverse('virtualization:cluster', args=[self.pk]) def clean(self): + super().clean() # If the Cluster is assigned to a Site, verify that all host Devices belong to that Site. if self.pk and self.site: @@ -317,7 +318,6 @@ class VirtualMachine(ChangeLoggedModel, ConfigContextModel, CustomFieldModel): super().validate_unique(exclude) def clean(self): - super().clean() # Validate primary IP addresses