Address second round of PR feedback on render_config permissions
Some checks failed
CI / build (20.x, 3.12) (push) Has been cancelled
CI / build (20.x, 3.13) (push) Has been cancelled

- Remove ConfigTemplate view permission check from render_config endpoint
- Add sanity check to TokenWritePermission for non-token auth
- Use named URL patterns instead of string concatenation in tests
- Remove extras.view_configtemplate from test permissions
- Add token write_enabled enforcement tests for all render endpoints
This commit is contained in:
Jason Novinger
2025-10-20 16:32:23 -05:00
parent 9967b20663
commit 2dd1b97afb
5 changed files with 106 additions and 20 deletions

View File

@@ -13,7 +13,8 @@ from ipam.choices import VLANQinQRoleChoices
from ipam.models import ASN, RIR, VLAN, VRF from ipam.models import ASN, RIR, VLAN, VRF
from netbox.api.serializers import GenericObjectSerializer from netbox.api.serializers import GenericObjectSerializer
from tenancy.models import Tenant from tenancy.models import Tenant
from users.models import User from users.constants import TOKEN_PREFIX
from users.models import Token, User
from utilities.testing import APITestCase, APIViewTestCases, create_test_device, disable_logging from utilities.testing import APITestCase, APIViewTestCases, create_test_device, disable_logging
from virtualization.models import Cluster, ClusterType from virtualization.models import Cluster, ClusterType
from wireless.choices import WirelessChannelChoices from wireless.choices import WirelessChannelChoices
@@ -1485,8 +1486,8 @@ class DeviceTest(APIViewTestCases.APIViewTestCase):
device.config_template = configtemplate device.config_template = configtemplate
device.save() device.save()
self.add_permissions('dcim.render_config_device', 'dcim.view_device', 'extras.view_configtemplate') self.add_permissions('dcim.render_config_device', 'dcim.view_device')
url = reverse('dcim-api:device-detail', kwargs={'pk': device.pk}) + 'render-config/' url = reverse('dcim-api:device-render-config', kwargs={'pk': device.pk})
response = self.client.post(url, {}, format='json', **self.header) response = self.client.post(url, {}, format='json', **self.header)
self.assertHttpStatus(response, status.HTTP_200_OK) self.assertHttpStatus(response, status.HTTP_200_OK)
self.assertEqual(response.data['content'], f'Config for device {device.name}') self.assertEqual(response.data['content'], f'Config for device {device.name}')
@@ -1502,10 +1503,41 @@ class DeviceTest(APIViewTestCases.APIViewTestCase):
device.save() device.save()
# No permissions added - user has no render_config permission # No permissions added - user has no render_config permission
url = reverse('dcim-api:device-detail', kwargs={'pk': device.pk}) + 'render-config/' url = reverse('dcim-api:device-render-config', kwargs={'pk': device.pk})
response = self.client.post(url, {}, format='json', **self.header) response = self.client.post(url, {}, format='json', **self.header)
self.assertHttpStatus(response, status.HTTP_404_NOT_FOUND) self.assertHttpStatus(response, status.HTTP_404_NOT_FOUND)
def test_render_config_token_write_enabled(self):
configtemplate = ConfigTemplate.objects.create(
name='Config Template 1',
template_code='Config for device {{ device.name }}'
)
device = Device.objects.first()
device.config_template = configtemplate
device.save()
self.add_permissions('dcim.render_config_device', 'dcim.view_device')
url = reverse('dcim-api:device-render-config', kwargs={'pk': device.pk})
# Request without token auth should fail with PermissionDenied
response = self.client.post(url, {}, format='json')
self.assertHttpStatus(response, status.HTTP_403_FORBIDDEN)
# Create token with write_enabled=False
token = Token.objects.create(version=2, user=self.user, write_enabled=False)
token_header = f'Bearer {TOKEN_PREFIX}{token.key}.{token.token}'
# Request with write-disabled token should fail
response = self.client.post(url, {}, format='json', HTTP_AUTHORIZATION=token_header)
self.assertHttpStatus(response, status.HTTP_403_FORBIDDEN)
# Enable write and retry
token.write_enabled = True
token.save()
response = self.client.post(url, {}, format='json', HTTP_AUTHORIZATION=token_header)
self.assertHttpStatus(response, status.HTTP_200_OK)
class ModuleTest(APIViewTestCases.APIViewTestCase): class ModuleTest(APIViewTestCases.APIViewTestCase):
model = Module model = Module

View File

