Fixes #23422: Skip validation for nested API serializers (#23466)

docs/plugins/development/rest-api.md

@@ -27,6 +27,8 @@ Serializers are responsible for converting Python objects to JSON data suitable
 
 The default nested representation of an object is defined by the `brief_fields` attributes under the serializer's `Meta` class. (Older versions of NetBox required the definition of a separate nested serializer.)
 
+When a serializer is used with `nested=True`, it resolves its input to an existing object and does not run its validators or `validate()` method.
+
 In addition to the base NetBoxModelSerializer class, the following serializer classes are also available for subclasses of standard base models.
 
 | Model Class           | Serializer Class                                       |

netbox/dcim/api/serializers_/device_components.py

@@ -370,76 +370,72 @@ class InterfaceSerializer(
     def validate(self, data):
         # Pop mac_address before model validation — it's a cached_property, not a model field,
         # and passing it to Interface(**attrs) in ValidatedModelSerializer.validate() would raise TypeError.
-        # data may be an Interface instance (not a dict) in some custom field code paths (#18887).
-        mac_address = _UNSET
-        if isinstance(data, dict):
-            mac_address = data.pop('mac_address', _UNSET)
+        mac_address = data.pop('mac_address', _UNSET)
         self._validate_no_mac_conflict(data, mac_address)
 
-        if not self.nested and isinstance(data, dict):
-            if mac_address not in (_UNSET, None):
-                try:
-                    EUI(mac_address, version=48)
-                except (AddrFormatError, ValueError, TypeError):
-                    raise serializers.ValidationError({
-                        'mac_address': _('Enter a valid MAC address (e.g. 00:11:22:33:44:55).')
-                    })
-
-            # Validate 802.1q mode and vlan(s)
-            mode = None
-            tagged_vlans = []
-
-            # Gather Information
-            if self.instance:
-                mode = data.get('mode') if 'mode' in data.keys() else self.instance.mode
-                untagged_vlan = data.get('untagged_vlan') if 'untagged_vlan' in data.keys() else \
-                    self.instance.untagged_vlan
-                qinq_svlan = data.get('qinq_svlan') if 'qinq_svlan' in data.keys() else \
-                    self.instance.qinq_svlan
-                tagged_vlans = data.get('tagged_vlans') if 'tagged_vlans' in data.keys() else \
-                    self.instance.tagged_vlans.all()
-            else:
-                mode = data.get('mode', None)
-                untagged_vlan = data.get('untagged_vlan') if 'untagged_vlan' in data.keys() else None
-                qinq_svlan = data.get('qinq_svlan') if 'qinq_svlan' in data.keys() else None
-                tagged_vlans = data.get('tagged_vlans') if 'tagged_vlans' in data.keys() else None
-
-            errors = {}
-
-            # Non Q-in-Q mode with service vlan set
-            if mode != InterfaceModeChoices.MODE_Q_IN_Q and qinq_svlan:
+        if mac_address not in (_UNSET, None):
+            try:
+                EUI(mac_address, version=48)
+            except (AddrFormatError, ValueError, TypeError):
+                raise serializers.ValidationError({
+                    'mac_address': _('Enter a valid MAC address (e.g. 00:11:22:33:44:55).')
+                })
+
+        # Validate 802.1q mode and vlan(s)
+        mode = None
+        tagged_vlans = []
+
+        # Gather Information
+        if self.instance:
+            mode = data.get('mode') if 'mode' in data.keys() else self.instance.mode
+            untagged_vlan = data.get('untagged_vlan') if 'untagged_vlan' in data.keys() else \
+                self.instance.untagged_vlan
+            qinq_svlan = data.get('qinq_svlan') if 'qinq_svlan' in data.keys() else \
+                self.instance.qinq_svlan
+            tagged_vlans = data.get('tagged_vlans') if 'tagged_vlans' in data.keys() else \
+                self.instance.tagged_vlans.all()
+        else:
+            mode = data.get('mode', None)
+            untagged_vlan = data.get('untagged_vlan') if 'untagged_vlan' in data.keys() else None
+            qinq_svlan = data.get('qinq_svlan') if 'qinq_svlan' in data.keys() else None
+            tagged_vlans = data.get('tagged_vlans') if 'tagged_vlans' in data.keys() else None
+
+        errors = {}
+
+        # Non Q-in-Q mode with service vlan set
+        if mode != InterfaceModeChoices.MODE_Q_IN_Q and qinq_svlan:
+            errors.update({
+                'qinq_svlan': _("Interface mode does not support q-in-q service vlan")
+            })
+        # Routed mode
+        if not mode:
+            # Untagged vlan
+            if untagged_vlan:
                 errors.update({
-                    'qinq_svlan': _("Interface mode does not support q-in-q service vlan")
+                    'untagged_vlan': _("Interface mode does not support untagged vlan")
                 })
-            # Routed mode
-            if not mode:
-                # Untagged vlan
-                if untagged_vlan:
-                    errors.update({
-                        'untagged_vlan': _("Interface mode does not support untagged vlan")
-                    })
-                # Tagged vlan
-                if tagged_vlans:
-                    errors.update({
-                        'tagged_vlans': _("Interface mode does not support tagged vlans")
-                    })
-            # Non-tagged mode
-            elif mode in (InterfaceModeChoices.MODE_TAGGED_ALL, InterfaceModeChoices.MODE_ACCESS) and tagged_vlans:
+            # Tagged vlan
+            if tagged_vlans:
                 errors.update({
                     'tagged_vlans': _("Interface mode does not support tagged vlans")
                 })
-
-            if errors:
-                raise serializers.ValidationError(errors)
-
-            # Validate many-to-many VLAN assignments
-            device = self.instance.device if self.instance else data.get('device')
-            for vlan in data.get('tagged_vlans', []):
-                if vlan.site not in [device.site, None]:
-                    raise serializers.ValidationError({
-                        'tagged_vlans': f"VLAN {vlan} must belong to the same site as the interface's parent device, "
-                                        f"or it must be global."
-                    })
+        # Non-tagged mode
+        elif mode in (InterfaceModeChoices.MODE_TAGGED_ALL, InterfaceModeChoices.MODE_ACCESS) and tagged_vlans:
+            errors.update({
+                'tagged_vlans': _("Interface mode does not support tagged vlans")
+            })
+
+        if errors:
+            raise serializers.ValidationError(errors)
+
+        # Validate many-to-many VLAN assignments
+        device = self.instance.device if self.instance else data.get('device')
+        for vlan in data.get('tagged_vlans', []):
+            if vlan.site not in [device.site, None]:
+                raise serializers.ValidationError({
+                    'tagged_vlans': f"VLAN {vlan} must belong to the same site as the interface's parent device, "
+                                    f"or it must be global."
+                })
 
         data = super().validate(data)
 

netbox/dcim/api/serializers_/devices.py

@@ -208,11 +208,6 @@ class ModuleSerializer(PrimaryModelSerializer):
         brief_fields = ('id', 'url', 'display', 'device', 'module_bay', 'module_type', 'description')
 
     def validate(self, data):
-        # When used as a nested serializer (e.g. as the `module` field on device component
-        # serializers), `data` is already a resolved Module instance — skip our custom logic.
-        if self.nested:
-            return super().validate(data)
-
         # Pop write-only transient fields before ValidatedModelSerializer tries to
         # construct a Module instance for full_clean(); restore them afterwards.
         replicate_components = data.pop('replicate_components', True)

netbox/dcim/api/serializers_/mixins.py

@@ -27,7 +27,7 @@ class MACAddressShortcutMixin:
         # The mac_address shortcut and the primary_mac_address field both set the primary MAC. Reject
         # only when both are supplied AND they disagree, so a read-modify-write round-trip (which echoes
         # both readable fields with matching values) is accepted while a genuine conflict is rejected.
-        if mac_address is _UNSET or not isinstance(data, dict) or 'primary_mac_address' not in data:
+        if mac_address is _UNSET or 'primary_mac_address' not in data:
             return
 
         primary = data['primary_mac_address']

netbox/extras/api/serializers_/journaling.py

@@ -51,7 +51,7 @@ class JournalEntrySerializer(NetBoxModelSerializer):
     def validate(self, data):
 
         # Validate that the parent object exists
-        if not self.nested and 'assigned_object_type' in data and 'assigned_object_id' in data:
+        if 'assigned_object_type' in data and 'assigned_object_id' in data:
             try:
                 data['assigned_object_type'].get_object_for_this_type(id=data['assigned_object_id'])
             except ObjectDoesNotExist:

netbox/extras/tests/test_customfields.py

@@ -21,7 +21,7 @@ from core.models import Job, ObjectChange, ObjectType
 from dcim.choices import InterfaceTypeChoices
 from dcim.filtersets import SiteFilterSet
 from dcim.forms import SiteImportForm
-from dcim.models import Cable, Device, Interface, Manufacturer, Rack, Site
+from dcim.models import Cable, Device, Interface, Manufacturer, Module, ModuleBay, ModuleType, Rack, Site
 from dcim.tables import SiteTable
 from extras.api.customfields import CustomFieldListSerializer
 from extras.choices import *
@@ -33,18 +33,19 @@ from extras.jobs import (
     provision_custom_field,
     purge_custom_field,
 )
-from extras.models import CustomField, CustomFieldChoiceSet, Tag
+from extras.models import CustomField, CustomFieldChoiceSet, ImageAttachment, JournalEntry, Tag
 from ipam.api.serializers import VLANSerializer
-from ipam.models import VLAN
+from ipam.models import VLAN, Service, ServiceTemplate
 from netbox.choices import CSVDelimiterChoices, ImportFormatChoices
 from netbox.context import query_cache
 from netbox.context_managers import event_tracking
 from netbox.tables.columns import CustomFieldColumn
 from netbox.tests.dummy_plugin.models import DummyNetBoxModel
+from users.models import Token
 from utilities.exceptions import AbortRequest
 from utilities.filters import MultiValueCharFilter, MultiValueContentTypeFilter
-from utilities.testing import APITestCase, TestCase, create_test_device
-from virtualization.models import VirtualMachine
+from utilities.testing import APITestCase, TestCase, create_test_device, create_test_virtualmachine
+from virtualization.models import VirtualMachine, VMInterface
 
 
 def get_primary_table_queries(queries, model):
@@ -2139,6 +2140,79 @@ class CustomFieldAPITestCase(APITestCase):
             list(original_cfvs['multiobject_field'])
         )
 
