From 5c9a14525573ab580f6ec26f2ac6f5620a2979fd Mon Sep 17 00:00:00 2001
From: Daniel Sheppard <dans@dansheps.com>
Date: Mon, 19 Aug 2024 20:26:27 -0500
Subject: [PATCH 01/12] Fixes: #15924 - Prevent API payload from allowing
 tagged_vlans while interface mode is set to taged-all

---
 netbox/dcim/api/serializers_/device_components.py | 8 ++++++++
 netbox/dcim/models/device_components.py           | 4 ++++
 2 files changed, 12 insertions(+)

diff --git a/netbox/dcim/api/serializers_/device_components.py b/netbox/dcim/api/serializers_/device_components.py
index fd32d95d0c6..9e021272da3 100644
--- a/netbox/dcim/api/serializers_/device_components.py
+++ b/netbox/dcim/api/serializers_/device_components.py
@@ -1,3 +1,4 @@
+from django.utils.translation import gettext as _
 from django.contrib.contenttypes.models import ContentType
 from drf_spectacular.utils import extend_schema_field
 from rest_framework import serializers
@@ -246,6 +247,13 @@ def validate(self, data):
                                         f"or it must be global."
                     })
 
+        # Validate that tagged-all payload does not include tagged_vlans
+        mode = data.get('mode') or self.instance.mode
+        if mode == InterfaceModeChoices.MODE_TAGGED_ALL and data.get('tagged_vlans'):
+            raise serializers.ValidationError({
+                'tagged_vlans': "Tagged-All interface mode must not include any tagged vlans"
+            })
+
         return super().validate(data)
 
 
diff --git a/netbox/dcim/models/device_components.py b/netbox/dcim/models/device_components.py
index 9438b741f58..5a087b495a8 100644
--- a/netbox/dcim/models/device_components.py
+++ b/netbox/dcim/models/device_components.py
@@ -912,6 +912,10 @@ def save(self, *args, **kwargs):
         if self.rf_channel and not self.rf_channel_width:
             self.rf_channel_width = get_channel_attr(self.rf_channel, 'width')
 
+        # Clear any tagged vlans set when mode is tagged-all
+        if self.mode == InterfaceModeChoices.MODE_TAGGED_ALL and self.tagged_vlans:
+            self.tagged_vlans.set([])
+
         super().save(*args, **kwargs)
 
     @property

From 282836c9a003e8ccd5608e323384a92f7b501c33 Mon Sep 17 00:00:00 2001
From: Daniel Sheppard <dans@dansheps.com>
Date: Mon, 19 Aug 2024 20:33:42 -0500
Subject: [PATCH 02/12] Prevent cleanup of tagged_vlans when no tagged_vlans
 set on interface

---
 netbox/dcim/models/device_components.py | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/netbox/dcim/models/device_components.py b/netbox/dcim/models/device_components.py
index 5a087b495a8..951230bffa2 100644
--- a/netbox/dcim/models/device_components.py
+++ b/netbox/dcim/models/device_components.py
@@ -913,7 +913,7 @@ def save(self, *args, **kwargs):
             self.rf_channel_width = get_channel_attr(self.rf_channel, 'width')
 
         # Clear any tagged vlans set when mode is tagged-all
-        if self.mode == InterfaceModeChoices.MODE_TAGGED_ALL and self.tagged_vlans:
+        if self.mode == InterfaceModeChoices.MODE_TAGGED_ALL and self.tagged_vlans.count():
             self.tagged_vlans.set([])
 
         super().save(*args, **kwargs)

From 4ea00474aa555388187c2ca40fd316ab8ed7f572 Mon Sep 17 00:00:00 2001
From: Daniel Sheppard <dans@dansheps.com>
Date: Mon, 19 Aug 2024 21:50:16 -0500
Subject: [PATCH 03/12] Fix test errors

---
 netbox/dcim/api/serializers_/device_components.py | 14 +++++++-------
 netbox/dcim/models/device_components.py           |  4 ++--
 netbox/utilities/testing/api.py                   |  2 ++
 3 files changed, 11 insertions(+), 9 deletions(-)

diff --git a/netbox/dcim/api/serializers_/device_components.py b/netbox/dcim/api/serializers_/device_components.py
index 9e021272da3..fd66f0a1fc1 100644
--- a/netbox/dcim/api/serializers_/device_components.py
+++ b/netbox/dcim/api/serializers_/device_components.py
@@ -237,8 +237,8 @@ class Meta:
 
     def validate(self, data):
 
-        # Validate many-to-many VLAN assignments
         if not self.nested:
+            # 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]:
@@ -247,12 +247,12 @@ def validate(self, data):
                                         f"or it must be global."
                     })
 
-        # Validate that tagged-all payload does not include tagged_vlans
-        mode = data.get('mode') or self.instance.mode
-        if mode == InterfaceModeChoices.MODE_TAGGED_ALL and data.get('tagged_vlans'):
-            raise serializers.ValidationError({
-                'tagged_vlans': "Tagged-All interface mode must not include any tagged vlans"
-            })
+            # Validate that tagged-all payload does not include tagged_vlans
+            mode = data.get('mode') or getattr(self.instance, 'mode', None)
+            if mode == InterfaceModeChoices.MODE_TAGGED_ALL and data.get('tagged_vlans'):
+                raise serializers.ValidationError({
+                    'tagged_vlans': "Tagged-All interface mode must not include any tagged vlans"
+                })
 
         return super().validate(data)
 
