mirror of
https://github.com/netbox-community/netbox.git
synced 2026-01-23 12:08:43 -06:00
Address PR feedback on render_config permissions
Remove redundant permission checks, add view permission enforcement via chained restrict() calls, and rename ConfigTemplate permission action from render_config to render for consistency.
This commit is contained in:
@@ -92,8 +92,8 @@ http://netbox:8000/api/extras/config-templates/123/render/ \
|
|||||||
```
|
```
|
||||||
|
|
||||||
!!! note "Permissions"
|
!!! note "Permissions"
|
||||||
Rendering configuration templates via the REST API requires the `render_config` permission for the relevant object type:
|
Rendering configuration templates via the REST API requires appropriate permissions for the relevant object type:
|
||||||
|
|
||||||
* To render a device's configuration via `/api/dcim/devices/{id}/render-config/`, assign a permission for "DCIM > Device" with the `render_config` action
|
* To render a device's configuration via `/api/dcim/devices/{id}/render-config/`, assign a permission for "DCIM > Device" with the `render_config` action
|
||||||
* To render a virtual machine's configuration via `/api/virtualization/virtual-machines/{id}/render-config/`, assign a permission for "Virtualization > Virtual Machine" with the `render_config` action
|
* To render a virtual machine's configuration via `/api/virtualization/virtual-machines/{id}/render-config/`, assign a permission for "Virtualization > Virtual Machine" with the `render_config` action
|
||||||
* To render a config template directly via `/api/extras/config-templates/{id}/render/`, assign a permission for "Extras > Config Template" with the `render_config` action
|
* To render a config template directly via `/api/extras/config-templates/{id}/render/`, assign a permission for "Extras > Config Template" with the `render` action
|
||||||
|
|||||||
@@ -1306,7 +1306,6 @@ class DeviceTest(APIViewTestCases.APIViewTestCase):
|
|||||||
}
|
}
|
||||||
user_permissions = (
|
user_permissions = (
|
||||||
'dcim.view_site', 'dcim.view_rack', 'dcim.view_location', 'dcim.view_devicerole', 'dcim.view_devicetype',
|
'dcim.view_site', 'dcim.view_rack', 'dcim.view_location', 'dcim.view_devicerole', 'dcim.view_devicetype',
|
||||||
'extras.view_configtemplate',
|
|
||||||
)
|
)
|
||||||
|
|
||||||
@classmethod
|
@classmethod
|
||||||
@@ -1486,7 +1485,7 @@ class DeviceTest(APIViewTestCases.APIViewTestCase):
|
|||||||
device.config_template = configtemplate
|
device.config_template = configtemplate
|
||||||
device.save()
|
device.save()
|
||||||
|
|
||||||
self.add_permissions('dcim.render_config_device')
|
self.add_permissions('dcim.render_config_device', 'dcim.view_device', 'extras.view_configtemplate')
|
||||||
url = reverse('dcim-api:device-detail', kwargs={'pk': device.pk}) + 'render-config/'
|
url = reverse('dcim-api:device-detail', kwargs={'pk': device.pk}) + 'render-config/'
|
||||||
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)
|
||||||
|
|||||||
@@ -8,7 +8,6 @@ from rest_framework.status import HTTP_400_BAD_REQUEST
|
|||||||
|
|
||||||
from netbox.api.authentication import TokenWritePermission
|
from netbox.api.authentication import TokenWritePermission
|
||||||
from netbox.api.renderers import TextRenderer
|
from netbox.api.renderers import TextRenderer
|
||||||
from utilities.permissions import get_permission_for_model
|
|
||||||
from .serializers import ConfigTemplateSerializer
|
from .serializers import ConfigTemplateSerializer
|
||||||
|
|
||||||
__all__ = (
|
__all__ = (
|
||||||
@@ -80,14 +79,11 @@ class RenderConfigMixin(ConfigTemplateRenderMixin):
|
|||||||
"""
|
"""
|
||||||
Resolve and render the preferred ConfigTemplate for this Device.
|
Resolve and render the preferred ConfigTemplate for this Device.
|
||||||
"""
|
"""
|
||||||
self.queryset = self.queryset.model.objects.all().restrict(request.user, 'render_config')
|
self.queryset = self.queryset.model.objects.restrict(request.user, 'render_config').restrict(
|
||||||
|
request.user, 'view'
|
||||||
|
)
|
||||||
instance = self.get_object()
|
instance = self.get_object()
|
||||||
|
|
||||||
# Check render_config permission
|
|
||||||
perm = get_permission_for_model(instance, 'render_config')
|
|
||||||
if not request.user.has_perm(perm, obj=instance):
|
|
||||||
raise PermissionDenied(_("This user does not have permission to render configurations for this object."))
|
|
||||||
|
|
||||||
object_type = instance._meta.model_name
|
object_type = instance._meta.model_name
|
||||||
configtemplate = instance.get_config_template()
|
configtemplate = instance.get_config_template()
|
||||||
if not configtemplate:
|
if not configtemplate:
|
||||||
@@ -95,6 +91,10 @@ 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)
|
||||||
|
|||||||
@@ -1,6 +1,5 @@
|
|||||||
from django.http import Http404
|
from django.http import Http404
|
||||||
from django.shortcuts import get_object_or_404
|
from django.shortcuts import get_object_or_404
|
||||||
from django.utils.translation import gettext_lazy as _
|
|
||||||
from django_rq.queues import get_connection
|
from django_rq.queues import get_connection
|
||||||
from drf_spectacular.utils import extend_schema, extend_schema_view
|
from drf_spectacular.utils import extend_schema, extend_schema_view
|
||||||
from rest_framework import status
|
from rest_framework import status
|
||||||
@@ -23,7 +22,6 @@ from netbox.api.metadata import ContentTypeMetadata
|
|||||||
from netbox.api.renderers import TextRenderer
|
from netbox.api.renderers import TextRenderer
|
||||||
from netbox.api.viewsets import BaseViewSet, NetBoxModelViewSet
|
from netbox.api.viewsets import BaseViewSet, NetBoxModelViewSet
|
||||||
from utilities.exceptions import RQWorkerNotRunningException
|
from utilities.exceptions import RQWorkerNotRunningException
|
||||||
from utilities.permissions import get_permission_for_model
|
|
||||||
from utilities.request import copy_safe_request
|
from utilities.request import copy_safe_request
|
||||||
from . import serializers
|
from . import serializers
|
||||||
from .mixins import ConfigTemplateRenderMixin
|
from .mixins import ConfigTemplateRenderMixin
|
||||||
@@ -252,14 +250,9 @@ class ConfigTemplateViewSet(SyncedDataMixin, ConfigTemplateRenderMixin, NetBoxMo
|
|||||||
Render a ConfigTemplate using the context data provided (if any). If the client requests "text/plain" data,
|
Render a ConfigTemplate using the context data provided (if any). If the client requests "text/plain" data,
|
||||||
return the raw rendered content, rather than serialized JSON.
|
return the raw rendered content, rather than serialized JSON.
|
||||||
"""
|
"""
|
||||||
self.queryset = self.queryset.model.objects.all().restrict(request.user, 'render_config')
|
self.queryset = self.queryset.model.objects.restrict(request.user, 'render').restrict(request.user, 'view')
|
||||||
configtemplate = self.get_object()
|
configtemplate = self.get_object()
|
||||||
|
|
||||||
# Check render_config permission
|
|
||||||
perm = get_permission_for_model(configtemplate, 'render_config')
|
|
||||||
if not request.user.has_perm(perm, obj=configtemplate):
|
|
||||||
raise PermissionDenied(_("This user does not have permission to render configuration templates."))
|
|
||||||
|
|
||||||
context = request.data
|
context = request.data
|
||||||
|
|
||||||
return self.render_configtemplate(request, configtemplate, context)
|
return self.render_configtemplate(request, configtemplate, context)
|
||||||
|
|||||||
@@ -858,7 +858,7 @@ class ConfigTemplateTest(APIViewTestCases.APIViewTestCase):
|
|||||||
def test_render(self):
|
def test_render(self):
|
||||||
configtemplate = ConfigTemplate.objects.first()
|
configtemplate = ConfigTemplate.objects.first()
|
||||||
|
|
||||||
self.add_permissions('extras.render_config_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-detail', kwargs={'pk': configtemplate.pk}) + 'render/'
|
||||||
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)
|
||||||
@@ -867,7 +867,7 @@ class ConfigTemplateTest(APIViewTestCases.APIViewTestCase):
|
|||||||
def test_render_without_permission(self):
|
def test_render_without_permission(self):
|
||||||
configtemplate = ConfigTemplate.objects.first()
|
configtemplate = ConfigTemplate.objects.first()
|
||||||
|
|
||||||
# No permissions added - user has no render_config 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-detail', kwargs={'pk': configtemplate.pk}) + 'render/'
|
||||||
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)
|
||||||
|
|||||||
@@ -281,7 +281,10 @@ class VirtualMachineTest(APIViewTestCases.APIViewTestCase):
|
|||||||
vm.config_template = configtemplate
|
vm.config_template = configtemplate
|
||||||
vm.save()
|
vm.save()
|
||||||
|
|
||||||
self.add_permissions('virtualization.render_config_virtualmachine')
|
self.add_permissions(
|
||||||
|
'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-detail', kwargs={'pk': vm.pk}) + 'render-config/'
|
||||||
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)
|
||||||
|
|||||||
Reference in New Issue
Block a user