+    @tag('regression')
+    def test_update_single_object_with_object_values_using_custom_validation(self):
+        """
+        Object and multi-object custom fields accept objects whose serializers implement validate() (#23422).
+        """
+        device = create_test_device('Device 1')
+        module_type = ModuleType.objects.create(manufacturer=device.device_type.manufacturer, model='Module Type 1')
+        module_bay = ModuleBay.objects.create(device=device, name='Module Bay 1')
+        targets = (
+            Interface.objects.create(device=device, name='eth0', type=InterfaceTypeChoices.TYPE_1GE_FIXED),
+            Module.objects.create(device=device, module_bay=module_bay, module_type=module_type),
+            ImageAttachment.objects.create(
+                object_type=ObjectType.objects.get_for_model(Device),
+                object_id=device.pk,
+                image='image-attachments/image.png',
+                image_height=100,
+                image_width=100,
+            ),
+            JournalEntry.objects.create(assigned_object=device, comments='Journal entry'),
+            Service.objects.create(parent=device, name='Service 1', port_mappings=['tcp/22']),
+            ServiceTemplate.objects.create(name='Service Template 1', port_mappings=['tcp/22']),
+            Token.objects.create(user=self.user),
+            self.user,
+            VMInterface.objects.create(virtual_machine=create_test_virtualmachine('VM 1'), name='eth0'),
+        )
+        site = Site.objects.get(name='Site 1')
+        site_type = ObjectType.objects.get_for_model(Site)
+        url = reverse('dcim-api:site-detail', kwargs={'pk': site.pk})
+        self.add_permissions('dcim.change_site')
+
+        for obj in targets:
+            name = obj._meta.model_name
+            with self.subTest(model=name):
+                for cf_type, cf_name in (
+                    (CustomFieldTypeChoices.TYPE_OBJECT, f'{name}_object'),
+                    (CustomFieldTypeChoices.TYPE_MULTIOBJECT, f'{name}_objects'),
+                ):
+                    cf = CustomField.objects.create(
+                        name=cf_name, type=cf_type, related_object_type=ObjectType.objects.get_for_model(obj)
+                    )
+                    cf.object_types.set([site_type])
+                # Reference the object both by attributes (which requires view permission) and by PK
+                self.add_permissions(f'{obj._meta.app_label}.view_{name}')
+                data = {'custom_fields': {f'{name}_object': {'pk': obj.pk}, f'{name}_objects': [obj.pk]}}
+
+                response = self.client.patch(url, data, format='json', **self.header)
+                self.assertHttpStatus(response, status.HTTP_200_OK)
+                site.refresh_from_db()
+                self.assertEqual(site.custom_field_data[f'{name}_object'], obj.pk)
+                self.assertEqual(site.custom_field_data[f'{name}_objects'], [obj.pk])
+
+    def test_update_single_object_rejects_unknown_object_values(self):
+        """Object and multi-object custom fields reject references which match no object."""
+        site2 = Site.objects.get(name='Site 2')
+        original_cf_data = {**site2.custom_field_data}
+        missing_pk = VLAN.objects.order_by('-pk').values_list('pk', flat=True).first() + 1
+        url = reverse('dcim-api:site-detail', kwargs={'pk': site2.pk})
+        self.add_permissions('dcim.change_site', 'ipam.view_vlan')
+
+        for field_name, value in (
+            ('object_field', missing_pk),
+            ('object_field', {'name': 'VLAN 99'}),
+            ('multiobject_field', [missing_pk]),
+            ('multiobject_field', [{'name': 'VLAN 99'}]),
+        ):
+            with self.subTest(field=field_name, value=value):
+                response = self.client.patch(url, {'custom_fields': {field_name: value}}, format='json', **self.header)
+                self.assertHttpStatus(response, status.HTTP_400_BAD_REQUEST)
+                self.assertIn('custom_fields', response.data)
+
+        site2.refresh_from_db()
+        self.assertEqual(site2.custom_field_data, original_cf_data)
+
     @tag('regression')
     def test_update_single_object_rejects_unknown_custom_fields(self):
         site2 = Site.objects.get(name='Site 2')