diff --git a/netbox/dcim/models/device_components.py b/netbox/dcim/models/device_components.py
index 951230bffa2..a785d5db14e 100644
--- a/netbox/dcim/models/device_components.py
+++ b/netbox/dcim/models/device_components.py
@@ -912,12 +912,12 @@ def save(self, *args, **kwargs):
         if self.rf_channel and not self.rf_channel_width:
             self.rf_channel_width = get_channel_attr(self.rf_channel, 'width')
 
+        super().save(*args, **kwargs)
+
         # Clear any tagged vlans set when mode is tagged-all
         if self.mode == InterfaceModeChoices.MODE_TAGGED_ALL and self.tagged_vlans.count():
             self.tagged_vlans.set([])
 
-        super().save(*args, **kwargs)
-
     @property
     def _occupied(self):
         return super()._occupied or bool(self.wireless_link_id)
diff --git a/netbox/utilities/testing/api.py b/netbox/utilities/testing/api.py
index 7bb349a6628..baa8e29209c 100644
--- a/netbox/utilities/testing/api.py
+++ b/netbox/utilities/testing/api.py
@@ -230,6 +230,8 @@ def test_create_object(self):
             obj_perm.object_types.add(ObjectType.objects.get_for_model(self.model))
 
             initial_count = self._get_queryset().count()
+            print(self._get_list_url())
+            print(self.create_data[0])
             response = self.client.post(self._get_list_url(), self.create_data[0], format='json', **self.header)
             self.assertHttpStatus(response, status.HTTP_201_CREATED)
             self.assertEqual(self._get_queryset().count(), initial_count + 1)

From d4c8e88fd771b516ab372be27d81578e4060f3b9 Mon Sep 17 00:00:00 2001
From: Daniel Sheppard <dans@dansheps.com>
Date: Thu, 22 Aug 2024 08:13:14 -0500
Subject: [PATCH 04/12] Remove accidental debug statements

---
 netbox/utilities/testing/api.py | 2 --
 1 file changed, 2 deletions(-)

diff --git a/netbox/utilities/testing/api.py b/netbox/utilities/testing/api.py
index baa8e29209c..7bb349a6628 100644
--- a/netbox/utilities/testing/api.py
+++ b/netbox/utilities/testing/api.py
@@ -230,8 +230,6 @@ def test_create_object(self):
             obj_perm.object_types.add(ObjectType.objects.get_for_model(self.model))
 
             initial_count = self._get_queryset().count()
-            print(self._get_list_url())
-            print(self.create_data[0])
             response = self.client.post(self._get_list_url(), self.create_data[0], format='json', **self.header)
             self.assertHttpStatus(response, status.HTTP_201_CREATED)
             self.assertEqual(self._get_queryset().count(), initial_count + 1)

From bab5a5b273be34853047b1caa34c835d29ed23df Mon Sep 17 00:00:00 2001
From: Daniel Sheppard <dans@dansheps.com>
Date: Sun, 25 Aug 2024 15:37:59 -0500
Subject: [PATCH 05/12] Update validation to model clean method instead of
 serializer

---
 netbox/dcim/api/serializers_/device_components.py | 7 -------
 netbox/dcim/models/device_components.py           | 6 ++++++
 2 files changed, 6 insertions(+), 7 deletions(-)

diff --git a/netbox/dcim/api/serializers_/device_components.py b/netbox/dcim/api/serializers_/device_components.py
index fd66f0a1fc1..d342a8c49ad 100644
--- a/netbox/dcim/api/serializers_/device_components.py
+++ b/netbox/dcim/api/serializers_/device_components.py
@@ -247,13 +247,6 @@ def validate(self, data):
                                         f"or it must be global."
                     })
 
-            # Validate that tagged-all payload does not include tagged_vlans
-            mode = data.get('mode') or getattr(self.instance, 'mode', None)
-            if mode == InterfaceModeChoices.MODE_TAGGED_ALL and data.get('tagged_vlans'):
-                raise serializers.ValidationError({
-                    'tagged_vlans': "Tagged-All interface mode must not include any tagged vlans"
-                })
-
         return super().validate(data)
 
 
diff --git a/netbox/dcim/models/device_components.py b/netbox/dcim/models/device_components.py
index a785d5db14e..88270bd855d 100644
--- a/netbox/dcim/models/device_components.py
+++ b/netbox/dcim/models/device_components.py
@@ -904,6 +904,12 @@ def clean(self):
                 ).format(untagged_vlan=self.untagged_vlan)
             })
 
+        # Validate that tagged-all payload does not include tagged_vlans
+        if self.mode != InterfaceModeChoices.MODE_TAGGED and self.tagged_vlans:
+            raise ValidationError({
+                'tagged_vlans': "Interface mode does not support including tagged vlans"
+            })
+
     def save(self, *args, **kwargs):
 
         # Set absolute channel attributes from selected options

From 4f830ce3cc63db0f08c896055e80e5bcb38735d6 Mon Sep 17 00:00:00 2001
From: Daniel Sheppard <dans@dansheps.com>
Date: Sun, 25 Aug 2024 15:39:22 -0500
Subject: [PATCH 06/12] Remove clearing of tagged vlans from `save()`

---
 netbox/dcim/models/device_components.py | 4 ----
 1 file changed, 4 deletions(-)

diff --git a/netbox/dcim/models/device_components.py b/netbox/dcim/models/device_components.py
index 88270bd855d..b4382b455c9 100644
--- a/netbox/dcim/models/device_components.py
+++ b/netbox/dcim/models/device_components.py
@@ -920,10 +920,6 @@ def save(self, *args, **kwargs):
 
         super().save(*args, **kwargs)
 