@@ -1,7 +1,5 @@
from django.utils.translation import gettext_lazy as _
from jinja2.exceptions import TemplateError from jinja2.exceptions import TemplateError
from rest_framework.decorators import action from rest_framework.decorators import action
from rest_framework.exceptions import PermissionDenied
from rest_framework.renderers import JSONRenderer from rest_framework.renderers import JSONRenderer
from rest_framework.response import Response from rest_framework.response import Response
from rest_framework.status import HTTP_400_BAD_REQUEST from rest_framework.status import HTTP_400_BAD_REQUEST
@@ -91,10 +89,6 @@ class RenderConfigMixin(ConfigTemplateRenderMixin):
'error': f'No config template found for this {object_type}.' 'error': f'No config template found for this {object_type}.'
}, status=HTTP_400_BAD_REQUEST) }, status=HTTP_400_BAD_REQUEST)
# Check view permission for ConfigTemplate
if not request.user.has_perm('extras.view_configtemplate', obj=configtemplate):
raise PermissionDenied(_("This user does not have permission to view this configuration template."))
# Compile context data # Compile context data
context_data = instance.get_config_context() context_data = instance.get_config_context()
context_data.update(request.data) context_data.update(request.data)

View File

@@ -12,7 +12,8 @@ from dcim.models import Device, DeviceRole, DeviceType, Manufacturer, Rack, Loca
from extras.choices import * from extras.choices import *
from extras.models import * from extras.models import *
from extras.scripts import BooleanVar, IntegerVar, Script as PythonClass, StringVar from extras.scripts import BooleanVar, IntegerVar, Script as PythonClass, StringVar
from users.models import Group, User from users.constants import TOKEN_PREFIX
from users.models import Group, Token, User
from utilities.testing import APITestCase, APIViewTestCases from utilities.testing import APITestCase, APIViewTestCases
@@ -859,7 +860,7 @@ class ConfigTemplateTest(APIViewTestCases.APIViewTestCase):
configtemplate = ConfigTemplate.objects.first() configtemplate = ConfigTemplate.objects.first()
self.add_permissions('extras.render_configtemplate', 'extras.view_configtemplate') self.add_permissions('extras.render_configtemplate', 'extras.view_configtemplate')
url = reverse('extras-api:configtemplate-detail', kwargs={'pk': configtemplate.pk}) + 'render/' url = reverse('extras-api:configtemplate-render', kwargs={'pk': configtemplate.pk})
response = self.client.post(url, {'foo': 'bar'}, format='json', **self.header) response = self.client.post(url, {'foo': 'bar'}, format='json', **self.header)
self.assertHttpStatus(response, status.HTTP_200_OK) self.assertHttpStatus(response, status.HTTP_200_OK)
self.assertEqual(response.data['content'], 'Foo: bar') self.assertEqual(response.data['content'], 'Foo: bar')
@@ -868,10 +869,34 @@ class ConfigTemplateTest(APIViewTestCases.APIViewTestCase):
configtemplate = ConfigTemplate.objects.first() configtemplate = ConfigTemplate.objects.first()
# No permissions added - user has no render permission # No permissions added - user has no render permission
url = reverse('extras-api:configtemplate-detail', kwargs={'pk': configtemplate.pk}) + 'render/' url = reverse('extras-api:configtemplate-render', kwargs={'pk': configtemplate.pk})
response = self.client.post(url, {'foo': 'bar'}, format='json', **self.header) response = self.client.post(url, {'foo': 'bar'}, format='json', **self.header)
self.assertHttpStatus(response, status.HTTP_404_NOT_FOUND) self.assertHttpStatus(response, status.HTTP_404_NOT_FOUND)
def test_render_token_write_enabled(self):
configtemplate = ConfigTemplate.objects.first()
self.add_permissions('extras.render_configtemplate', 'extras.view_configtemplate')
url = reverse('extras-api:configtemplate-render', kwargs={'pk': configtemplate.pk})
# Request without token auth should fail with PermissionDenied
response = self.client.post(url, {'foo': 'bar'}, format='json')
self.assertHttpStatus(response, status.HTTP_403_FORBIDDEN)
# Create token with write_enabled=False
token = Token.objects.create(version=2, user=self.user, write_enabled=False)
token_header = f'Bearer {TOKEN_PREFIX}{token.key}.{token.token}'
# Request with write-disabled token should fail
response = self.client.post(url, {'foo': 'bar'}, format='json', HTTP_AUTHORIZATION=token_header)
self.assertHttpStatus(response, status.HTTP_403_FORBIDDEN)
# Enable write and retry
token.write_enabled = True
token.save()
response = self.client.post(url, {'foo': 'bar'}, format='json', HTTP_AUTHORIZATION=token_header)
self.assertHttpStatus(response, status.HTTP_200_OK)
class ScriptTest(APITestCase): class ScriptTest(APITestCase):

View File