netbox/netbox/api/serializers/base.py

@@ -3,6 +3,7 @@ from functools import cached_property
 from drf_spectacular.types import OpenApiTypes
 from drf_spectacular.utils import extend_schema_field
 from rest_framework import serializers
+from rest_framework.fields import empty
 
 from netbox.api.fields import RelatedObjectCountField
 from utilities.api import get_related_object_by_attrs
@@ -44,11 +45,27 @@ class BaseModelSerializer(serializers.ModelSerializer):
 
         super().__init__(*args, **kwargs)
 
+    @property
+    def resolves_existing_object(self):
+        """
+        Indicates whether this serializer resolves its input to an existing object, rather than validating new data.
+        """
+        return self.nested
+
+    def run_validation(self, data=empty):
+        # An existing object needs no validation, so skip validators and validate()
+        if self.resolves_existing_object:
+            is_empty_value, data = self.validate_empty_values(data)
+            if is_empty_value:
+                return data
+            return self.to_internal_value(data)
+
+        return super().run_validation(data)
+
     def to_internal_value(self, data):
 
-        # If initialized as a nested serializer, we should expect to receive the attrs or PK
-        # identifying a related object.
-        if self.nested:
+        # Expect to receive the attrs or PK identifying an existing object
+        if self.resolves_existing_object:
             queryset = self.Meta.model.objects.all()
             request = self.context.get('request')
             user = request.user if request else None