-        # Clear any tagged vlans set when mode is tagged-all
-        if self.mode == InterfaceModeChoices.MODE_TAGGED_ALL and self.tagged_vlans.count():
-            self.tagged_vlans.set([])
-
     @property
     def _occupied(self):
         return super()._occupied or bool(self.wireless_link_id)

From d15954775875f19ac8c9f26ba6248b24341e16ac Mon Sep 17 00:00:00 2001
From: Daniel Sheppard <dans@dansheps.com>
Date: Mon, 26 Aug 2024 23:57:55 -0500
Subject: [PATCH 07/12] Make changes to validation to account for M2M not being
 available under model in addition to not being able to check incoming vlans
 under same model.

---
 .../api/serializers_/device_components.py     | 21 +++++++++++++++++++
 netbox/dcim/forms/model_forms.py              | 20 ++++++++++++++++++
 netbox/dcim/models/device_components.py       |  8 ++-----
 3 files changed, 43 insertions(+), 6 deletions(-)

diff --git a/netbox/dcim/api/serializers_/device_components.py b/netbox/dcim/api/serializers_/device_components.py
index d342a8c49ad..f26e81aa14b 100644
--- a/netbox/dcim/api/serializers_/device_components.py
+++ b/netbox/dcim/api/serializers_/device_components.py
@@ -238,6 +238,27 @@ class Meta:
     def validate(self, data):
 
         if not self.nested:
+
+            # Validate 802.1q mode and vlan(s)
+            mode = None
+            tagged_vlans = []
+
+            if self.instance.pk and 'mode' in data.keys():
+                mode = data.get('mode') if 'mode' in self.data.keys() else self.instance.get('mode')
+            elif 'mode' in data.keys():
+                mode = data.get('mode')
+
+            if self.instance.pk and 'tagged_vlans' in data.keys():
+                tagged_vlans = data.get('tagged_vlans') if 'tagged_vlans' in data.keys() else \
+                    self.instance.tagged_vlans.all()
+            elif 'tagged_vlans' in data.keys():
+                tagged_vlans = data.get('tagged_vlans')
+
+            if mode != InterfaceModeChoices.MODE_TAGGED and tagged_vlans:
+                raise serializers.ValidationError({
+                    'tagged_vlans': "Interface mode does not support including tagged vlans"
+                })
+
             # Validate many-to-many VLAN assignments
             device = self.instance.device if self.instance else data.get('device')
             for vlan in data.get('tagged_vlans', []):
diff --git a/netbox/dcim/forms/model_forms.py b/netbox/dcim/forms/model_forms.py
index c12ddccde67..a03175725d6 100644
--- a/netbox/dcim/forms/model_forms.py
+++ b/netbox/dcim/forms/model_forms.py
@@ -1363,6 +1363,26 @@ class Meta:
             'mode': '802.1Q Mode',
         }
 
+    def clean(self):
+        mode = None
+        tagged_vlans = []
+
+        if self.instance.pk and 'mode' in self.cleaned_data.keys():
+            mode = self.cleaned_data.get('mode') if 'mode' in self.cleaned_data.keys() else self.instance.get('mode')
+        elif 'mode' in self.cleaned_data.keys():
+            mode = self.cleaned_data.get('mode')
+
+        if self.instance.pk and 'tagged_vlans' in self.cleaned_data.keys():
+            tagged_vlans = self.cleaned_data.get('tagged_vlans') if 'tagged_vlans' in self.cleaned_data.keys() else\
+                self.instance.tagged_vlans.all()
+        elif 'tagged_vlans' in self.cleaned_data.keys():
+            tagged_vlans = self.cleaned_data.get('tagged_vlans')
+
+        if mode != InterfaceModeChoices.MODE_TAGGED and tagged_vlans:
+            raise forms.ValidationError({
+                'tagged_vlans': "Interface mode does not support including tagged vlans"
+            })
+
 
 class FrontPortForm(ModularDeviceComponentForm):
     rear_port = DynamicModelChoiceField(
diff --git a/netbox/dcim/models/device_components.py b/netbox/dcim/models/device_components.py
index b4382b455c9..101a1a45902 100644
--- a/netbox/dcim/models/device_components.py
+++ b/netbox/dcim/models/device_components.py
@@ -894,6 +894,8 @@ def clean(self):
                 raise ValidationError({'rf_channel_width': _("Cannot specify custom width with channel selected.")})
 
         # VLAN validation
+        if self.mode is None and self.untagged_vlan:
+            raise ValidationError({'untagged_vlan': _("Interface mode does not support including an untagged vlan.")})
 
         # Validate untagged VLAN
         if self.untagged_vlan and self.untagged_vlan.site not in [self.device.site, None]:
@@ -904,12 +906,6 @@ def clean(self):
                 ).format(untagged_vlan=self.untagged_vlan)
             })
 
-        # Validate that tagged-all payload does not include tagged_vlans
-        if self.mode != InterfaceModeChoices.MODE_TAGGED and self.tagged_vlans:
-            raise ValidationError({
-                'tagged_vlans': "Interface mode does not support including tagged vlans"
-            })
-
     def save(self, *args, **kwargs):
 
         # Set absolute channel attributes from selected options

From 9a13caae7a942675a4802c89f867b3c91a49897b Mon Sep 17 00:00:00 2001
From: Daniel Sheppard <dans@dansheps.com>
Date: Tue, 27 Aug 2024 00:21:38 -0500
Subject: [PATCH 08/12] Optimize untagged vlan check

---
 netbox/dcim/api/serializers_/device_components.py | 10 ++++++++--
 netbox/dcim/forms/model_forms.py                  |  9 ++++++++-
 netbox/dcim/models/device_components.py           |  2 +-
 3 files changed, 17 insertions(+), 4 deletions(-)