@@ -169,10 +169,13 @@ class TokenWritePermission(BasePermission):
Verify the token has write_enabled for unsafe methods, without requiring specific model permissions. Verify the token has write_enabled for unsafe methods, without requiring specific model permissions.
Used for custom actions that accept user data but don't map to standard CRUD operations. Used for custom actions that accept user data but don't map to standard CRUD operations.
""" """
def has_permission(self, request, view): def has_permission(self, request, view):
if isinstance(request.auth, Token): if not isinstance(request.auth, Token):
return request.method in SAFE_METHODS or request.auth.write_enabled raise exceptions.PermissionDenied(
return True "TokenWritePermission requires token authentication."
)
return bool(request.method in SAFE_METHODS or request.auth.write_enabled)
class IsAuthenticatedOrLoginNotRequired(BasePermission): class IsAuthenticatedOrLoginNotRequired(BasePermission):

View File

@@ -12,6 +12,8 @@ from extras.choices import CustomFieldTypeChoices
from extras.models import ConfigTemplate, CustomField from extras.models import ConfigTemplate, CustomField
from ipam.choices import VLANQinQRoleChoices from ipam.choices import VLANQinQRoleChoices
from ipam.models import Prefix, VLAN, VRF from ipam.models import Prefix, VLAN, VRF
from users.constants import TOKEN_PREFIX
from users.models import Token
from utilities.testing import ( from utilities.testing import (
APITestCase, APIViewTestCases, create_test_device, create_test_virtualmachine, disable_logging, APITestCase, APIViewTestCases, create_test_device, create_test_virtualmachine, disable_logging,
) )
@@ -282,10 +284,9 @@ class VirtualMachineTest(APIViewTestCases.APIViewTestCase):
vm.save() vm.save()
self.add_permissions( self.add_permissions(
'virtualization.render_config_virtualmachine', 'virtualization.view_virtualmachine', 'virtualization.render_config_virtualmachine', 'virtualization.view_virtualmachine'
'extras.view_configtemplate'
) )
url = reverse('virtualization-api:virtualmachine-detail', kwargs={'pk': vm.pk}) + 'render-config/' url = reverse('virtualization-api:virtualmachine-render-config', kwargs={'pk': vm.pk})
response = self.client.post(url, {}, format='json', **self.header) response = self.client.post(url, {}, format='json', **self.header)
self.assertHttpStatus(response, status.HTTP_200_OK) self.assertHttpStatus(response, status.HTTP_200_OK)
self.assertEqual(response.data['content'], f'Config for virtual machine {vm.name}') self.assertEqual(response.data['content'], f'Config for virtual machine {vm.name}')
@@ -301,10 +302,41 @@ class VirtualMachineTest(APIViewTestCases.APIViewTestCase):
vm.save() vm.save()
# No permissions added - user has no render_config permission # No permissions added - user has no render_config permission
url = reverse('virtualization-api:virtualmachine-detail', kwargs={'pk': vm.pk}) + 'render-config/' url = reverse('virtualization-api:virtualmachine-render-config', kwargs={'pk': vm.pk})
response = self.client.post(url, {}, format='json', **self.header) response = self.client.post(url, {}, format='json', **self.header)
self.assertHttpStatus(response, status.HTTP_404_NOT_FOUND) self.assertHttpStatus(response, status.HTTP_404_NOT_FOUND)
def test_render_config_token_write_enabled(self):
configtemplate = ConfigTemplate.objects.create(
name='Config Template 1',
template_code='Config for virtual machine {{ virtualmachine.name }}'
)
vm = VirtualMachine.objects.first()
vm.config_template = configtemplate
vm.save()
self.add_permissions('virtualization.render_config_virtualmachine', 'virtualization.view_virtualmachine')
url = reverse('virtualization-api:virtualmachine-render-config', kwargs={'pk': vm.pk})
# Request without token auth should fail with PermissionDenied
response = self.client.post(url, {}, format='json')
self.assertHttpStatus(response, status.HTTP_403_FORBIDDEN)
# Create token with write_enabled=False
token = Token.objects.create(version=2, user=self.user, write_enabled=False)
token_header = f'Bearer {TOKEN_PREFIX}{token.key}.{token.token}'
# Request with write-disabled token should fail
response = self.client.post(url, {}, format='json', HTTP_AUTHORIZATION=token_header)
self.assertHttpStatus(response, status.HTTP_403_FORBIDDEN)
# Enable write and retry
token.write_enabled = True
token.save()
response = self.client.post(url, {}, format='json', HTTP_AUTHORIZATION=token_header)
self.assertHttpStatus(response, status.HTTP_200_OK)
class VMInterfaceTest(APIViewTestCases.APIViewTestCase): class VMInterfaceTest(APIViewTestCases.APIViewTestCase):
model = VMInterface model = VMInterface