From 86ea67d6400aa1300d00b047090c8b7d54bb2dac Mon Sep 17 00:00:00 2001 From: Martin Hauser Date: Thu, 4 Jun 2026 19:57:15 +0200 Subject: [PATCH] fix(extras): Prevent direct access to TableConfig create view Add GET handler to TableConfigEditView that redirects users to home with a warning if they attempt to access the create form directly without required object_type and table parameters from a source list view. Fixes #22237 --- netbox/extras/forms/bulk_edit.py | 2 +- netbox/extras/forms/model_forms.py | 23 ++++++++- netbox/extras/models/models.py | 6 ++- netbox/extras/tests/test_forms.py | 51 ++++++++++++++++++- netbox/extras/tests/test_models.py | 20 ++++++++ netbox/extras/tests/test_views.py | 49 ++++++++++++++++-- netbox/extras/views.py | 8 +++ netbox/templates/extras/tableconfig_edit.html | 5 ++ 8 files changed, 156 insertions(+), 8 deletions(-) diff --git a/netbox/extras/forms/bulk_edit.py b/netbox/extras/forms/bulk_edit.py index 9fecfdb7a..abfe22f5a 100644 --- a/netbox/extras/forms/bulk_edit.py +++ b/netbox/extras/forms/bulk_edit.py @@ -212,7 +212,7 @@ class SavedFilterBulkEditForm(ChangelogMessageMixin, OwnerMixin, BulkEditForm): nullable_fields = ('description',) -class TableConfigBulkEditForm(BulkEditForm): +class TableConfigBulkEditForm(ChangelogMessageMixin, BulkEditForm): pk = forms.ModelMultipleChoiceField( queryset=TableConfig.objects.all(), widget=forms.MultipleHiddenInput diff --git a/netbox/extras/forms/model_forms.py b/netbox/extras/forms/model_forms.py index f1b23e0d5..ba995c701 100644 --- a/netbox/extras/forms/model_forms.py +++ b/netbox/extras/forms/model_forms.py @@ -402,7 +402,7 @@ class SavedFilterForm(ChangelogMessageMixin, OwnerMixin, forms.ModelForm): super().__init__(*args, initial=initial, **kwargs) -class TableConfigForm(forms.ModelForm): +class TableConfigForm(ChangelogMessageMixin, forms.ModelForm): object_type = ContentTypeChoiceField( label=_('Object type'), queryset=ObjectType.objects.all() @@ -438,10 +438,29 @@ class TableConfigForm(forms.ModelForm): def __init__(self, data=None, *args, **kwargs): super().__init__(data, *args, **kwargs) - object_type = ObjectType.objects.get(pk=get_field_value(self, 'object_type')) + self.fields['available_columns'].widget.choices = () + self.fields['columns'].widget.choices = () + + # Table context may be absent e.g. when the add view is requested directly + object_type_pk = get_field_value(self, 'object_type') + object_type_pk = getattr(object_type_pk, 'pk', object_type_pk) + if not object_type_pk: + return + + try: + object_type = ObjectType.objects.get(pk=object_type_pk) + except (ObjectType.DoesNotExist, TypeError, ValueError): + return + model = object_type.model_class() + if model is None: + return + table_name = get_field_value(self, 'table') table_class = get_table_for_model(model, table_name) + if table_class is None: + return + table = table_class([]) if columns := self._get_columns(): diff --git a/netbox/extras/models/models.py b/netbox/extras/models/models.py index 14cae8d9d..d538f94bf 100644 --- a/netbox/extras/models/models.py +++ b/netbox/extras/models/models.py @@ -649,6 +649,10 @@ class TableConfig(CloningMixin, ChangeLoggedModel): def clean(self): super().clean() + # Skip table validation until the object type and table have been set + if not self.object_type_id or not self.table: + return + # Validate table if self.table_class is None: raise ValidationError({ @@ -667,7 +671,7 @@ class TableConfig(CloningMixin, ChangeLoggedModel): }) # Validate selected columns - for name in self.columns: + for name in self.columns or []: if name not in table.columns: raise ValidationError({ 'columns': _('Unknown column: {name}').format(name=name) diff --git a/netbox/extras/tests/test_forms.py b/netbox/extras/tests/test_forms.py index 26de1de80..909d72801 100644 --- a/netbox/extras/tests/test_forms.py +++ b/netbox/extras/tests/test_forms.py @@ -10,7 +10,7 @@ from core.models import DataSource, ObjectType from dcim.forms import SiteForm from dcim.models import Site from extras.choices import CustomFieldTypeChoices -from extras.forms import SavedFilterForm +from extras.forms import SavedFilterForm, TableConfigBulkEditForm, TableConfigForm from extras.forms.model_forms import CustomFieldChoiceSetForm from extras.forms.scripts import ScriptFileForm from extras.models import CustomField, CustomFieldChoiceSet, ScriptModule @@ -288,3 +288,52 @@ class ScriptFileFormTestCase(TestCase): form = ScriptFileForm(files={'upload_file': upload_file}, instance=self._new_module()) self.assertTrue(form.is_valid()) + + +class TableConfigFormTestCase(TestCase): + + def test_form_without_table_context(self): + """The form must be constructible without an object type.""" + form = TableConfigForm() + self.assertEqual(list(form.fields['available_columns'].widget.choices), []) + self.assertEqual(list(form.fields['columns'].widget.choices), []) + + def test_form_with_invalid_object_type(self): + """An unknown object type must yield empty column choices.""" + last_pk = ObjectType.objects.order_by('pk').last().pk + form = TableConfigForm(initial={'object_type': last_pk + 1}) + self.assertEqual(list(form.fields['available_columns'].widget.choices), []) + self.assertEqual(list(form.fields['columns'].widget.choices), []) + + def test_form_with_unknown_table(self): + """An unresolvable table name must yield empty column choices.""" + object_type = ObjectType.objects.get_for_model(Site) + form = TableConfigForm(initial={'object_type': object_type.pk, 'table': 'NoSuchTable'}) + self.assertEqual(list(form.fields['columns'].widget.choices), []) + + def test_form_with_table_context(self): + """Column choices must be populated from the resolved table.""" + object_type = ObjectType.objects.get_for_model(Site) + form = TableConfigForm(initial={ + 'object_type': object_type.pk, + 'table': 'SiteTable', + 'columns': ['name', 'status'], + }) + self.assertEqual( + [name for name, _ in form.fields['columns'].widget.choices], + ['name', 'status'] + ) + self.assertIn('region', dict(form.fields['available_columns'].widget.choices)) + + def test_form_includes_changelog_message(self): + """The model form must expose the changelog_message meta field.""" + object_type = ObjectType.objects.get_for_model(Site) + form = TableConfigForm(initial={'object_type': object_type.pk, 'table': 'SiteTable'}) + self.assertIn('changelog_message', form.fields) + self.assertIn('changelog_message', form.meta_fields) + + def test_bulk_edit_form_includes_changelog_message(self): + """The bulk edit form must expose the changelog_message meta field.""" + form = TableConfigBulkEditForm() + self.assertIn('changelog_message', form.fields) + self.assertIn('changelog_message', form.meta_fields) diff --git a/netbox/extras/tests/test_models.py b/netbox/extras/tests/test_models.py index d827c641b..a3945f282 100644 --- a/netbox/extras/tests/test_models.py +++ b/netbox/extras/tests/test_models.py @@ -210,6 +210,26 @@ class TableConfigTestCase(TestCase): # Must not raise TypeError: 'NoneType' object is not iterable tc.full_clean() + def test_clean_without_object_type(self): + """full_clean() on an instance missing its object type must raise ValidationError.""" + tc = TableConfig( + table=self.table_name, + name='No object type', + columns=['name'], + ) + with self.assertRaises(ValidationError): + tc.full_clean() + + def test_clean_accepts_columns_none(self): + """full_clean() must report missing columns rather than raise TypeError.""" + tc = TableConfig( + object_type=self.site_ct, + table=self.table_name, + name='No columns', + ) + with self.assertRaises(ValidationError): + tc.full_clean() + class TagTestCase(TestCase): diff --git a/netbox/extras/tests/test_views.py b/netbox/extras/tests/test_views.py index 74c88cea8..f97b3581b 100644 --- a/netbox/extras/tests/test_views.py +++ b/netbox/extras/tests/test_views.py @@ -2,6 +2,7 @@ import uuid from unittest.mock import PropertyMock, patch from django.contrib.contenttypes.models import ContentType +from django.contrib.messages import get_messages from django.test import tag from django.urls import reverse @@ -277,13 +278,16 @@ class SavedFilterTestCase(ViewTestCases.PrimaryObjectViewTestCase): class TableConfigTestCase( ViewTestCases.GetObjectViewTestCase, ViewTestCases.GetObjectChangelogViewTestCase, - ViewTestCases.ListObjectsViewTestCase, + ViewTestCases.CreateObjectViewTestCase, + ViewTestCases.EditObjectViewTestCase, ViewTestCases.DeleteObjectViewTestCase, + ViewTestCases.ListObjectsViewTestCase, + ViewTestCases.BulkEditObjectsViewTestCase, ViewTestCases.BulkDeleteObjectsViewTestCase, ): - # Add/Edit/BulkEdit views require an object_type pre-context from the source - # table view, so they are not exercised here. model = TableConfig + # Selected columns are POSTed as a list but compared as a CSV string + validation_excluded_fields = ('columns',) @classmethod def setUpTestData(cls): @@ -320,6 +324,45 @@ class TableConfigTestCase( ) TableConfig.objects.bulk_create(table_configs) + cls.form_data = { + 'name': 'Table Config X', + 'object_type': site_type.pk, + 'table': 'SiteTable', + 'description': 'A table config', + 'weight': 100, + 'enabled': True, + 'shared': True, + 'columns': ['name', 'status'], + 'ordering': 'name', + } + cls.bulk_edit_data = { + 'weight': 999, + } + + def _get_url(self, action, instance=None): + url = super()._get_url(action, instance) + # The add view requires the table context from the source table view + if action == 'add': + site_type = ObjectType.objects.get_for_model(Site) + url = f'{url}?object_type={site_type.pk}&table=SiteTable' + return url + + def test_add_view_without_table_context(self): + """A GET without the table context params must redirect to the home page.""" + self.add_permissions('extras.add_tableconfig') + response = self.client.get(reverse('extras:tableconfig_add')) + self.assertRedirects(response, reverse('home')) + + messages_list = list(get_messages(response.wsgi_request)) + self.assertEqual(len(messages_list), 1) + self.assertEqual(str(messages_list[0]), 'Table configurations must be created from an object list view.') + + def test_add_view_post_without_table_context(self): + """A POST without the table context must return form errors rather than a server error.""" + self.add_permissions('extras.add_tableconfig') + response = self.client.post(reverse('extras:tableconfig_add'), data={}) + self.assertHttpStatus(response, 200) + class BookmarkTestCase( ViewTestCases.DeleteObjectViewTestCase, diff --git a/netbox/extras/views.py b/netbox/extras/views.py index 344830e67..cec119378 100644 --- a/netbox/extras/views.py +++ b/netbox/extras/views.py @@ -481,6 +481,14 @@ class TableConfigEditView(SharedObjectViewMixin, generic.ObjectEditView): form = forms.TableConfigForm template_name = 'extras/tableconfig_edit.html' + def get(self, request, *args, **kwargs): + # The add view requires the object_type & table parameters from the source table view + if not kwargs and not (request.GET.get('object_type') and request.GET.get('table')): + messages.warning(request, _('Table configurations must be created from an object list view.')) + return redirect('home') + + return super().get(request, *args, **kwargs) + def alter_object(self, obj, request, url_args, url_kwargs): if not obj.pk: obj.user = request.user diff --git a/netbox/templates/extras/tableconfig_edit.html b/netbox/templates/extras/tableconfig_edit.html index 31057c298..afd37d72c 100644 --- a/netbox/templates/extras/tableconfig_edit.html +++ b/netbox/templates/extras/tableconfig_edit.html @@ -45,4 +45,9 @@ + + {# Meta fields #} +
+ {% render_field form.changelog_message %} +
{% endblock %}