diff --git a/netbox/dcim/api/serializers_/device_components.py b/netbox/dcim/api/serializers_/device_components.py
index f26e81aa14b..16b68671bb6 100644
--- a/netbox/dcim/api/serializers_/device_components.py
+++ b/netbox/dcim/api/serializers_/device_components.py
@@ -244,7 +244,7 @@ def validate(self, data):
             tagged_vlans = []
 
             if self.instance.pk and 'mode' in data.keys():
-                mode = data.get('mode') if 'mode' in self.data.keys() else self.instance.get('mode')
+                mode = data.get('mode') if 'mode' in data.keys() else self.instance.get('mode')
             elif 'mode' in data.keys():
                 mode = data.get('mode')
 
@@ -254,9 +254,15 @@ def validate(self, data):
             elif 'tagged_vlans' in data.keys():
                 tagged_vlans = data.get('tagged_vlans')
 
+            if self.instance.pk and 'untagged_vlan' in data.keys():
+                untagged_vlan = data.get('untagged_vlan') if 'untagged_vlan' in data.keys() else \
+                    self.instance.untagged_vlan
+            elif 'untagged_vlan' in data.keys():
+                untagged_vlan = data.get('untagged_vlan')
+
             if mode != InterfaceModeChoices.MODE_TAGGED and tagged_vlans:
                 raise serializers.ValidationError({
-                    'tagged_vlans': "Interface mode does not support including tagged vlans"
+                    'tagged_vlans': _("Interface mode does not support including tagged vlans")
                 })
 
             # Validate many-to-many VLAN assignments
diff --git a/netbox/dcim/forms/model_forms.py b/netbox/dcim/forms/model_forms.py
index a03175725d6..b26247a946f 100644
--- a/netbox/dcim/forms/model_forms.py
+++ b/netbox/dcim/forms/model_forms.py
@@ -1366,6 +1366,7 @@ class Meta:
     def clean(self):
         mode = None
         tagged_vlans = []
+        untagged_vlan = None
 
         if self.instance.pk and 'mode' in self.cleaned_data.keys():
             mode = self.cleaned_data.get('mode') if 'mode' in self.cleaned_data.keys() else self.instance.get('mode')
@@ -1378,9 +1379,15 @@ def clean(self):
         elif 'tagged_vlans' in self.cleaned_data.keys():
             tagged_vlans = self.cleaned_data.get('tagged_vlans')
 
+        if self.instance.pk and 'untagged_vlan' in self.cleaned_data.keys():
+            untagged_vlan = self.cleaned_data.get('untagged_vlan') if 'untagged_vlan' in self.cleaned_data.keys() else\
+                self.instance.untagged_vlan
+        elif 'untagged_vlan' in self.cleaned_data.keys():
+            untagged_vlan = self.cleaned_data.get('untagged_vlan')
+
         if mode != InterfaceModeChoices.MODE_TAGGED and tagged_vlans:
             raise forms.ValidationError({
-                'tagged_vlans': "Interface mode does not support including tagged vlans"
+                'tagged_vlans': _("Interface mode does not support including tagged vlans")
             })
 
 
diff --git a/netbox/dcim/models/device_components.py b/netbox/dcim/models/device_components.py
index 101a1a45902..f38343f5dd4 100644
--- a/netbox/dcim/models/device_components.py
+++ b/netbox/dcim/models/device_components.py
@@ -894,7 +894,7 @@ def clean(self):
                 raise ValidationError({'rf_channel_width': _("Cannot specify custom width with channel selected.")})
 
         # VLAN validation
-        if self.mode is None and self.untagged_vlan:
+        if not self.mode and self.untagged_vlan:
             raise ValidationError({'untagged_vlan': _("Interface mode does not support including an untagged vlan.")})
 
         # Validate untagged VLAN

From 07444e6f1abf5fb2fcc212c81bffafd9ca055bd1 Mon Sep 17 00:00:00 2001
From: Daniel Sheppard <dans@dansheps.com>
Date: Tue, 27 Aug 2024 00:28:39 -0500
Subject: [PATCH 09/12] Re-ordering statements in validators

---
 .../api/serializers_/device_components.py     | 32 +++++++++---------
 netbox/dcim/forms/model_forms.py              | 33 ++++++++++---------
 2 files changed, 33 insertions(+), 32 deletions(-)

diff --git a/netbox/dcim/api/serializers_/device_components.py b/netbox/dcim/api/serializers_/device_components.py
index 16b68671bb6..468c7b4960d 100644
--- a/netbox/dcim/api/serializers_/device_components.py
+++ b/netbox/dcim/api/serializers_/device_components.py
@@ -243,22 +243,22 @@ def validate(self, data):
             mode = None
             tagged_vlans = []
 
