feat(forms): Preserve restricted related values on edit forms
Add read-only preservation of related object values hidden by object permissions. Single-value fields are disabled entirely; multi-value fields remain editable for visible values while restricted current values appear as disabled options and are merged back on save. Fixes data loss when users edit objects with related values they cannot view, such as tags or tenants constrained by permissions.
This commit is contained in:
Martin Hauser
parent
158f6846c3
commit
b81bf3eea1
|
|
@ -113,3 +113,7 @@ Device.objects.filter(
|
|||
### Creating and Modifying Objects
|
||||
|
||||
The same sort of logic is in play when a user attempts to create or modify an object in NetBox, with a twist. Once validation has completed, NetBox starts an atomic database transaction to facilitate the change, and the object is created or saved normally. Next, still within the transaction, NetBox issues a second query to retrieve the newly created/updated object, filtering the restricted queryset with the object's primary key. If this query fails to return the object, NetBox knows that the new revision does not match the constraints imposed by the permission. The transaction is then rolled back, leaving the database in its original state prior to the change, and the user is informed of the violation.
|
||||
|
||||
#### Preserving Restricted Related Values
|
||||
|
||||
When editing an object that references related objects the user is not permitted to view (for example a tag, tenant, or custom field value constrained away by a permission), those current values are shown read-only on the edit form and preserved when the form is saved. Single-value fields are disabled entirely; multi-value fields (such as tags) remain editable for the values the user can see, while the restricted current values are rendered as disabled options. This prevents a user from unintentionally clearing related objects they cannot see, without exposing any other restricted objects or allowing the restricted values to be replaced.
|
||||
|
|
|
|||
|
|
@ -213,6 +213,10 @@ class CircuitTerminationForm(NetBoxModelForm):
|
|||
FieldSet('port_speed', 'upstream_speed', 'xconnect_id', 'pp_info', name=_('Termination Details')),
|
||||
)
|
||||
|
||||
restricted_related_selectors = {
|
||||
'termination': {'path': 'termination', 'lock_fields': ('termination_type',)},
|
||||
}
|
||||
|
||||
class Meta:
|
||||
model = CircuitTermination
|
||||
fields = [
|
||||
|
|
@ -307,6 +311,10 @@ class CircuitGroupAssignmentForm(NetBoxModelForm):
|
|||
FieldSet('group', 'member_type', 'member', 'priority', 'tags', name=_('Group Assignment')),
|
||||
)
|
||||
|
||||
restricted_related_selectors = {
|
||||
'member': {'path': 'member', 'lock_fields': ('member_type',)},
|
||||
}
|
||||
|
||||
class Meta:
|
||||
model = CircuitGroupAssignment
|
||||
fields = [
|
||||
|
|
|
|||
|
|
@ -38,6 +38,11 @@ class ScopedForm(forms.Form):
|
|||
selector=True
|
||||
)
|
||||
|
||||
# Duplicated by ipam.VLANGroupForm, which needs broader scope_type choices and cannot inherit ScopedForm.
|
||||
restricted_related_selectors = {
|
||||
'scope': {'path': 'scope', 'lock_fields': ('scope_type',)},
|
||||
}
|
||||
|
||||
def __init__(self, *args, **kwargs):
|
||||
instance = kwargs.get('instance')
|
||||
initial = kwargs.get('initial', {})
|
||||
|
|
|
|||
|
|
@ -12,7 +12,7 @@ from extras.models import ConfigTemplate
|
|||
from ipam.choices import VLANQinQRoleChoices
|
||||
from ipam.models import ASN, VLAN, VRF, IPAddress, VLANGroup, VLANTranslationPolicy
|
||||
from netbox.forms import NestedGroupModelForm, NetBoxModelForm, OrganizationalModelForm, PrimaryModelForm
|
||||
from netbox.forms.mixins import ChangelogMessageMixin, OwnerMixin
|
||||
from netbox.forms.mixins import ChangelogMessageMixin, OwnerMixin, RestrictedRelatedFieldsMixin
|
||||
from tenancy.forms import TenancyForm
|
||||
from users.models import User
|
||||
from utilities.forms import add_blank_choice, get_field_value
|
||||
|
|
@ -1043,7 +1043,7 @@ class VCMemberSelectForm(forms.Form):
|
|||
# Device component templates
|
||||
#
|
||||
|
||||
class ComponentTemplateForm(ChangelogMessageMixin, forms.ModelForm):
|
||||
class ComponentTemplateForm(RestrictedRelatedFieldsMixin, ChangelogMessageMixin, forms.ModelForm):
|
||||
device_type = DynamicModelChoiceField(
|
||||
label=_('Device type'),
|
||||
queryset=DeviceType.objects.all(),
|
||||
|
|
@ -1059,6 +1059,11 @@ class ComponentTemplateForm(ChangelogMessageMixin, forms.ModelForm):
|
|||
if self.instance.pk:
|
||||
self.fields['device_type'].disabled = True
|
||||
|
||||
def clean(self):
|
||||
# Merge restricted current members hidden from the user back into multi-value fields so they survive on save.
|
||||
self._merge_restricted_preserved_members()
|
||||
return super().clean()
|
||||
|
||||
|
||||
class ModularComponentTemplateForm(ComponentTemplateForm):
|
||||
device_type = DynamicModelChoiceField(
|
||||
|
|
@ -1363,6 +1368,17 @@ class InventoryItemTemplateForm(ComponentTemplateForm):
|
|||
label=_('Rear port template')
|
||||
)
|
||||
|
||||
# Sibling selectors stored as the component GenericForeignKey; the model picks the matching field.
|
||||
restricted_related_selectors = {
|
||||
'consoleporttemplate': {'path': 'component', 'model': ConsolePortTemplate},
|
||||
'consoleserverporttemplate': {'path': 'component', 'model': ConsoleServerPortTemplate},
|
||||
'frontporttemplate': {'path': 'component', 'model': FrontPortTemplate},
|
||||
'interfacetemplate': {'path': 'component', 'model': InterfaceTemplate},
|
||||
'poweroutlettemplate': {'path': 'component', 'model': PowerOutletTemplate},
|
||||
'powerporttemplate': {'path': 'component', 'model': PowerPortTemplate},
|
||||
'rearporttemplate': {'path': 'component', 'model': RearPortTemplate},
|
||||
}
|
||||
|
||||
fieldsets = (
|
||||
FieldSet(
|
||||
'device_type', 'parent', 'name', 'label', 'role', 'manufacturer', 'part_id', 'description',
|
||||
|
|
@ -1846,6 +1862,17 @@ class InventoryItemForm(DeviceComponentForm):
|
|||
label=_('Rear port')
|
||||
)
|
||||
|
||||
# Sibling selectors stored as the component GenericForeignKey; the model picks the matching field.
|
||||
restricted_related_selectors = {
|
||||
'consoleport': {'path': 'component', 'model': ConsolePort},
|
||||
'consoleserverport': {'path': 'component', 'model': ConsoleServerPort},
|
||||
'frontport': {'path': 'component', 'model': FrontPort},
|
||||
'interface': {'path': 'component', 'model': Interface},
|
||||
'poweroutlet': {'path': 'component', 'model': PowerOutlet},
|
||||
'powerport': {'path': 'component', 'model': PowerPort},
|
||||
'rearport': {'path': 'component', 'model': RearPort},
|
||||
}
|
||||
|
||||
fieldsets = (
|
||||
FieldSet(
|
||||
'device', 'parent', 'name', 'label', 'status', 'role', 'description', 'tags',
|
||||
|
|
@ -2009,6 +2036,12 @@ class MACAddressForm(PrimaryModelForm):
|
|||
),
|
||||
)
|
||||
|
||||
restricted_related_selectors = {
|
||||
# The selectors are stored as the assigned_object GenericForeignKey; the model picks the matching one.
|
||||
'interface': {'path': 'assigned_object', 'model': Interface},
|
||||
'vminterface': {'path': 'assigned_object', 'model': VMInterface},
|
||||
}
|
||||
|
||||
class Meta:
|
||||
model = MACAddress
|
||||
fields = [
|
||||
|
|
|
|||
|
|
@ -16,7 +16,7 @@ from dcim.models import *
|
|||
from ipam.models import ASN, RIR, VLAN
|
||||
from utilities.exceptions import AbortRequest
|
||||
from utilities.forms.rendering import M2MAddRemoveFields
|
||||
from utilities.testing import create_test_device
|
||||
from utilities.testing import create_test_device, simulate_restrict
|
||||
from virtualization.models import Cluster, ClusterGroup, ClusterType
|
||||
|
||||
|
||||
|
|
@ -666,3 +666,46 @@ class SiteFormTestCase(TestCase):
|
|||
self.assertEqual(site.asns.count(), M2MAddRemoveFields.THRESHOLD)
|
||||
self.assertTrue(site.asns.filter(pk__in=add_pks).count() == 3)
|
||||
self.assertFalse(site.asns.filter(pk__in=remove_pks).exists())
|
||||
|
||||
|
||||
class RestrictedComponentTemplateFormTest(TestCase):
|
||||
"""Component-template forms (not NetBoxModelForms) preserve hidden FK selectors via the mixin on the base."""
|
||||
|
||||
@classmethod
|
||||
def setUpTestData(cls):
|
||||
manufacturer = Manufacturer.objects.create(name='Manufacturer 1', slug='manufacturer-1')
|
||||
cls.device_type = DeviceType.objects.create(manufacturer=manufacturer, model='Model 1', slug='model-1')
|
||||
|
||||
def test_poweroutlettemplate_hidden_power_port_is_preserved(self):
|
||||
"""Editing a power outlet template whose power_port is hidden preserves it on save."""
|
||||
pp = PowerPortTemplate.objects.create(device_type=self.device_type, name='psu0')
|
||||
outlet = PowerOutletTemplate.objects.create(device_type=self.device_type, name='out0', power_port=pp)
|
||||
|
||||
form = PowerOutletTemplateForm(
|
||||
data={'device_type': self.device_type.pk, 'name': 'out0'},
|
||||
instance=outlet,
|
||||
)
|
||||
simulate_restrict(form, 'power_port', PowerPortTemplate.objects.none())
|
||||
|
||||
self.assertTrue(form.fields['power_port'].disabled)
|
||||
self.assertTrue(form.is_valid(), form.errors)
|
||||
outlet = form.save()
|
||||
self.assertEqual(outlet.power_port, pp)
|
||||
|
||||
def test_interfacetemplate_hidden_bridge_is_preserved(self):
|
||||
"""Editing an interface template whose bridge is hidden preserves it on save."""
|
||||
other = InterfaceTemplate.objects.create(device_type=self.device_type, name='br0', type='1000base-t')
|
||||
iface = InterfaceTemplate.objects.create(
|
||||
device_type=self.device_type, name='eth1', type='1000base-t', bridge=other
|
||||
)
|
||||
|
||||
form = InterfaceTemplateForm(
|
||||
data={'device_type': self.device_type.pk, 'name': 'eth1', 'type': '1000base-t'},
|
||||
instance=iface,
|
||||
)
|
||||
simulate_restrict(form, 'bridge', InterfaceTemplate.objects.none())
|
||||
|
||||
self.assertTrue(form.fields['bridge'].disabled)
|
||||
self.assertTrue(form.is_valid(), form.errors)
|
||||
iface = form.save()
|
||||
self.assertEqual(iface.bridge, other)
|
||||
|
|
|
|||
|
|
@ -14,7 +14,7 @@ from extras.constants import IMAGE_ATTACHMENT_IMAGE_FORMATS
|
|||
from extras.models import *
|
||||
from netbox.events import get_event_type_choices
|
||||
from netbox.forms import NetBoxModelForm, PrimaryModelForm
|
||||
from netbox.forms.mixins import ChangelogMessageMixin, OwnerMixin
|
||||
from netbox.forms.mixins import ChangelogMessageMixin, OwnerMixin, RestrictedRelatedFieldsMixin
|
||||
from tenancy.models import Tenant, TenantGroup
|
||||
from users.models import Group, User
|
||||
from utilities.forms import get_field_value
|
||||
|
|
@ -53,7 +53,7 @@ __all__ = (
|
|||
)
|
||||
|
||||
|
||||
class CustomFieldForm(ChangelogMessageMixin, OwnerMixin, forms.ModelForm):
|
||||
class CustomFieldForm(RestrictedRelatedFieldsMixin, ChangelogMessageMixin, OwnerMixin, forms.ModelForm):
|
||||
object_types = ContentTypeMultipleChoiceField(
|
||||
label=_('Object types'),
|
||||
queryset=ObjectType.objects.with_feature('custom_fields'),
|
||||
|
|
@ -492,7 +492,7 @@ class BookmarkForm(forms.ModelForm):
|
|||
fields = ('object_type', 'object_id')
|
||||
|
||||
|
||||
class NotificationGroupForm(ChangelogMessageMixin, forms.ModelForm):
|
||||
class NotificationGroupForm(RestrictedRelatedFieldsMixin, ChangelogMessageMixin, forms.ModelForm):
|
||||
groups = DynamicModelMultipleChoiceField(
|
||||
label=_('Groups'),
|
||||
required=False,
|
||||
|
|
@ -510,6 +510,7 @@ class NotificationGroupForm(ChangelogMessageMixin, forms.ModelForm):
|
|||
|
||||
def clean(self):
|
||||
super().clean()
|
||||
self._merge_restricted_preserved_members()
|
||||
|
||||
# At least one User or Group must be assigned
|
||||
if not self.cleaned_data['groups'] and not self.cleaned_data['users']:
|
||||
|
|
@ -578,6 +579,11 @@ class EventRuleForm(OwnerMixin, NetBoxModelForm):
|
|||
FieldSet('action_type', 'action_choice', 'action_data', name=_('Action')),
|
||||
)
|
||||
|
||||
# action_object_type/action_object_id are recomputed in clean() from these two fields.
|
||||
restricted_related_selectors = {
|
||||
'action_choice': {'path': 'action_object', 'lock_fields': ('action_type',)},
|
||||
}
|
||||
|
||||
class Meta:
|
||||
model = EventRule
|
||||
fields = (
|
||||
|
|
@ -709,7 +715,9 @@ class ConfigContextProfileForm(SyncedDataMixin, PrimaryModelForm):
|
|||
)
|
||||
|
||||
|
||||
class ConfigContextForm(ChangelogMessageMixin, SyncedDataMixin, OwnerMixin, forms.ModelForm):
|
||||
class ConfigContextForm(
|
||||
RestrictedRelatedFieldsMixin, ChangelogMessageMixin, SyncedDataMixin, OwnerMixin, forms.ModelForm
|
||||
):
|
||||
profile = DynamicModelChoiceField(
|
||||
label=_('Profile'),
|
||||
queryset=ConfigContextProfile.objects.all(),
|
||||
|
|
@ -819,6 +827,7 @@ class ConfigContextForm(ChangelogMessageMixin, SyncedDataMixin, OwnerMixin, form
|
|||
|
||||
def clean(self):
|
||||
super().clean()
|
||||
self._merge_restricted_preserved_members()
|
||||
|
||||
if not self.cleaned_data.get('data') and not self.cleaned_data.get('data_file'):
|
||||
raise forms.ValidationError(_("Must specify either local data or a data file"))
|
||||
|
|
@ -826,7 +835,9 @@ class ConfigContextForm(ChangelogMessageMixin, SyncedDataMixin, OwnerMixin, form
|
|||
return self.cleaned_data
|
||||
|
||||
|
||||
class ConfigTemplateForm(ChangelogMessageMixin, SyncedDataMixin, OwnerMixin, forms.ModelForm):
|
||||
class ConfigTemplateForm(
|
||||
RestrictedRelatedFieldsMixin, ChangelogMessageMixin, SyncedDataMixin, OwnerMixin, forms.ModelForm
|
||||
):
|
||||
tags = DynamicModelMultipleChoiceField(
|
||||
label=_('Tags'),
|
||||
queryset=Tag.objects.all(),
|
||||
|
|
@ -866,6 +877,7 @@ class ConfigTemplateForm(ChangelogMessageMixin, SyncedDataMixin, OwnerMixin, for
|
|||
|
||||
def clean(self):
|
||||
super().clean()
|
||||
self._merge_restricted_preserved_members()
|
||||
|
||||
if not self.cleaned_data.get('template_code') and not self.cleaned_data.get('data_file'):
|
||||
raise forms.ValidationError(_("Must specify either local content or a data file"))
|
||||
|
|
|
|||
|
|
@ -10,10 +10,28 @@ 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, TableConfigBulkEditForm, TableConfigForm
|
||||
from extras.forms import (
|
||||
ConfigContextForm,
|
||||
ConfigTemplateForm,
|
||||
CustomFieldForm,
|
||||
NotificationGroupForm,
|
||||
SavedFilterForm,
|
||||
TableConfigBulkEditForm,
|
||||
TableConfigForm,
|
||||
)
|
||||
from extras.forms.model_forms import CustomFieldChoiceSetForm
|
||||
from extras.forms.scripts import ScriptFileForm
|
||||
from extras.models import CustomField, CustomFieldChoiceSet, ScriptModule
|
||||
from extras.models import (
|
||||
ConfigContext,
|
||||
ConfigTemplate,
|
||||
CustomField,
|
||||
CustomFieldChoiceSet,
|
||||
NotificationGroup,
|
||||
ScriptModule,
|
||||
Tag,
|
||||
)
|
||||
from users.models import Group
|
||||
from utilities.testing import simulate_restrict
|
||||
|
||||
|
||||
class CustomFieldModelFormTestCase(TestCase):
|
||||
|
|
@ -337,3 +355,83 @@ class TableConfigFormTestCase(TestCase):
|
|||
form = TableConfigBulkEditForm()
|
||||
self.assertIn('changelog_message', form.fields)
|
||||
self.assertIn('changelog_message', form.meta_fields)
|
||||
|
||||
|
||||
class RestrictedCustomFieldFormTest(TestCase):
|
||||
"""CustomFieldForm (not a NetBoxModelForm) preserves a hidden choice_set FK via the mixin."""
|
||||
|
||||
def test_hidden_choice_set_is_preserved(self):
|
||||
"""Editing a custom field whose choice_set is hidden preserves it on save."""
|
||||
site_type = ObjectType.objects.get_for_model(Site)
|
||||
choice_set = CustomFieldChoiceSet.objects.create(name='Choice Set 1', extra_choices=(('A', 'A'), ('B', 'B')))
|
||||
cf = CustomField.objects.create(name='field_x', label='Field X', type='select', choice_set=choice_set)
|
||||
cf.object_types.set([site_type])
|
||||
|
||||
form = CustomFieldForm(
|
||||
data={
|
||||
'name': 'field_x',
|
||||
'label': 'Field X',
|
||||
'type': 'select',
|
||||
'object_types': [site_type.pk],
|
||||
'search_weight': 1000,
|
||||
'filter_logic': 'exact',
|
||||
'weight': 100,
|
||||
'ui_visible': 'always',
|
||||
'ui_editable': 'yes',
|
||||
},
|
||||
instance=cf,
|
||||
)
|
||||
simulate_restrict(form, 'choice_set', CustomFieldChoiceSet.objects.none())
|
||||
|
||||
self.assertTrue(form.fields['choice_set'].disabled)
|
||||
self.assertTrue(form.is_valid(), form.errors)
|
||||
cf = form.save()
|
||||
self.assertEqual(cf.choice_set, choice_set)
|
||||
|
||||
|
||||
class RestrictedExtrasM2MFormTest(TestCase):
|
||||
"""Extras M2M forms (not NetBoxModelForms) preserve hidden members via the mixin + clean() merge."""
|
||||
|
||||
def test_configcontext_hidden_site_is_preserved(self):
|
||||
"""Editing a config context with a hidden assigned site keeps it on save."""
|
||||
visible = Site.objects.create(name='Visible', slug='visible')
|
||||
hidden = Site.objects.create(name='Hidden', slug='hidden')
|
||||
cc = ConfigContext.objects.create(name='CC 1', weight=100, data={'foo': 123})
|
||||
cc.sites.set([visible, hidden])
|
||||
|
||||
form = ConfigContextForm(
|
||||
data={'name': 'CC 1', 'weight': 100, 'is_active': True, 'data': '{"foo": 123}', 'sites': [visible.pk]},
|
||||
instance=cc,
|
||||
)
|
||||
simulate_restrict(form, 'sites', Site.objects.filter(pk=visible.pk))
|
||||
self.assertTrue(form.is_valid(), form.errors)
|
||||
cc = form.save()
|
||||
self.assertEqual(set(cc.sites.all()), {visible, hidden})
|
||||
|
||||
def test_configtemplate_hidden_tag_is_preserved(self):
|
||||
"""Editing a config template with a hidden tag keeps it while visible tags stay editable."""
|
||||
visible = Tag.objects.create(name='Visible', slug='visible')
|
||||
hidden = Tag.objects.create(name='Hidden', slug='hidden')
|
||||
ct = ConfigTemplate.objects.create(name='CT 1', template_code='x')
|
||||
ct.tags.set([visible, hidden])
|
||||
|
||||
form = ConfigTemplateForm(
|
||||
data={'name': 'CT 1', 'template_code': 'x', 'tags': [visible.pk]},
|
||||
instance=ct,
|
||||
)
|
||||
simulate_restrict(form, 'tags', Tag.objects.filter(pk=visible.pk))
|
||||
self.assertTrue(form.is_valid(), form.errors)
|
||||
ct = form.save()
|
||||
self.assertEqual(set(ct.tags.all()), {visible, hidden})
|
||||
|
||||
def test_notificationgroup_only_hidden_group_stays_valid_and_preserved(self):
|
||||
"""A notification group whose only group is hidden stays valid and keeps it (merge precedes validation)."""
|
||||
hidden = Group.objects.create(name='Hidden group')
|
||||
ng = NotificationGroup.objects.create(name='NG 1')
|
||||
ng.groups.set([hidden])
|
||||
|
||||
form = NotificationGroupForm(data={'name': 'NG 1'}, instance=ng)
|
||||
simulate_restrict(form, 'groups', Group.objects.none())
|
||||
self.assertTrue(form.is_valid(), form.errors)
|
||||
ng = form.save()
|
||||
self.assertEqual(set(ng.groups.all()), {hidden})
|
||||
|
|
|
|||
|
|
@ -11,6 +11,7 @@ from ipam.constants import *
|
|||
from ipam.formfields import IPNetworkFormField
|
||||
from ipam.models import *
|
||||
from netbox.forms import NetBoxModelForm, OrganizationalModelForm, PrimaryModelForm
|
||||
from netbox.forms.mixins import RestrictedRelatedFieldsMixin
|
||||
from tenancy.forms import TenancyForm
|
||||
from utilities.exceptions import PermissionsViolation
|
||||
from utilities.forms import add_blank_choice
|
||||
|
|
@ -356,6 +357,13 @@ class IPAddressForm(TenancyForm, PrimaryModelForm):
|
|||
FieldSet('nat_inside', name=_('NAT IP (Inside)')),
|
||||
)
|
||||
|
||||
restricted_related_selectors = {
|
||||
# The selectors are stored as the assigned_object GenericForeignKey; the model picks the matching one.
|
||||
'interface': {'path': 'assigned_object', 'model': Interface},
|
||||
'vminterface': {'path': 'assigned_object', 'model': VMInterface},
|
||||
'fhrpgroup': {'path': 'assigned_object', 'model': FHRPGroup},
|
||||
}
|
||||
|
||||
class Meta:
|
||||
model = IPAddress
|
||||
fields = [
|
||||
|
|
@ -587,7 +595,7 @@ class FHRPGroupForm(PrimaryModelForm):
|
|||
})
|
||||
|
||||
|
||||
class FHRPGroupAssignmentForm(forms.ModelForm):
|
||||
class FHRPGroupAssignmentForm(RestrictedRelatedFieldsMixin, forms.ModelForm):
|
||||
group = DynamicModelChoiceField(
|
||||
label=_('Group'),
|
||||
queryset=FHRPGroup.objects.all()
|
||||
|
|
@ -645,6 +653,11 @@ class VLANGroupForm(TenancyForm, OrganizationalModelForm):
|
|||
FieldSet('tenant_group', 'tenant', name=_('Tenancy')),
|
||||
)
|
||||
|
||||
# Mirror of dcim ScopedForm.restricted_related_selectors; keep in sync (VLANGroup uses VLANGROUP_SCOPE_TYPES).
|
||||
restricted_related_selectors = {
|
||||
'scope': {'path': 'scope', 'lock_fields': ('scope_type',)},
|
||||
}
|
||||
|
||||
class Meta:
|
||||
model = VLANGroup
|
||||
fields = [
|
||||
|
|
@ -834,6 +847,10 @@ class ServiceForm(PrimaryModelForm):
|
|||
),
|
||||
)
|
||||
|
||||
restricted_related_selectors = {
|
||||
'parent': {'path': 'parent', 'lock_fields': ('parent_object_type',)},
|
||||
}
|
||||
|
||||
class Meta:
|
||||
model = Service
|
||||
fields = [
|
||||
|
|
|
|||
|
|
@ -3,8 +3,10 @@ from django.test import TestCase
|
|||
|
||||
from dcim.constants import InterfaceTypeChoices
|
||||
from dcim.models import Device, DeviceRole, DeviceType, Interface, Location, Manufacturer, Region, Site, SiteGroup
|
||||
from ipam.forms import PrefixForm, VLANIDBulkCreateForm
|
||||
from ipam.forms import FHRPGroupAssignmentForm, PrefixForm, VLANIDBulkCreateForm
|
||||
from ipam.forms.bulk_import import IPAddressImportForm
|
||||
from ipam.models import FHRPGroup, FHRPGroupAssignment
|
||||
from utilities.testing import create_test_device, simulate_restrict
|
||||
|
||||
|
||||
class PrefixFormTestCase(TestCase):
|
||||
|
|
@ -195,3 +197,28 @@ class VLANFormTestCase(TestCase):
|
|||
form = VLANIDBulkCreateForm({'pattern': pattern})
|
||||
self.assertFalse(form.is_valid())
|
||||
self.assertIn('pattern', form.errors)
|
||||
|
||||
|
||||
class RestrictedFHRPGroupAssignmentFormTest(TestCase):
|
||||
"""FHRPGroupAssignmentForm (not a NetBoxModelForm) preserves a hidden group FK via the mixin."""
|
||||
|
||||
@classmethod
|
||||
def setUpTestData(cls):
|
||||
cls.device = create_test_device('Device 1')
|
||||
cls.interface = Interface.objects.create(device=cls.device, name='eth0', type='1000base-t')
|
||||
|
||||
def test_hidden_group_is_preserved(self):
|
||||
"""Editing an FHRP group assignment whose group is hidden preserves it on save."""
|
||||
group = FHRPGroup.objects.create(protocol='vrrp2', group_id=1)
|
||||
assignment = FHRPGroupAssignment.objects.create(interface=self.interface, group=group, priority=10)
|
||||
|
||||
form = FHRPGroupAssignmentForm(
|
||||
data={'group': group.pk, 'priority': 10},
|
||||
instance=assignment,
|
||||
)
|
||||
simulate_restrict(form, 'group', FHRPGroup.objects.none())
|
||||
|
||||
self.assertTrue(form.fields['group'].disabled)
|
||||
self.assertTrue(form.is_valid(), form.errors)
|
||||
assignment = form.save()
|
||||
self.assertEqual(assignment.group, group)
|
||||
|
|
|
|||
|
|
@ -1,17 +1,24 @@
|
|||
from django import forms
|
||||
from django.contrib.contenttypes.models import ContentType
|
||||
from django.core.exceptions import FieldDoesNotExist
|
||||
from django.db.models import Model
|
||||
from django.utils.html import format_html
|
||||
from django.utils.translation import gettext as _
|
||||
from django.utils.translation import gettext_lazy
|
||||
|
||||
from core.models import ObjectType
|
||||
from extras.choices import *
|
||||
from extras.models import *
|
||||
from users.models import Owner, OwnerGroup
|
||||
from utilities.forms.fields import DynamicModelChoiceField, DynamicModelMultipleChoiceField
|
||||
from utilities.forms.widgets.misc import RestrictedChoiceLabel
|
||||
|
||||
__all__ = (
|
||||
'ChangelogMessageMixin',
|
||||
'CustomFieldsMixin',
|
||||
'OwnerFilterMixin',
|
||||
'OwnerMixin',
|
||||
'RestrictedRelatedFieldsMixin',
|
||||
'SavedFiltersMixin',
|
||||
'TagsMixin',
|
||||
)
|
||||
|
|
@ -190,3 +197,339 @@ class OwnerFilterMixin(forms.Form):
|
|||
},
|
||||
label=_('Owner'),
|
||||
)
|
||||
|
||||
|
||||
class RestrictedRelatedFieldsMixin:
|
||||
"""
|
||||
Form mixin that preserves already-assigned related values hidden by object-permission filtering.
|
||||
|
||||
Driven by restrict_form_fields() (or prepare_restricted_queryset_fields() directly): when a field's queryset is
|
||||
narrowed so the current value is no longer a choice, that value is shown read-only and preserved on save. Kept as
|
||||
a standalone mixin so forms that are not NetBoxModelForms (e.g. component-template forms) can opt in.
|
||||
"""
|
||||
|
||||
restricted_value_help_text = gettext_lazy(
|
||||
'This field includes one or more restricted values that cannot be changed. '
|
||||
'They will be preserved when this form is saved.'
|
||||
)
|
||||
|
||||
# Maps selector form fields whose current value is stored under a different instance attribute (e.g. a
|
||||
# GenericForeignKey). Keys are form field names; entries may declare:
|
||||
# path: dotted attribute path on the instance holding the current value (read from the instance only)
|
||||
# model: expected model class; set when several selector fields share one path (picks the matching field)
|
||||
# lock_fields: controller field names locked alongside the selector when its current value is hidden
|
||||
# Merged across the MRO by __init_subclass__: a subclass's entries combine with those declared by base
|
||||
# classes (e.g. ScopedForm's 'scope'), the subclass winning on key conflicts, so inherited selectors are
|
||||
# never silently dropped.
|
||||
restricted_related_selectors = {}
|
||||
|
||||
def __init_subclass__(cls, **kwargs):
|
||||
super().__init_subclass__(**kwargs)
|
||||
merged = {}
|
||||
for klass in reversed(cls.__mro__):
|
||||
merged.update(klass.__dict__.get('restricted_related_selectors', {}))
|
||||
cls.restricted_related_selectors = merged
|
||||
|
||||
def prepare_restricted_queryset_fields(self, restricted_fields, user=None, action='view'):
|
||||
"""
|
||||
Render assigned values which were removed by object-permission filtering as read-only.
|
||||
|
||||
`restricted_fields` maps each restricted field name to its original (pre-restriction) queryset. `user` and
|
||||
`action` allow re-checking visibility for GenericForeignKey values whose model differs from the field.
|
||||
|
||||
Security rules:
|
||||
|
||||
* Only values already assigned to the current instance are added back; the rest of the original queryset is
|
||||
never exposed.
|
||||
* A value is preserved only when the user cannot view it; values excluded by the form's own base queryset
|
||||
are left untouched.
|
||||
* Restricted current values are read-only and submitted or tampered values for them are ignored.
|
||||
|
||||
Scalar fields (and fields already disabled, e.g. read-only custom fields) are disabled entirely. Editable
|
||||
multi-value fields stay editable so the user can manage permitted values, while restricted current members
|
||||
are shown as disabled options and preserved server-side in clean().
|
||||
"""
|
||||
# Guard against running twice on the same form (which would double-wrap labels and duplicate help text).
|
||||
if getattr(self, '_restricted_queryset_fields_prepared', False) or not self.instance.pk:
|
||||
return
|
||||
self._restricted_queryset_fields_prepared = True
|
||||
self._restricted_preserved_members = {}
|
||||
|
||||
for field_name, original_queryset in restricted_fields.items():
|
||||
field = self.fields.get(field_name)
|
||||
if field is None:
|
||||
continue
|
||||
|
||||
restricted_queryset = getattr(field, 'queryset', None)
|
||||
if restricted_queryset is None:
|
||||
continue
|
||||
|
||||
current_objects = self._get_restricted_queryset_field_current_objects(field_name, original_queryset)
|
||||
if not current_objects:
|
||||
continue
|
||||
|
||||
hidden_objects = self._hidden_restricted_objects(
|
||||
restricted_queryset, original_queryset, current_objects, user, action,
|
||||
declared_selector=field_name in self.restricted_related_selectors,
|
||||
)
|
||||
if not hidden_objects:
|
||||
continue
|
||||
|
||||
if isinstance(field, forms.ModelMultipleChoiceField) and not field.disabled:
|
||||
self._prepare_restricted_multiple_field(
|
||||
field_name, field, original_queryset, restricted_queryset, current_objects, hidden_objects
|
||||
)
|
||||
else:
|
||||
self._lock_restricted_queryset_field(field_name, field, current_objects, hidden_objects)
|
||||
|
||||
def _lock_restricted_queryset_field(self, field_name, field, current_objects, hidden_objects):
|
||||
"""
|
||||
Show only the current value(s) and make the whole field read-only for this request.
|
||||
|
||||
Disabling the field makes Django ignore submitted data and clean the initial value instead. The queryset is
|
||||
narrowed to the current objects using their own model, so a GenericForeignKey value is preserved even when
|
||||
the submitted type field points at a different model. Controller fields declared for the selector
|
||||
(lock_fields) are locked too so construct_instance() and clean() cannot corrupt the preserved value.
|
||||
"""
|
||||
multiple = isinstance(field, forms.ModelMultipleChoiceField)
|
||||
field.disabled = True
|
||||
field.required = False
|
||||
field.widget.is_required = False
|
||||
|
||||
field.queryset = self._queryset_for_objects(current_objects)
|
||||
|
||||
if multiple:
|
||||
initial = [field.prepare_value(obj) for obj in current_objects]
|
||||
else:
|
||||
initial = field.prepare_value(current_objects[0])
|
||||
field.initial = initial
|
||||
self.initial[field_name] = initial
|
||||
|
||||
self._lock_controller_fields(field_name, hidden_objects)
|
||||
self._append_restricted_value_help_text(field)
|
||||
|
||||
def _queryset_for_objects(self, objects):
|
||||
"""
|
||||
Build a queryset over the objects' own model restricted to their PKs. A scalar selector holds one type at a
|
||||
time, so all objects share a model.
|
||||
"""
|
||||
model = type(objects[0])
|
||||
return model.objects.filter(pk__in=[obj.pk for obj in objects])
|
||||
|
||||
def _lock_controller_fields(self, field_name, hidden_objects):
|
||||
"""
|
||||
Lock the controller fields declared for a selector (lock_fields) so a submitted value cannot corrupt the
|
||||
preserved assignment. A content type controller is pinned to the current object's own type; any other
|
||||
controller is pinned to the value stored on the instance.
|
||||
"""
|
||||
selector = self.restricted_related_selectors.get(field_name) or {}
|
||||
for controller_name in selector.get('lock_fields', ()):
|
||||
controller = self.fields.get(controller_name)
|
||||
if controller is None:
|
||||
continue
|
||||
queryset = getattr(controller, 'queryset', None)
|
||||
if queryset is not None and issubclass(queryset.model, ContentType):
|
||||
content_type = ContentType.objects.get_for_model(type(hidden_objects[0]))
|
||||
controller.queryset = queryset.model.objects.filter(pk=content_type.pk)
|
||||
initial = content_type.pk
|
||||
else:
|
||||
initial = getattr(self.instance, controller_name, None)
|
||||
if isinstance(initial, Model):
|
||||
initial = initial.pk
|
||||
controller.disabled = True
|
||||
controller.required = False
|
||||
controller.widget.is_required = False
|
||||
controller.initial = initial
|
||||
self.initial[controller_name] = initial
|
||||
|
||||
def _prepare_restricted_multiple_field(
|
||||
self, field_name, field, original_queryset, restricted_queryset, current_objects, hidden_objects
|
||||
):
|
||||
"""
|
||||
Keep an editable multi-value field editable while rendering restricted current members as disabled options.
|
||||
|
||||
The user can still add or remove permitted values. Restricted current members are added back to the
|
||||
queryset (so the current values validate and render selected) and shown as disabled options as a read-only
|
||||
hint. Preservation is enforced in clean(): the hidden members are merged back into the field value
|
||||
regardless of what the widget submits. Forged values are still rejected, because only the already-assigned
|
||||
restricted members are added to the queryset.
|
||||
"""
|
||||
hidden_pks = [obj.pk for obj in hidden_objects]
|
||||
|
||||
field.required = False
|
||||
field.widget.is_required = False
|
||||
|
||||
# Allow the visible choices plus the already-assigned restricted members; expose nothing else. The two sides
|
||||
# are disjoint by PK (hidden members are exactly those absent from the restricted queryset), so no DISTINCT.
|
||||
hidden_queryset = original_queryset.filter(pk__in=hidden_pks)
|
||||
field.queryset = restricted_queryset | hidden_queryset
|
||||
|
||||
# Show all current members selected on render.
|
||||
initial = [field.prepare_value(obj) for obj in current_objects]
|
||||
field.initial = initial
|
||||
self.initial[field_name] = initial
|
||||
|
||||
# Render the restricted members as disabled options (a read-only hint; preservation is enforced server-side).
|
||||
self._mark_restricted_choice_labels_disabled(field, hidden_objects)
|
||||
|
||||
# Preserve the hidden members on save without mutating submitted data.
|
||||
self._restricted_preserved_members[field_name] = hidden_objects
|
||||
|
||||
self._append_restricted_value_help_text(field)
|
||||
|
||||
def _mark_restricted_choice_labels_disabled(self, field, hidden_objects):
|
||||
"""
|
||||
Wrap label_from_instance so the restricted members render as disabled options.
|
||||
"""
|
||||
hidden_values = {str(field.prepare_value(obj)) for obj in hidden_objects}
|
||||
label_from_instance = field.label_from_instance
|
||||
|
||||
def label_with_restricted_marker(obj):
|
||||
label = label_from_instance(obj)
|
||||
if str(field.prepare_value(obj)) in hidden_values:
|
||||
return RestrictedChoiceLabel(label)
|
||||
return label
|
||||
|
||||
field.label_from_instance = label_with_restricted_marker
|
||||
|
||||
def _append_restricted_value_help_text(self, field):
|
||||
if field.help_text:
|
||||
field.help_text = format_html('{} {}', field.help_text, self.restricted_value_help_text)
|
||||
else:
|
||||
field.help_text = self.restricted_value_help_text
|
||||
|
||||
def _get_objects_from_queryset(self, queryset, pks):
|
||||
"""
|
||||
Return objects for `pks` from `queryset`, preserving order. Used for selectors whose current PKs are known
|
||||
to belong to the queryset's model.
|
||||
"""
|
||||
if not pks:
|
||||
return []
|
||||
objects_by_pk = {str(obj.pk): obj for obj in queryset.filter(pk__in=pks)}
|
||||
return [objects_by_pk[str(pk)] for pk in pks if str(pk) in objects_by_pk]
|
||||
|
||||
def _hidden_restricted_objects(self, restricted_queryset, original_queryset, current_objects, user, action,
|
||||
declared_selector=False):
|
||||
"""
|
||||
Return the subset of `current_objects` the user cannot view (and which were valid choices before
|
||||
restriction).
|
||||
|
||||
Objects whose model matches the field are checked against the field's own restricted/original querysets in a
|
||||
single pair of queries. Objects of a different model (a GenericForeignKey whose paired type field was changed,
|
||||
of which a selector holds at most one) are checked individually against their own model, so a same-PK object
|
||||
of another model is never mistaken for the current value.
|
||||
|
||||
For a declared selector (`restricted_related_selectors`), the field queryset is derived from submitted
|
||||
controller data and may be empty (a blanked optional type field leaves the selector on its default `.none()`)
|
||||
or of the wrong model. The current value is read from the instance, so visibility is always judged against the
|
||||
object's own model, never the field queryset.
|
||||
"""
|
||||
field_model = getattr(restricted_queryset, 'model', None)
|
||||
if declared_selector:
|
||||
same_model = []
|
||||
other_model = list(current_objects)
|
||||
else:
|
||||
same_model = [obj for obj in current_objects if field_model is not None and type(obj) is field_model]
|
||||
other_model = [obj for obj in current_objects if field_model is None or type(obj) is not field_model]
|
||||
|
||||
hidden = []
|
||||
if same_model:
|
||||
pks = [obj.pk for obj in same_model]
|
||||
was_choice = set(original_queryset.filter(pk__in=pks).values_list('pk', flat=True))
|
||||
visible = set(restricted_queryset.filter(pk__in=pks).values_list('pk', flat=True))
|
||||
hidden += [obj for obj in same_model if obj.pk in was_choice and obj.pk not in visible]
|
||||
|
||||
for obj in other_model:
|
||||
manager = type(obj).objects
|
||||
if user is not None and hasattr(manager, 'restrict'):
|
||||
if not manager.restrict(user, action).filter(pk=obj.pk).exists():
|
||||
hidden.append(obj)
|
||||
else:
|
||||
# Without a user we cannot re-check visibility for a different model, so preserve to avoid data loss.
|
||||
hidden.append(obj)
|
||||
|
||||
return hidden
|
||||
|
||||
def _get_restricted_queryset_field_current_objects(self, field_name, original_queryset):
|
||||
"""
|
||||
Return the current assigned objects for a restricted form field, read from the instance (never submitted
|
||||
data). Objects carry their true model, so a GenericForeignKey value is never looked up against a model
|
||||
chosen from submitted data.
|
||||
"""
|
||||
if selector := self.restricted_related_selectors.get(field_name):
|
||||
return self._get_restricted_selector_current_objects(selector)
|
||||
|
||||
if field_name in getattr(self, 'custom_fields', {}):
|
||||
pks = self._get_restricted_custom_field_current_pks(field_name)
|
||||
return self._get_objects_from_queryset(original_queryset, pks)
|
||||
|
||||
return self._get_current_objects_from_instance(field_name)
|
||||
|
||||
def _get_restricted_selector_current_objects(self, selector):
|
||||
"""
|
||||
Resolve a declared selector's current value by walking its dotted path from the instance. When the
|
||||
declaration names a model, a value of any other model belongs to a sibling selector and is skipped.
|
||||
"""
|
||||
obj = self.instance
|
||||
for attr in selector['path'].split('.'):
|
||||
obj = getattr(obj, attr, None)
|
||||
if obj is None:
|
||||
return []
|
||||
model = selector.get('model')
|
||||
if model is not None and type(obj) is not model:
|
||||
return []
|
||||
return [obj]
|
||||
|
||||
def _get_current_objects_from_instance(self, field_name):
|
||||
"""
|
||||
Resolve current assigned objects from the instance for forward/reverse M2M, FK/O2O, and same-named
|
||||
GenericForeignKey fields. Returns model instances of their true type.
|
||||
"""
|
||||
try:
|
||||
model_field = self.instance._meta.get_field(field_name)
|
||||
except FieldDoesNotExist:
|
||||
model_field = None
|
||||
|
||||
# Forward many-to-many (includes django-taggit tags).
|
||||
if model_field is not None and getattr(model_field, 'many_to_many', False):
|
||||
return list(getattr(self.instance, field_name).all())
|
||||
|
||||
attr = getattr(self.instance, field_name, None)
|
||||
|
||||
# Reverse many-to-many exposed directly on a form.
|
||||
if hasattr(attr, 'all') and hasattr(attr, 'values_list'):
|
||||
return list(attr.all())
|
||||
|
||||
# Forward FK/O2O or a same-named GenericForeignKey: a single related object.
|
||||
if isinstance(attr, Model):
|
||||
return [attr]
|
||||
|
||||
return []
|
||||
|
||||
def _get_restricted_custom_field_current_pks(self, field_name):
|
||||
"""
|
||||
Return current serialized PKs for object and multi-object custom fields.
|
||||
"""
|
||||
customfield = self.custom_fields[field_name]
|
||||
value = self.instance.custom_field_data.get(customfield.name)
|
||||
|
||||
if customfield.type == CustomFieldTypeChoices.TYPE_OBJECT:
|
||||
if value is None:
|
||||
return []
|
||||
return [value.pk if isinstance(value, Model) else value]
|
||||
|
||||
if customfield.type == CustomFieldTypeChoices.TYPE_MULTIOBJECT:
|
||||
return [
|
||||
obj.pk if isinstance(obj, Model) else obj
|
||||
for obj in value or []
|
||||
]
|
||||
|
||||
return []
|
||||
|
||||
def _merge_restricted_preserved_members(self):
|
||||
for field_name, objects in getattr(self, '_restricted_preserved_members', {}).items():
|
||||
if field_name not in self.cleaned_data:
|
||||
continue
|
||||
existing = list(self.cleaned_data[field_name])
|
||||
existing_pks = {obj.pk for obj in existing}
|
||||
self.cleaned_data[field_name] = existing + [obj for obj in objects if obj.pk not in existing_pks]
|
||||
|
|
|
|||
|
|
@ -8,7 +8,7 @@ from extras.choices import *
|
|||
from utilities.forms.fields import CommentField, SlugField
|
||||
from utilities.forms.mixins import CheckLastUpdatedMixin
|
||||
|
||||
from .mixins import ChangelogMessageMixin, CustomFieldsMixin, OwnerMixin, TagsMixin
|
||||
from .mixins import ChangelogMessageMixin, CustomFieldsMixin, OwnerMixin, RestrictedRelatedFieldsMixin, TagsMixin
|
||||
|
||||
__all__ = (
|
||||
'NestedGroupModelForm',
|
||||
|
|
@ -19,6 +19,7 @@ __all__ = (
|
|||
|
||||
|
||||
class NetBoxModelForm(
|
||||
RestrictedRelatedFieldsMixin,
|
||||
ChangelogMessageMixin,
|
||||
CheckLastUpdatedMixin,
|
||||
CustomFieldsMixin,
|
||||
|
|
@ -50,6 +51,9 @@ class NetBoxModelForm(
|
|||
return customfield.to_form_field()
|
||||
|
||||
def clean(self):
|
||||
# Merge restricted current members the user could not see back into multi-value fields so they survive on
|
||||
# save (their options are disabled in the widget and so are not submitted).
|
||||
self._merge_restricted_preserved_members()
|
||||
|
||||
# Save custom field data on instance
|
||||
for cf_name, customfield in self.custom_fields.items():
|
||||
|
|
|
|||
File diff suppressed because it is too large
Load Diff
|
|
@ -13,6 +13,7 @@ from core.models import ObjectType
|
|||
from ipam.formfields import IPNetworkFormField
|
||||
from ipam.validators import prefix_validator
|
||||
from netbox.config import get_config
|
||||
from netbox.forms.mixins import RestrictedRelatedFieldsMixin
|
||||
from netbox.preferences import PREFERENCES
|
||||
from netbox.registry import registry
|
||||
from users.choices import TokenVersionChoices
|
||||
|
|
@ -204,7 +205,7 @@ class TokenForm(UserTokenForm):
|
|||
return self.cleaned_data
|
||||
|
||||
|
||||
class UserForm(forms.ModelForm):
|
||||
class UserForm(RestrictedRelatedFieldsMixin, forms.ModelForm):
|
||||
password = forms.CharField(
|
||||
label=_('Password'),
|
||||
widget=forms.PasswordInput(),
|
||||
|
|
@ -260,6 +261,7 @@ class UserForm(forms.ModelForm):
|
|||
return instance
|
||||
|
||||
def clean(self):
|
||||
self._merge_restricted_preserved_members()
|
||||
|
||||
# Check that password confirmation matches if password is set
|
||||
if self.cleaned_data['password'] and self.cleaned_data['password'] != self.cleaned_data['confirm_password']:
|
||||
|
|
@ -270,7 +272,7 @@ class UserForm(forms.ModelForm):
|
|||
password_validation.validate_password(self.cleaned_data['password'], self.instance)
|
||||
|
||||
|
||||
class GroupForm(forms.ModelForm):
|
||||
class GroupForm(RestrictedRelatedFieldsMixin, forms.ModelForm):
|
||||
users = DynamicModelMultipleChoiceField(
|
||||
label=_('Users'),
|
||||
required=False,
|
||||
|
|
@ -301,6 +303,11 @@ class GroupForm(forms.ModelForm):
|
|||
if self.instance.pk:
|
||||
self.fields['users'].initial = self.instance.users.values_list('id', flat=True)
|
||||
|
||||
def clean(self):
|
||||
super().clean()
|
||||
self._merge_restricted_preserved_members()
|
||||
return self.cleaned_data
|
||||
|
||||
def save(self, *args, **kwargs):
|
||||
instance = super().save(*args, **kwargs)
|
||||
|
||||
|
|
@ -331,7 +338,7 @@ def get_object_types_choices():
|
|||
return list(choices_by_app.items())
|
||||
|
||||
|
||||
class ObjectPermissionForm(forms.ModelForm):
|
||||
class ObjectPermissionForm(RestrictedRelatedFieldsMixin, forms.ModelForm):
|
||||
object_types = ContentTypeMultipleChoiceField(
|
||||
label=_('Object types'),
|
||||
queryset=ObjectType.objects.all(),
|
||||
|
|
@ -482,6 +489,7 @@ class ObjectPermissionForm(forms.ModelForm):
|
|||
|
||||
def clean(self):
|
||||
super().clean()
|
||||
self._merge_restricted_preserved_members()
|
||||
|
||||
object_types = self.cleaned_data.get('object_types', [])
|
||||
constraints = self.cleaned_data.get('constraints')
|
||||
|
|
@ -551,7 +559,7 @@ class OwnerGroupForm(forms.ModelForm):
|
|||
]
|
||||
|
||||
|
||||
class OwnerForm(forms.ModelForm):
|
||||
class OwnerForm(RestrictedRelatedFieldsMixin, forms.ModelForm):
|
||||
fieldsets = (
|
||||
FieldSet('name', 'group', 'description', name=_('Owner')),
|
||||
FieldSet('user_groups', name=_('Groups')),
|
||||
|
|
@ -580,3 +588,8 @@ class OwnerForm(forms.ModelForm):
|
|||
fields = [
|
||||
'name', 'group', 'description', 'user_groups', 'users',
|
||||
]
|
||||
|
||||
def clean(self):
|
||||
super().clean()
|
||||
self._merge_restricted_preserved_members()
|
||||
return self.cleaned_data
|
||||
|
|
|
|||
|
|
@ -0,0 +1,71 @@
|
|||
from django.test import TestCase
|
||||
|
||||
from core.models import ObjectType
|
||||
from dcim.models import Site
|
||||
from users.forms import GroupForm, ObjectPermissionForm, OwnerForm, UserForm
|
||||
from users.models import Group, ObjectPermission, Owner, User
|
||||
from utilities.testing import simulate_restrict
|
||||
|
||||
|
||||
class RestrictedUsersFormTest(TestCase):
|
||||
"""Users-app M2M forms (not NetBoxModelForms) preserve hidden members via the mixin + clean() merge."""
|
||||
|
||||
def test_userform_hidden_group_is_preserved(self):
|
||||
"""Editing a user with a hidden group keeps it on save (Meta.fields save_m2m path)."""
|
||||
visible = Group.objects.create(name='Visible')
|
||||
hidden = Group.objects.create(name='Hidden')
|
||||
user = User.objects.create(username='user1')
|
||||
user.groups.set([visible, hidden])
|
||||
|
||||
form = UserForm(data={'username': 'user1', 'groups': [visible.pk]}, instance=user)
|
||||
simulate_restrict(form, 'groups', Group.objects.filter(pk=visible.pk))
|
||||
self.assertTrue(form.is_valid(), form.errors)
|
||||
user = form.save()
|
||||
self.assertEqual(set(user.groups.all()), {visible, hidden})
|
||||
|
||||
def test_groupform_hidden_user_is_preserved(self):
|
||||
"""Editing a group with a hidden member keeps it on save (custom .set() path)."""
|
||||
visible = User.objects.create(username='visible')
|
||||
hidden = User.objects.create(username='hidden')
|
||||
group = Group.objects.create(name='Group 1')
|
||||
group.users.set([visible, hidden])
|
||||
|
||||
form = GroupForm(data={'name': 'Group 1', 'users': [visible.pk]}, instance=group)
|
||||
simulate_restrict(form, 'users', User.objects.filter(pk=visible.pk))
|
||||
self.assertTrue(form.is_valid(), form.errors)
|
||||
group = form.save()
|
||||
self.assertEqual(set(group.users.all()), {visible, hidden})
|
||||
|
||||
def test_objectpermissionform_hidden_user_is_preserved(self):
|
||||
"""Editing an object permission with a hidden user keeps it on save (custom .set() path)."""
|
||||
visible = User.objects.create(username='visible')
|
||||
hidden = User.objects.create(username='hidden')
|
||||
perm = ObjectPermission.objects.create(name='Perm 1', actions=['view'])
|
||||
perm.users.set([visible, hidden])
|
||||
|
||||
form = ObjectPermissionForm(
|
||||
data={
|
||||
'name': 'Perm 1',
|
||||
'object_types_1': [ObjectType.objects.get_for_model(Site).pk],
|
||||
'actions': 'view',
|
||||
'users': [visible.pk],
|
||||
},
|
||||
instance=perm,
|
||||
)
|
||||
simulate_restrict(form, 'users', User.objects.filter(pk=visible.pk))
|
||||
self.assertTrue(form.is_valid(), form.errors)
|
||||
perm = form.save()
|
||||
self.assertEqual(set(perm.users.all()), {visible, hidden})
|
||||
|
||||
def test_ownerform_hidden_user_group_is_preserved(self):
|
||||
"""Editing an owner with a hidden user group keeps it on save (Meta.fields save_m2m path)."""
|
||||
visible = Group.objects.create(name='Visible')
|
||||
hidden = Group.objects.create(name='Hidden')
|
||||
owner = Owner.objects.create(name='Owner 1')
|
||||
owner.user_groups.set([visible, hidden])
|
||||
|
||||
form = OwnerForm(data={'name': 'Owner 1', 'user_groups': [visible.pk]}, instance=owner)
|
||||
simulate_restrict(form, 'user_groups', Group.objects.filter(pk=visible.pk))
|
||||
self.assertTrue(form.is_valid(), form.errors)
|
||||
owner = form.save()
|
||||
self.assertEqual(set(owner.user_groups.all()), {visible, hidden})
|
||||
|
|
@ -219,10 +219,32 @@ def restrict_form_fields(form, user, action='view'):
|
|||
"""
|
||||
Restrict all form fields which reference a RestrictedQuerySet. This ensures that users see only permitted objects
|
||||
as available choices.
|
||||
|
||||
Forms may optionally expose already-assigned values which were removed by permission filtering as read-only. This
|
||||
is handled by the form: only values already assigned to the current instance are added back (never the rest of the
|
||||
original queryset), they are rendered read-only, and submitted/tampered values for them are ignored.
|
||||
"""
|
||||
for field in form.fields.values():
|
||||
restricted_fields = {}
|
||||
|
||||
for name, field in form.fields.items():
|
||||
if hasattr(field, 'queryset') and issubclass(field.queryset.__class__, RestrictedQuerySet):
|
||||
field.queryset = field.queryset.restrict(user, action)
|
||||
original_queryset = field.queryset
|
||||
restricted_queryset = original_queryset.restrict(user, action)
|
||||
field.queryset = restricted_queryset
|
||||
# restrict() returns the same queryset object (identity unchanged) whenever no row-level restriction
|
||||
# applies: superusers, permission-exempt models, and users who hold the permission with no attribute
|
||||
# constraints all fall through to `return self`. Those fields are excluded here by the identity check
|
||||
# below. A field is recorded only when restrict() narrows the queryset to a new object;
|
||||
# prepare_restricted_queryset_fields() then locks a value only when it is genuinely hidden.
|
||||
if restricted_queryset is not original_queryset:
|
||||
restricted_fields[name] = original_queryset
|
||||
|
||||
if (
|
||||
restricted_fields and
|
||||
getattr(getattr(form, 'instance', None), 'pk', None) and
|
||||
hasattr(form, 'prepare_restricted_queryset_fields')
|
||||
):
|
||||
form.prepare_restricted_queryset_fields(restricted_fields, user=user, action=action)
|
||||
|
||||
|
||||
def parse_csv(reader):
|
||||
|
|
|
|||
|
|
@ -6,6 +6,7 @@ __all__ = (
|
|||
'ClearableFileInput',
|
||||
'MarkdownWidget',
|
||||
'NumberWithOptions',
|
||||
'RestrictedChoiceLabel',
|
||||
'SlugWidget',
|
||||
)
|
||||
|
||||
|
|
@ -85,3 +86,18 @@ class ChoicesWidget(forms.Textarea):
|
|||
if type(value) is list:
|
||||
return '\n'.join([f'{k}:{v}' for k, v in value])
|
||||
return value
|
||||
|
||||
|
||||
class RestrictedChoiceLabel:
|
||||
"""
|
||||
Wraps a choice label so widgets/select_option.html renders that single <option> as disabled. Used to identify a
|
||||
restricted current value as read-only in widgets which honor disabled options. Preservation itself is enforced
|
||||
server-side, not by the widget.
|
||||
"""
|
||||
disabled = True
|
||||
|
||||
def __init__(self, label):
|
||||
self.label = label
|
||||
|
||||
def __str__(self):
|
||||
return str(self.label)
|
||||
|
|
|
|||
|
|
@ -1 +1,6 @@
|
|||
{% comment %}
|
||||
A RestrictedChoiceLabel (netbox.forms.model_forms) sets label.disabled=True to render its single <option> as
|
||||
disabled, marking a restricted current value read-only. Preservation is enforced server-side, not by this attribute.
|
||||
Any label object exposing a truthy .disabled triggers this; plain string labels never do.
|
||||
{% endcomment %}
|
||||
<option value="{{ widget.value }}"{% include "django/forms/widgets/attrs.html" %}{% if widget.label.disabled %} disabled="disabled"{% endif %}>{{ widget.label.label|default:widget.label }}</option>
|
||||
|
|
|
|||
|
|
@ -41,6 +41,18 @@ def post_data(data):
|
|||
return ret
|
||||
|
||||
|
||||
def simulate_restrict(form, field_name, restricted_queryset, original_queryset=None):
|
||||
"""
|
||||
Stand in for restrict_form_fields() in a form unit test: record the original (pre-restriction) queryset, swap in
|
||||
the restricted one, then prepare the read-only display. `original_queryset` defaults to the field's current
|
||||
queryset. The form must expose prepare_restricted_queryset_fields() (i.e. use RestrictedRelatedFieldsMixin).
|
||||
"""
|
||||
if original_queryset is None:
|
||||
original_queryset = form.fields[field_name].queryset
|
||||
form.fields[field_name].queryset = restricted_queryset
|
||||
form.prepare_restricted_queryset_fields({field_name: original_queryset})
|
||||
|
||||
|
||||
def create_test_device(name, site=None, **attrs):
|
||||
"""
|
||||
Convenience method for creating a Device (e.g. for component testing).
|
||||
|
|
|
|||
|
|
@ -253,6 +253,13 @@ class TunnelTerminationForm(NetBoxModelForm):
|
|||
FieldSet('tunnel', 'role', 'type', 'parent', 'termination', 'outside_ip', 'tags'),
|
||||
)
|
||||
|
||||
# type is not locked: clean() writes instance.termination from the locked selector and never branches on it.
|
||||
restricted_related_selectors = {
|
||||
'termination': {'path': 'termination'},
|
||||
# parent is an auxiliary selector for the device or VM owning the termination.
|
||||
'parent': {'path': 'termination.parent_object'},
|
||||
}
|
||||
|
||||
class Meta:
|
||||
model = TunnelTermination
|
||||
fields = [
|
||||
|
|
@ -455,6 +462,13 @@ class L2VPNTerminationForm(NetBoxModelForm):
|
|||
),
|
||||
)
|
||||
|
||||
restricted_related_selectors = {
|
||||
# The selectors are stored as the assigned_object GenericForeignKey; the model picks the matching one.
|
||||
'interface': {'path': 'assigned_object', 'model': Interface},
|
||||
'vminterface': {'path': 'assigned_object', 'model': VMInterface},
|
||||
'vlan': {'path': 'assigned_object', 'model': VLAN},
|
||||
}
|
||||
|
||||
class Meta:
|
||||
model = L2VPNTermination
|
||||
fields = ('l2vpn', 'tags')
|
||||
|
|
|
|||
Loading…
Reference in New Issue