diff --git a/netbox/core/tests/test_views.py b/netbox/core/tests/test_views.py index ad89d002a..2d1060e7e 100644 --- a/netbox/core/tests/test_views.py +++ b/netbox/core/tests/test_views.py @@ -105,6 +105,25 @@ class DataFileTestCase( ) DataFile.objects.bulk_create(data_files) + def test_content_is_not_cacheable(self): + """ + The detail view renders file content inline, which may include plaintext secrets, so the + response must instruct the browser not to persist it to its local cache. + """ + datafile = DataFile.objects.first() + datafile.data = b'super-secret-password' + datafile.save() + + self.add_permissions('core.view_datafile') + response = self.client.get(datafile.get_absolute_url()) + self.assertHttpStatus(response, 200) + + # Confirm the content is in fact rendered in the response + self.assertIn('super-secret-password', str(response.content)) + + # Confirm the response is not cacheable + self.assertNotCacheable(response) + class JobTestCase( ViewTestCases.GetObjectViewTestCase, diff --git a/netbox/core/views.py b/netbox/core/views.py index 14887eb18..ab5a428a7 100644 --- a/netbox/core/views.py +++ b/netbox/core/views.py @@ -12,8 +12,10 @@ from django.db import DatabaseError, connection from django.http import Http404, HttpResponse, HttpResponseForbidden from django.shortcuts import get_object_or_404, redirect, render from django.urls import reverse +from django.utils.decorators import method_decorator from django.utils.http import content_disposition_header from django.utils.translation import gettext_lazy as _ +from django.views.decorators.cache import never_cache from django.views.generic import View from django_rq.queues import get_queue_by_index, get_redis_connection from django_rq.settings import get_queues_list, get_queues_map @@ -195,6 +197,7 @@ class DataFileListView(generic.ObjectListView): @register_model_view(DataFile) +@method_decorator(never_cache, name='dispatch') class DataFileView(generic.ObjectView): queryset = DataFile.objects.all() actions = (DeleteObject,) diff --git a/netbox/dcim/tests/test_views.py b/netbox/dcim/tests/test_views.py index ba0c20091..9d50dfdd5 100644 --- a/netbox/dcim/tests/test_views.py +++ b/netbox/dcim/tests/test_views.py @@ -16,7 +16,7 @@ from core.models import ObjectChange, ObjectType from dcim.choices import * from dcim.constants import * from dcim.models import * -from extras.models import ConfigTemplate +from extras.models import ConfigContext, ConfigTemplate from ipam.models import ASN, RIR, VLAN, VRF from netbox.choices import CSVDelimiterChoices, ImportFormatChoices, WeightUnitChoices from tenancy.models import Tenant @@ -2538,6 +2538,54 @@ class DeviceTestCase(ViewTestCases.PrimaryObjectViewTestCase): self.assertHttpStatus(response, 200) self.assertIn(b'Error rendering template', response.content) + def test_device_configcontext_is_not_cacheable(self): + """ + The config context tab renders the merged context data, which may contain sensitive + values, so the response must not be cached by the browser. + """ + ConfigContext.objects.create(name='Config Context 1', data={'password': 'super-secret-password'}) + device = Device.objects.first() + + self.add_permissions('dcim.view_device', 'extras.view_configcontext') + url = reverse('dcim:device_configcontext', kwargs={'pk': device.pk}) + response = self.client.get(url) + self.assertHttpStatus(response, 200) + + # Confirm the context data is in fact rendered in the response + self.assertIn(b'super-secret-password', response.content) + + self.assertNotCacheable(response) + + def test_device_renderconfig_is_not_cacheable(self): + """ + The render config tab renders the config template with context data substituted into it, + which may contain sensitive values, so the response must not be cached by the browser. + """ + configtemplate = ConfigTemplate.objects.create( + name='Test Config Template', + template_code='enable secret super-secret-password' + ) + device = Device.objects.first() + device.config_template = configtemplate + device.save() + + self.add_permissions('dcim.view_device', 'dcim.render_config_device') + url = reverse('dcim:device_render-config', kwargs={'pk': device.pk}) + + response = self.client.get(url) + self.assertHttpStatus(response, 200) + + # Confirm the rendered config is in fact present in the response + self.assertIn(b'super-secret-password', response.content) + + self.assertNotCacheable(response) + + # The direct export of the rendered config must not be cached either + response = self.client.get(url, {'export': 1}) + self.assertHttpStatus(response, 200) + self.assertIn(b'super-secret-password', response.content) + self.assertNotCacheable(response) + def test_device_role_display_colored(self): parent_role = DeviceRole.objects.create(name='Parent Role', slug='parent-role', color='111111') child_role = DeviceRole.objects.create(name='Child Role', slug='child-role', parent=parent_role, color='aa00bb') diff --git a/netbox/extras/tests/test_views.py b/netbox/extras/tests/test_views.py index a977b4f2f..800aa3158 100644 --- a/netbox/extras/tests/test_views.py +++ b/netbox/extras/tests/test_views.py @@ -589,6 +589,24 @@ class ExportTemplateTestCase(ViewTestCases.PrimaryObjectViewTestCase): 'as_attachment': True, } + def test_content_is_not_cacheable(self): + """ + The detail view renders the template code inline, which may have been synced from a data + file containing sensitive values, so the response must not be cached by the browser. + """ + export_template = ExportTemplate.objects.first() + export_template.template_code = 'super-secret-password' + export_template.save() + + self.add_permissions('extras.view_exporttemplate') + response = self.client.get(export_template.get_absolute_url()) + self.assertHttpStatus(response, 200) + + # Confirm the template code is in fact rendered in the response + self.assertIn(b'super-secret-password', response.content) + + self.assertNotCacheable(response) + class ExportTemplateExportFlowTestCase(TestCase): """ @@ -852,6 +870,31 @@ class ConfigContextProfileTestCase(ViewTestCases.PrimaryObjectViewTestCase): f"{profiles[2].pk},New description", ) + def test_content_is_not_cacheable(self): + """ + The detail view renders the schema inline, which may have been synced from a data file + containing sensitive values, so the response must not be cached by the browser. + """ + instance = ConfigContextProfile.objects.first() + instance.schema = { + 'properties': { + 'password': { + 'type': 'string', + 'default': 'super-secret-password', + } + } + } + instance.save() + + self.add_permissions('extras.view_configcontextprofile') + response = self.client.get(instance.get_absolute_url()) + self.assertHttpStatus(response, 200) + + # Confirm the schema is in fact rendered in the response + self.assertIn(b'super-secret-password', response.content) + + self.assertNotCacheable(response) + # TODO: Change base class to PrimaryObjectViewTestCase # Blocked by absence of standard create/edit, bulk create views @@ -902,6 +945,24 @@ class ConfigContextTestCase( 'description': 'New description', } + def test_content_is_not_cacheable(self): + """ + The detail view renders the data inline, which may have been synced from a data + file containing sensitive values, so the response must not be cached by the browser. + """ + instance = ConfigContext.objects.first() + instance.data = {'password': 'super-secret-password'} + instance.save() + + self.add_permissions('extras.view_configcontext') + response = self.client.get(instance.get_absolute_url()) + self.assertHttpStatus(response, 200) + + # Confirm the context data is in fact rendered in the response + self.assertIn(b'super-secret-password', response.content) + + self.assertNotCacheable(response) + class ConfigTemplateTestCase( ViewTestCases.GetObjectViewTestCase, @@ -959,6 +1020,24 @@ class ConfigTemplateTestCase( 'as_attachment': True, } + def test_content_is_not_cacheable(self): + """ + The detail view renders the template code inline, which may have been synced from a data + file containing sensitive values, so the response must not be cached by the browser. + """ + instance = ConfigTemplate.objects.first() + instance.template_code = 'super-secret-password' + instance.save() + + self.add_permissions('extras.view_configtemplate') + response = self.client.get(instance.get_absolute_url()) + self.assertHttpStatus(response, 200) + + # Confirm the template code is in fact rendered in the response + self.assertIn(b'super-secret-password', response.content) + + self.assertNotCacheable(response) + class JournalEntryTestCase( # ViewTestCases.GetObjectViewTestCase, diff --git a/netbox/extras/views.py b/netbox/extras/views.py index 8ce407e9b..a4b412071 100644 --- a/netbox/extras/views.py +++ b/netbox/extras/views.py @@ -9,9 +9,11 @@ from django.http import Http404, HttpResponse, HttpResponseBadRequest, HttpRespo from django.shortcuts import get_object_or_404, redirect, render from django.urls import reverse from django.utils import timezone +from django.utils.decorators import method_decorator from django.utils.http import content_disposition_header from django.utils.module_loading import import_string from django.utils.translation import gettext_lazy as _ +from django.views.decorators.cache import never_cache from django.views.generic import View from core.choices import ManagedFileRootPathChoices @@ -308,6 +310,7 @@ class ExportTemplateListView(generic.ObjectListView): @register_model_view(ExportTemplate) +@method_decorator(never_cache, name='dispatch') class ExportTemplateView(generic.ObjectView): queryset = ExportTemplate.objects.all() template_name = 'generic/object.html' @@ -994,6 +997,7 @@ class ConfigContextProfileListView(generic.ObjectListView): @register_model_view(ConfigContextProfile) +@method_decorator(never_cache, name='dispatch') class ConfigContextProfileView(generic.ObjectView): queryset = ConfigContextProfile.objects.all() template_name = 'generic/object.html' @@ -1069,6 +1073,7 @@ class ConfigContextListView(generic.ObjectListView): @register_model_view(ConfigContext) +@method_decorator(never_cache, name='dispatch') class ConfigContextView(generic.ObjectView): queryset = ConfigContext.objects.all() template_name = 'generic/object.html' @@ -1155,6 +1160,7 @@ class ConfigContextBulkSyncDataView(generic.BulkSyncDataView): queryset = ConfigContext.objects.all() +@method_decorator(never_cache, name='dispatch') class ObjectConfigContextView(generic.ObjectView): base_template = None template_name = 'extras/object_configcontext.html' @@ -1199,6 +1205,7 @@ class ConfigTemplateListView(generic.ObjectListView): @register_model_view(ConfigTemplate) +@method_decorator(never_cache, name='dispatch') class ConfigTemplateView(generic.ObjectView): queryset = ConfigTemplate.objects.all() template_name = 'generic/object.html' @@ -1260,6 +1267,7 @@ class ConfigTemplateBulkSyncDataView(generic.BulkSyncDataView): queryset = ConfigTemplate.objects.all() +@method_decorator(never_cache, name='dispatch') class ObjectRenderConfigView(generic.ObjectView): base_template = None template_name = 'extras/object_render_config.html' diff --git a/netbox/utilities/testing/base.py b/netbox/utilities/testing/base.py index d697d4eb6..8aee671d1 100644 --- a/netbox/utilities/testing/base.py +++ b/netbox/utilities/testing/base.py @@ -125,6 +125,20 @@ class TestCase(_TestCase): err_message = f"Expected HTTP status {expected_status}; received {response.status_code}: {err}" self.assertEqual(response.status_code, expected_status, err_message) + def assertNotCacheable(self, response): + """ + TestCase method. Assert that a response instructs the browser not to persist its content + to the local cache. Views which render potentially sensitive content (e.g. the contents of + a synced data file) must not be written to the browser's cache, where they would remain + readable after the session has ended. + """ + cache_control = response.headers.get('Cache-Control', '') + self.assertIn( + 'no-store', + cache_control, + f"Expected a no-store cache directive; received Cache-Control: '{cache_control}'" + ) + class ModelTestCase(TestCase): """ diff --git a/netbox/virtualization/tests/test_views.py b/netbox/virtualization/tests/test_views.py index 06b2b9091..f10faff35 100644 --- a/netbox/virtualization/tests/test_views.py +++ b/netbox/virtualization/tests/test_views.py @@ -5,7 +5,7 @@ from django.urls import reverse from dcim.choices import InterfaceModeChoices from dcim.models import DeviceRole, Platform, Site -from extras.models import ConfigTemplate +from extras.models import ConfigContext, ConfigTemplate from ipam.models import VLAN, VRF from utilities.testing import ViewTestCases, create_tags, create_test_device, create_test_virtualmachine from virtualization.choices import * @@ -556,6 +556,54 @@ class VirtualMachineTestCase(ViewTestCases.PrimaryObjectViewTestCase): self.assertHttpStatus(response, 200) self.assertIn(b'Error rendering template', response.content) + def test_virtualmachine_configcontext_is_not_cacheable(self): + """ + The config context tab renders the merged context data, which may contain sensitive + values, so the response must not be cached by the browser. + """ + ConfigContext.objects.create(name='Config Context 1', data={'password': 'super-secret-password'}) + vm = VirtualMachine.objects.first() + + self.add_permissions('virtualization.view_virtualmachine', 'extras.view_configcontext') + url = reverse('virtualization:virtualmachine_configcontext', kwargs={'pk': vm.pk}) + response = self.client.get(url) + self.assertHttpStatus(response, 200) + + # Confirm the context data is in fact rendered in the response + self.assertIn(b'super-secret-password', response.content) + + self.assertNotCacheable(response) + + def test_virtualmachine_renderconfig_is_not_cacheable(self): + """ + The render config tab renders the config template with context data substituted into it, + which may contain sensitive values, so the response must not be cached by the browser. + """ + configtemplate = ConfigTemplate.objects.create( + name='Test Config Template', + template_code='enable secret super-secret-password' + ) + vm = VirtualMachine.objects.first() + vm.config_template = configtemplate + vm.save() + + self.add_permissions('virtualization.view_virtualmachine', 'virtualization.render_config_virtualmachine') + url = reverse('virtualization:virtualmachine_render-config', kwargs={'pk': vm.pk}) + + response = self.client.get(url) + self.assertHttpStatus(response, 200) + + # Confirm the rendered config is in fact present in the response + self.assertIn(b'super-secret-password', response.content) + + self.assertNotCacheable(response) + + # The direct export of the rendered config must not be cached either + response = self.client.get(url, {'export': 1}) + self.assertHttpStatus(response, 200) + self.assertIn(b'super-secret-password', response.content) + self.assertNotCacheable(response) + class VMInterfaceTestCase(ViewTestCases.DeviceComponentViewTestCase): model = VMInterface