-            if self.instance.pk and 'mode' in data.keys():
-                mode = data.get('mode') if 'mode' in data.keys() else self.instance.get('mode')
-            elif 'mode' in data.keys():
-                mode = data.get('mode')
-
-            if self.instance.pk and 'tagged_vlans' in data.keys():
-                tagged_vlans = data.get('tagged_vlans') if 'tagged_vlans' in data.keys() else \
-                    self.instance.tagged_vlans.all()
-            elif 'tagged_vlans' in data.keys():
-                tagged_vlans = data.get('tagged_vlans')
-
-            if self.instance.pk and 'untagged_vlan' in data.keys():
-                untagged_vlan = data.get('untagged_vlan') if 'untagged_vlan' in data.keys() else \
-                    self.instance.untagged_vlan
-            elif 'untagged_vlan' in data.keys():
-                untagged_vlan = data.get('untagged_vlan')
+            if self.instance:
+                if 'mode' in data.keys():
+                    mode = data.get('mode') if 'mode' in data.keys() else self.instance.get('mode')
+                if 'tagged_vlans' in data.keys():
+                    tagged_vlans = data.get('tagged_vlans') if 'tagged_vlans' in data.keys() else \
+                        self.instance.tagged_vlans.all()
+                if 'untagged_vlan' in data.keys():
+                    untagged_vlan = data.get('untagged_vlan') if 'untagged_vlan' in data.keys() else \
+                        self.instance.untagged_vlan
+            else:
+                if 'mode' in data.keys():
+                    mode = data.get('mode')
+                if 'tagged_vlans' in data.keys():
+                    tagged_vlans = data.get('tagged_vlans')
+                if 'untagged_vlan' in data.keys():
+                    untagged_vlan = data.get('untagged_vlan')
 
             if mode != InterfaceModeChoices.MODE_TAGGED and tagged_vlans:
                 raise serializers.ValidationError({
diff --git a/netbox/dcim/forms/model_forms.py b/netbox/dcim/forms/model_forms.py
index b26247a946f..95908bd419b 100644
--- a/netbox/dcim/forms/model_forms.py
+++ b/netbox/dcim/forms/model_forms.py
@@ -1368,22 +1368,23 @@ def clean(self):
         tagged_vlans = []
         untagged_vlan = None
 
-        if self.instance.pk and 'mode' in self.cleaned_data.keys():
-            mode = self.cleaned_data.get('mode') if 'mode' in self.cleaned_data.keys() else self.instance.get('mode')
-        elif 'mode' in self.cleaned_data.keys():
-            mode = self.cleaned_data.get('mode')
-
-        if self.instance.pk and 'tagged_vlans' in self.cleaned_data.keys():
-            tagged_vlans = self.cleaned_data.get('tagged_vlans') if 'tagged_vlans' in self.cleaned_data.keys() else\
-                self.instance.tagged_vlans.all()
-        elif 'tagged_vlans' in self.cleaned_data.keys():
-            tagged_vlans = self.cleaned_data.get('tagged_vlans')
-
-        if self.instance.pk and 'untagged_vlan' in self.cleaned_data.keys():
-            untagged_vlan = self.cleaned_data.get('untagged_vlan') if 'untagged_vlan' in self.cleaned_data.keys() else\
-                self.instance.untagged_vlan
-        elif 'untagged_vlan' in self.cleaned_data.keys():
-            untagged_vlan = self.cleaned_data.get('untagged_vlan')
+        if self.instance:
+            if 'mode' in self.cleaned_data.keys():
+                mode = self.cleaned_data.get('mode') if 'mode' in self.cleaned_data.keys() else\
+                    self.instance.get('mode')
+            if 'tagged_vlans' in self.cleaned_data.keys():
+                tagged_vlans = self.cleaned_data.get('tagged_vlans') if 'tagged_vlans' in self.cleaned_data.keys() else\
+                    self.instance.tagged_vlans.all()
+            if 'untagged_vlan' in self.cleaned_data.keys():
+                untagged_vlan = self.cleaned_data.get('untagged_vlan') if 'untagged_vlan' in self.cleaned_data.keys()\
+                    else self.instance.untagged_vlan
+        else:
+            if 'mode' in self.cleaned_data.keys():
+                mode = self.cleaned_data.get('mode')
+            if 'tagged_vlans' in self.cleaned_data.keys():
+                tagged_vlans = self.cleaned_data.get('tagged_vlans')
+            if 'untagged_vlan' in self.cleaned_data.keys():
+                untagged_vlan = self.cleaned_data.get('untagged_vlan')
 
         if mode != InterfaceModeChoices.MODE_TAGGED and tagged_vlans:
             raise forms.ValidationError({

From 5776db645079d2a07cfcead6a903fff027e20fd2 Mon Sep 17 00:00:00 2001
From: Daniel Sheppard <dans@dansheps.com>
Date: Tue, 27 Aug 2024 16:02:49 -0500
Subject: [PATCH 10/12] Forgot to call super().clean()

---
 netbox/dcim/forms/model_forms.py | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/netbox/dcim/forms/model_forms.py b/netbox/dcim/forms/model_forms.py
index 95908bd419b..fe686bd8850 100644
--- a/netbox/dcim/forms/model_forms.py
+++ b/netbox/dcim/forms/model_forms.py
@@ -1364,6 +1364,9 @@ class Meta:
         }
 
     def clean(self):
+        super().clean()
+
+        # Validate VLAN config
         mode = None
         tagged_vlans = []
         untagged_vlan = None

From 17898625c57a5e42ad9e8bef32d189b548141dbc Mon Sep 17 00:00:00 2001
From: Daniel Sheppard <dans@dansheps.com>
Date: Mon, 23 Sep 2024 17:17:41 -0500
Subject: [PATCH 11/12] Adjust logic for form and serializer.  Add tests

---
 .../api/serializers_/device_components.py     |  21 +--
 netbox/dcim/forms/common.py                   |   9 +-
 netbox/dcim/forms/model_forms.py              |  31 ----
 netbox/dcim/models/device_components.py       |   2 +-
 netbox/dcim/tests/test_api.py                 |  30 +++-
 netbox/dcim/tests/test_forms.py               | 143 +++++++++++++++++-
 6 files changed, 183 insertions(+), 53 deletions(-)

diff --git a/netbox/dcim/api/serializers_/device_components.py b/netbox/dcim/api/serializers_/device_components.py
index 867781995cd..d2f1954bd33 100644
--- a/netbox/dcim/api/serializers_/device_components.py
+++ b/netbox/dcim/api/serializers_/device_components.py
@@ -239,25 +239,16 @@ def validate(self, data):
             tagged_vlans = []
 
             if self.instance:
-                if 'mode' in data.keys():
-                    mode = data.get('mode') if 'mode' in data.keys() else self.instance.get('mode')
-                if 'tagged_vlans' in data.keys():
-                    tagged_vlans = data.get('tagged_vlans') if 'tagged_vlans' in data.keys() else \
-                        self.instance.tagged_vlans.all()
-                if 'untagged_vlan' in data.keys():
-                    untagged_vlan = data.get('untagged_vlan') if 'untagged_vlan' in data.keys() else \
-                        self.instance.untagged_vlan
+                mode = data.get('mode') if 'mode' in data.keys() else self.instance.mode
+                tagged_vlans = data.get('tagged_vlans') if 'tagged_vlans' in data.keys() else \
+                    self.instance.tagged_vlans.all()
             else:
-                if 'mode' in data.keys():
-                    mode = data.get('mode')
-                if 'tagged_vlans' in data.keys():
-                    tagged_vlans = data.get('tagged_vlans')
-                if 'untagged_vlan' in data.keys():
-                    untagged_vlan = data.get('untagged_vlan')
+                mode = data.get('mode', None)
+                tagged_vlans = data.get('tagged_vlans') if 'tagged_vlans' in data.keys() else None
 
             if mode != InterfaceModeChoices.MODE_TAGGED and tagged_vlans:
                 raise serializers.ValidationError({
-                    'tagged_vlans': _("Interface mode does not support including tagged vlans")
+                    'tagged_vlans': _("Interface mode does not support tagged vlans")
                 })
 
             # Validate many-to-many VLAN assignments
diff --git a/netbox/dcim/forms/common.py b/netbox/dcim/forms/common.py
index 4341ec04110..f943fc49b1f 100644
--- a/netbox/dcim/forms/common.py
+++ b/netbox/dcim/forms/common.py
@@ -42,7 +42,8 @@ def clean(self):
         super().clean()
 
         parent_field = 'device' if 'device' in self.cleaned_data else 'virtual_machine'
-        tagged_vlans = self.cleaned_data.get('tagged_vlans')
+        interface_mode = get_field_value(self, parent_field)
+        tagged_vlans = get_field_value(self, 'tagged_vlans') if 'tagged_vlans' in self.fields.keys() else []
 
         # Untagged interfaces cannot be assigned tagged VLANs
         if self.cleaned_data['mode'] == InterfaceModeChoices.MODE_ACCESS and tagged_vlans:
@@ -51,8 +52,10 @@ def clean(self):
             })
 
         # Remove all tagged VLAN assignments from "tagged all" interfaces
-        elif self.cleaned_data['mode'] == InterfaceModeChoices.MODE_TAGGED_ALL:
-            self.cleaned_data['tagged_vlans'] = []
+        elif self.cleaned_data['mode'] == InterfaceModeChoices.MODE_TAGGED_ALL and tagged_vlans:
+            raise forms.ValidationError({
+                'mode': _("An tagged-all interface cannot have tagged VLANs assigned.")
+            })
 
         # Validate tagged VLANs; must be a global VLAN or in the same site
         elif self.cleaned_data['mode'] == InterfaceModeChoices.MODE_TAGGED and tagged_vlans:
diff --git a/netbox/dcim/forms/model_forms.py b/netbox/dcim/forms/model_forms.py
index 7ef655278e4..7dc6d52397e 100644
--- a/netbox/dcim/forms/model_forms.py
+++ b/netbox/dcim/forms/model_forms.py
@@ -1415,37 +1415,6 @@ class Meta:
             'mode': '802.1Q Mode',
         }
 
