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
This commit is contained in:
parent
b905e99e63
commit
86ea67d640
|
|
@ -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
|
||||
|
|
|
|||
|
|
@ -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():
|
||||
|
|
|
|||
|
|
@ -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)
|
||||
|
|
|
|||
|
|
@ -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)
|
||||
|
|
|
|||
|
|
@ -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):
|
||||
|
||||
|
|
|
|||
|
|
@ -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,
|
||||
|
|
|
|||
|
|
@ -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
|
||||
|
|
|
|||
|
|
@ -45,4 +45,9 @@
|
|||
</div>
|
||||
</div>
|
||||
</div>
|
||||
|
||||
{# Meta fields #}
|
||||
<div class="bg-primary-subtle border border-primary rounded-1 pt-3 px-3 mb-3">
|
||||
{% render_field form.changelog_message %}
|
||||
</div>
|
||||
{% endblock %}
|
||||
|
|
|
|||
Loading…
Reference in New Issue