@@ -104,11 +121,6 @@ class ValidatedModelSerializer(BaseModelSerializer):
         return []
 
     def validate(self, data):
-
-        # Skip validation if we're being used to represent a nested object
-        if self.nested:
-            return data
-
         attrs = data.copy()
 
         # Skip ManyToManyFields

netbox/netbox/api/serializers/features.py

@@ -69,10 +69,6 @@ class TaggableModelSerializer(serializers.Serializer):
         return ret
 
     def validate(self, data):
-        # Skip validation for nested serializer representations (e.g. when used as a related field)
-        if type(data) is not dict:
-            return super().validate(data)
-
         if data.get('tags') and (data.get('add_tags') or data.get('remove_tags')):
             raise serializers.ValidationError({
                 'tags': 'Cannot specify "tags" together with "add_tags" or "remove_tags".'

netbox/netbox/api/serializers/nested.py

@@ -1,5 +1,4 @@
 from extras.models import Tag
-from utilities.api import get_related_object_by_attrs
 
 from .base import BaseModelSerializer
 
@@ -15,11 +14,9 @@ class WritableNestedSerializer(BaseModelSerializer):
     dictionary of attributes which can be used to uniquely identify the related object. This class should be
     subclassed to return a full representation of the related object on read.
     """
-    def to_internal_value(self, data):
-        queryset = self.Meta.model.objects.all()
-        request = self.context.get('request')
-        user = request.user if request else None
-        return get_related_object_by_attrs(queryset, data, user=user)
+    @property
+    def resolves_existing_object(self):
+        return True
 
 
 # Declared here for use by PrimaryModelSerializer

netbox/netbox/tests/test_api.py

@@ -23,7 +23,7 @@ from ipam.models import VRF, RouteTarget
 from netbox.api.exceptions import QuerySetNotOrdered, SerializerNotFound
 from netbox.api.fields import ContentTypeField, IntegerRangeSerializer, RelatedObjectCountField
 from netbox.api.pagination import NetBoxPagination
-from netbox.api.serializers import ValidatedModelSerializer
+from netbox.api.serializers import ValidatedModelSerializer, WritableNestedSerializer
 from users.constants import TOKEN_PREFIX
 from users.models import Token, User
 from utilities.api import get_serializer_for_model
@@ -437,7 +437,7 @@ class ValidatedModelSerializerTestCase(TestCase):
         self.assertEqual(opted_in.validated_data['template_code'], '{{ synced }}')
 
     def test_nested_serializer_skips_model_validation(self):
-        """A serializer representing a nested object returns its input untouched."""
+        """A serializer representing a nested object resolves its input without calling validate()."""
 
         class OptedInSerializer(ValidatedModelSerializer):
             class Meta:
@@ -446,10 +446,34 @@ class ValidatedModelSerializerTestCase(TestCase):
                 brief_fields = ('name',)
                 model_clean_fields = ('template_code',)
 
+        export_template = ExportTemplate.objects.create(name='Export Template 1', template_code='{# untouched #}')
         serializer = OptedInSerializer(nested=True)
-        data = {'template_code': '{# untouched #}'}
 
-        self.assertEqual(serializer.validate(data), data)
+        with patch.object(OptedInSerializer, 'validate') as validate:
+            self.assertEqual(serializer.run_validation(export_template.pk), export_template)
+        validate.assert_not_called()
+
+
+class WritableNestedSerializerTestCase(TestCase):
+
+    def test_skips_validation(self):
+        """A WritableNestedSerializer resolves its input without running validators or validate()."""
+
+        class NestedExportTemplateSerializer(WritableNestedSerializer):
+            class Meta:
+                model = ExportTemplate
+                fields = ['id', 'name']
+
+        export_template = ExportTemplate.objects.create(name='Export Template 1', template_code='{# untouched #}')
+        serializer = NestedExportTemplateSerializer()
+
+        with (
+            patch.object(NestedExportTemplateSerializer, 'run_validators') as run_validators,
+            patch.object(NestedExportTemplateSerializer, 'validate') as validate,
+        ):
+            self.assertEqual(serializer.run_validation(export_template.pk), export_template)
+        run_validators.assert_not_called()
+        validate.assert_not_called()
 
 
 ROUTED_ALIAS = 'routed'

netbox/users/api/serializers_/users.py

@@ -63,7 +63,7 @@ class UserSerializer(ValidatedModelSerializer):
     def validate(self, data):
 
         # Enforce password validation rules (if configured)
-        if not self.nested and data.get('password'):
+        if data.get('password'):
             password_validation.validate_password(data['password'], self.instance)
 
         return super().validate(data)

netbox/virtualization/api/serializers_/virtualmachines.py

@@ -150,13 +150,10 @@ class VMInterfaceSerializer(MACAddressShortcutMixin, OwnerMixin, NetBoxModelSeri
 
     def validate(self, data):
         # Pop mac_address before model validation — it's a cached_property, not a model field.
-        # data may be a VMInterface instance (not a dict) in some custom field code paths (#18887).
-        mac_address = _UNSET
-        if isinstance(data, dict):
-            mac_address = data.pop('mac_address', _UNSET)
+        mac_address = data.pop('mac_address', _UNSET)
         self._validate_no_mac_conflict(data, mac_address)
 
-        if not self.nested and isinstance(data, dict) and mac_address not in (_UNSET, None):
+        if mac_address not in (_UNSET, None):
             try:
                 EUI(mac_address, version=48)
             except (AddrFormatError, ValueError, TypeError):
@@ -165,26 +162,9 @@ class VMInterfaceSerializer(MACAddressShortcutMixin, OwnerMixin, NetBoxModelSeri
                 })
 
         # Validate many-to-many VLAN assignments
-        virtual_machine = None
-        tagged_vlans = []
-
-        # #18887
-        # There seem to be multiple code paths coming through here. Previously, we might either get
-        # the VirtualMachine instance from self.instance or from incoming data. However, #18887
-        # illustrated that this is also being called when a custom field pointing to an object_type
-        # of VMInterface is on the right side of a custom-field assignment coming in from an API
-        # request. As such, we need to check a third way to access the VirtualMachine
-        # instance--where `data` is the VMInterface instance itself and we can get the associated
-        # VirtualMachine via attribute access.
-        if isinstance(data, dict):
-            virtual_machine = self.instance.virtual_machine if self.instance else data.get('virtual_machine')
-            tagged_vlans = data.get('tagged_vlans', [])
-        elif isinstance(data, VMInterface):
-            virtual_machine = data.virtual_machine
-            tagged_vlans = data.tagged_vlans.all()
-
+        virtual_machine = self.instance.virtual_machine if self.instance else data.get('virtual_machine')
         if virtual_machine:
-            for vlan in tagged_vlans:
+            for vlan in data.get('tagged_vlans', []):
                 if vlan.site not in [virtual_machine.site, None]:
                     raise serializers.ValidationError({
                         'tagged_vlans': f"VLAN {vlan} must belong to the same site as the interface's parent virtual "