-    def clean(self):
-        super().clean()
-
-        # Validate VLAN config
-        mode = None
-        tagged_vlans = []
-        untagged_vlan = None
-
-        if self.instance:
-            if 'mode' in self.cleaned_data.keys():
-                mode = self.cleaned_data.get('mode') if 'mode' in self.cleaned_data.keys() else\
-                    self.instance.get('mode')
-            if 'tagged_vlans' in self.cleaned_data.keys():
-                tagged_vlans = self.cleaned_data.get('tagged_vlans') if 'tagged_vlans' in self.cleaned_data.keys() else\
-                    self.instance.tagged_vlans.all()
-            if 'untagged_vlan' in self.cleaned_data.keys():
-                untagged_vlan = self.cleaned_data.get('untagged_vlan') if 'untagged_vlan' in self.cleaned_data.keys()\
-                    else self.instance.untagged_vlan
-        else:
-            if 'mode' in self.cleaned_data.keys():
-                mode = self.cleaned_data.get('mode')
-            if 'tagged_vlans' in self.cleaned_data.keys():
-                tagged_vlans = self.cleaned_data.get('tagged_vlans')
-            if 'untagged_vlan' in self.cleaned_data.keys():
-                untagged_vlan = self.cleaned_data.get('untagged_vlan')
-
-        if mode != InterfaceModeChoices.MODE_TAGGED and tagged_vlans:
-            raise forms.ValidationError({
-                'tagged_vlans': _("Interface mode does not support including tagged vlans")
-            })
-
 
 class FrontPortForm(ModularDeviceComponentForm):
     rear_port = DynamicModelChoiceField(
diff --git a/netbox/dcim/models/device_components.py b/netbox/dcim/models/device_components.py
index 445a36be528..d3b8ef6bf2d 100644
--- a/netbox/dcim/models/device_components.py
+++ b/netbox/dcim/models/device_components.py
@@ -895,7 +895,7 @@ def clean(self):
 
         # VLAN validation
         if not self.mode and self.untagged_vlan:
-            raise ValidationError({'untagged_vlan': _("Interface mode does not support including an untagged vlan.")})
+            raise ValidationError({'untagged_vlan': _("Interface mode does not support an untagged vlan.")})
 
         # Validate untagged VLAN
         if self.untagged_vlan and self.untagged_vlan.site not in [self.device.site, None]:
diff --git a/netbox/dcim/tests/test_api.py b/netbox/dcim/tests/test_api.py
index 573fdbb9673..6397d350225 100644
--- a/netbox/dcim/tests/test_api.py
+++ b/netbox/dcim/tests/test_api.py
@@ -1,3 +1,5 @@
+import json
+
 from django.test import override_settings
 from django.urls import reverse
 from django.utils.translation import gettext as _
@@ -11,7 +13,7 @@
 from netbox.api.serializers import GenericObjectSerializer
 from tenancy.models import Tenant
 from users.models import User
-from utilities.testing import APITestCase, APIViewTestCases, create_test_device
+from utilities.testing import APITestCase, APIViewTestCases, create_test_device, disable_warnings
 from virtualization.models import Cluster, ClusterType
 from wireless.choices import WirelessChannelChoices
 from wireless.models import WirelessLAN
@@ -1718,6 +1720,32 @@ def test_bulk_delete_child_interfaces(self):
         self.client.delete(self._get_list_url(), data, format='json', **self.header)
         self.assertEqual(device.interfaces.count(), 2)  # Child & parent were both deleted
 
+    def test_create_child_interfaces_mode_invalid_data(self):
+        """
+        POST a single object without permission.
+        """
+        self.add_permissions('dcim.add_interface')
+        url = self._get_list_url()
+        initial_count = self._get_queryset().count()
+
+        device = Device.objects.first()
+        vlans = VLAN.objects.all()[0:3]
+
+        create_data = {
+            'device': device.pk,
+            'name': 'Untagged Interface 1',
+            'type': InterfaceTypeChoices.TYPE_1GE_FIXED,
+            'mode': InterfaceModeChoices.MODE_ACCESS,
+            'tagged_vlans': [vlans[0].pk, vlans[1].pk],
+            'untagged_vlan': vlans[2].pk,
+        }
+
+        response = self.client.post(self._get_list_url(), create_data, format='json', **self.header)
+        self.assertHttpStatus(response, status.HTTP_400_BAD_REQUEST)
+        content = json.loads(response.content)
+        self.assertIn('tagged_vlans', content)
+        self.assertIsNone(content.get('data'))
+
 
 class FrontPortTest(APIViewTestCases.APIViewTestCase):
     model = FrontPort
diff --git a/netbox/dcim/tests/test_forms.py b/netbox/dcim/tests/test_forms.py
index 7a57bf3f0b9..57056bcb95e 100644
--- a/netbox/dcim/tests/test_forms.py
+++ b/netbox/dcim/tests/test_forms.py
@@ -1,8 +1,9 @@
 from django.test import TestCase
 
-from dcim.choices import DeviceFaceChoices, DeviceStatusChoices, InterfaceTypeChoices
+from dcim.choices import DeviceFaceChoices, DeviceStatusChoices, InterfaceTypeChoices, InterfaceModeChoices
 from dcim.forms import *
 from dcim.models import *
+from ipam.models import VLAN
 from utilities.testing import create_test_device
 from virtualization.models import Cluster, ClusterGroup, ClusterType
 
@@ -117,11 +118,23 @@ def test_non_racked_device_with_position(self):
         self.assertIn('position', form.errors)
 
 
-class LabelTestCase(TestCase):
+class InterfaceTestCase(TestCase):
 
     @classmethod
     def setUpTestData(cls):
         cls.device = create_test_device('Device 1')
+        cls.vlans = (
+            VLAN(name='VLAN 1', vid=1),
+            VLAN(name='VLAN 2', vid=2),
+            VLAN(name='VLAN 3', vid=3),
+        )
+        VLAN.objects.bulk_create(cls.vlans)
+        cls.interface = Interface.objects.create(
+            device=cls.device,
+            name='Interface 1',
+            type=InterfaceTypeChoices.TYPE_1GE_GBIC,
+            mode=InterfaceModeChoices.MODE_TAGGED,
+        )
 
     def test_interface_label_count_valid(self):
         """
@@ -151,3 +164,129 @@ def test_interface_label_count_mismatch(self):
 
         self.assertFalse(form.is_valid())
         self.assertIn('label', form.errors)
+
+    def test_create_interface_mode_valid_data(self):
+        """
+        Test that saving valid interface mode and tagged/untagged vlans works properly
+        """
+
+        # Validate access mode
+        data = {
+            'device': self.device.pk,
+            'name': 'ethernet1/1',
+            'type': InterfaceTypeChoices.TYPE_1GE_GBIC,
+            'mode': InterfaceModeChoices.MODE_ACCESS,
+            'untagged_vlan': self.vlans[0].pk
+        }
+        form = InterfaceCreateForm(data)
+
+        self.assertTrue(form.is_valid())
+
+        # Validate tagged vlans
+        data = {
+            'device': self.device.pk,
+            'name': 'ethernet1/2',
+            'type': InterfaceTypeChoices.TYPE_1GE_GBIC,
+            'mode': InterfaceModeChoices.MODE_TAGGED,
+            'untagged_vlan': self.vlans[0].pk,
+            'tagged_vlans': [self.vlans[1].pk, self.vlans[2].pk]
+        }
+        form = InterfaceCreateForm(data)
+        self.assertTrue(form.is_valid())
+
+        # Validate tagged vlans
+        data = {
+            'device': self.device.pk,
+            'name': 'ethernet1/3',
+            'type': InterfaceTypeChoices.TYPE_1GE_GBIC,
+            'mode': InterfaceModeChoices.MODE_TAGGED_ALL,
+            'untagged_vlan': self.vlans[0].pk,
+        }
+        form = InterfaceCreateForm(data)
+        self.assertTrue(form.is_valid())
+
+    def test_create_interface_mode_access_invalid_data(self):
+        """
+        Test that saving invalid interface mode and tagged/untagged vlans works properly
+        """
+        data = {
+            'device': self.device.pk,
+            'name': 'ethernet1/4',
+            'type': InterfaceTypeChoices.TYPE_1GE_GBIC,
+            'mode': InterfaceModeChoices.MODE_ACCESS,
+            'untagged_vlan': self.vlans[0].pk,
+            'tagged_vlans': [self.vlans[1].pk, self.vlans[2].pk]
+        }
+        form = InterfaceCreateForm(data)
+
+        self.assertTrue(form.is_valid())
+
+    def test_edit_interface_mode_access_invalid_data(self):
+        """
+        Test that saving invalid interface mode and tagged/untagged vlans works properly
+        """
+        data = {
+            'device': self.device.pk,
+            'name': 'Ethernet 1/5',
+            'type': InterfaceTypeChoices.TYPE_1GE_GBIC,
+            'mode': InterfaceModeChoices.MODE_ACCESS,
+            'tagged_vlans': [self.vlans[0].pk, self.vlans[1].pk, self.vlans[2].pk]
+        }
+        form = InterfaceForm(data, instance=self.interface)
+
+        self.assertTrue(form.is_valid())
+
+    def test_create_interface_mode_tagged_all_invalid_data(self):
+        """
+        Test that saving invalid interface mode and tagged/untagged vlans works properly
+        """
+        data = {
+            'device': self.device.pk,
+            'name': 'ethernet1/6',
+            'type': InterfaceTypeChoices.TYPE_1GE_GBIC,
+            'mode': InterfaceModeChoices.MODE_TAGGED_ALL,
+            'tagged_vlans': [self.vlans[0].pk, self.vlans[1].pk, self.vlans[2]]
+        }
+        form = InterfaceCreateForm(data)
+
+        self.assertTrue(form.is_valid())
+
+    def test_edit_interface_mode_tagged_all_invalid_data(self):
+        """
+        Test that saving invalid interface mode and tagged/untagged vlans works properly
+        """
+        data = {
+            'device': self.device.pk,
+            'name': 'Ethernet 1/7',
+            'type': InterfaceTypeChoices.TYPE_1GE_GBIC,
+            'mode': InterfaceModeChoices.MODE_TAGGED_ALL,
+            'tagged_vlans': [self.vlans[0].pk, self.vlans[1].pk, self.vlans[2]]
+        }
+        form = InterfaceForm(data, instance=self.interface)
+        form.is_valid()
+        self.assertTrue(form.is_valid())
+
+    def test_edit_interface_mode_tagged_all_existing_invalid_data(self):
+        """
+        Test that saving invalid interface mode and tagged/untagged vlans works properly
+        """
+        self.interface.tagged_vlans.add(self.vlans[0])
+        self.interface.tagged_vlans.add(self.vlans[1])
+        data = {
+            'device': self.device.pk,
+            'name': 'Ethernet 1/8',
+            'type': InterfaceTypeChoices.TYPE_1GE_GBIC,
+            'mode': InterfaceModeChoices.MODE_TAGGED_ALL,
+        }
+        form = InterfaceForm(data, instance=self.interface)
+        self.assertFalse(form.is_valid())
+
+        data = {
+            'device': self.device.pk,
+            'name': 'Ethernet 1/9',
+            'type': InterfaceTypeChoices.TYPE_1GE_GBIC,
+            'mode': InterfaceModeChoices.MODE_ACCESS,
+        }
+        form = InterfaceForm(data, instance=self.interface)
+        self.assertFalse(form.is_valid())
+        self.interface.tagged_vlans.clear()

From 365e2fbbf7c96deb30d22bcfc9c91e559484732d Mon Sep 17 00:00:00 2001
From: Daniel Sheppard <dans@dansheps.com>
Date: Wed, 16 Oct 2024 23:27:21 -0500
Subject: [PATCH 12/12] Fix test failure

---
 netbox/dcim/forms/common.py | 6 +++++-
 1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/netbox/dcim/forms/common.py b/netbox/dcim/forms/common.py
index f943fc49b1f..36a0e81cac8 100644
--- a/netbox/dcim/forms/common.py
+++ b/netbox/dcim/forms/common.py
@@ -43,7 +43,11 @@ def clean(self):
 
         parent_field = 'device' if 'device' in self.cleaned_data else 'virtual_machine'
         interface_mode = get_field_value(self, parent_field)
-        tagged_vlans = get_field_value(self, 'tagged_vlans') if 'tagged_vlans' in self.fields.keys() else []
+        if 'tagged_vlans' in self.fields.keys():
+            tagged_vlans = self.cleaned_data.get('tagged_vlans') if self.is_bound else \
+                self.get_initial_for_field(self.fields['tagged_vlans'], 'tagged_vlans')
+        else:
+            tagged_vlans = []
 
         # Untagged interfaces cannot be assigned tagged VLANs
         if self.cleaned_data['mode'] == InterfaceModeChoices.MODE_ACCESS and tagged_vlans: