From c92793d97cb227dc3398f97f572fa015a4a7abee Mon Sep 17 00:00:00 2001 From: Gagan Deep Date: Tue, 29 Jul 2025 22:55:22 +0530 Subject: [PATCH 1/2] [fix:ux] Fixed rendering of JSON fields in readonly mode #848 Fixes #848 --- openwisp_controller/config/admin.py | 68 +++++- .../config/css/readonly-json-widget.css | 13 ++ .../config/tests/test_selenium.py | 215 +++++++++++++++++- openwisp_controller/config/widgets.py | 22 ++ 4 files changed, 313 insertions(+), 5 deletions(-) create mode 100644 openwisp_controller/config/static/config/css/readonly-json-widget.css diff --git a/openwisp_controller/config/admin.py b/openwisp_controller/config/admin.py index b78d048f6..fe4f67723 100644 --- a/openwisp_controller/config/admin.py +++ b/openwisp_controller/config/admin.py @@ -2,6 +2,7 @@ import logging from collections.abc import Iterable +import jsonfield import reversion from django import forms from django.conf import settings @@ -9,12 +10,14 @@ from django.contrib.admin import helpers from django.contrib.admin.actions import delete_selected from django.contrib.admin.models import ADDITION, LogEntry +from django.contrib.auth import get_permission_codename from django.contrib.contenttypes.models import ContentType from django.core.exceptions import ( FieldDoesNotExist, ObjectDoesNotExist, ValidationError, ) +from django.db import models from django.http import Http404, HttpResponse, HttpResponseRedirect, JsonResponse from django.http.response import HttpResponseForbidden from django.shortcuts import get_object_or_404 @@ -45,7 +48,7 @@ from .exportable import DeviceResource from .filters import DeviceGroupFilter, GroupFilter, TemplatesFilter from .utils import send_file -from .widgets import DeviceGroupJsonSchemaWidget, JsonSchemaWidget +from .widgets import DeviceGroupJsonSchemaWidget, JsonSchemaWidget, ReadOnlyJsonWidget logger = logging.getLogger(__name__) prefix = "config/" @@ -102,7 +105,63 @@ def get_extra(self, request, obj=None, **kwargs): return super().get_extra(request, obj, **kwargs) -class BaseConfigAdmin(BaseAdmin): +class ReadOnlyJsonFieldMixin(object): + """ + Mixin to display JSON fields as read-only for users without change permission. + + This mixin overrides the default widgets for JSON fields, replacing them with + read-only alternatives when the user lacks permission to modify the fields. + + It works around a Django limitation that prevents a single widget from being reused + in both editable and read-only contexts. + + For context, see: https://code.djangoproject.com/ticket/30577#comment:10 + """ + + def _change_json_fields_widgets(self, obj, form, can_change): + if obj and not can_change: + for field in form._meta.fields: + try: + if isinstance( + self.model._meta.get_field(field), + (models.JSONField, jsonfield.JSONField), + ): + form.base_fields[field] = forms.JSONField( + widget=ReadOnlyJsonWidget, disabled=True + ) + except FieldDoesNotExist: + # A field could be a Model property or ModelAdmin method, + # which does not exist in the model. + continue + + def get_formset(self, request, obj=None, **kwargs): + """ + This method is used by the inline admin to generate the form. + """ + formset = super().get_formset(request, obj, **kwargs) + form = formset.form + # The user should have change permission on both the parent model + # and the inline model to be able to change the JSON fields. + can_change = self.has_change_permission(request, obj) and request.user.has_perm( + "{}.{}".format( + self.parent_model._meta.app_label, + get_permission_codename("change", self.parent_model._meta), + ) + ) + self._change_json_fields_widgets(obj, form, can_change) + return formset + + def get_form(self, request, obj=None, **kwargs): + """ + This method is used by the ModelAdmin to generate the form. + """ + form = super().get_form(request, obj, **kwargs) + can_change = self.has_change_permission(request, obj) + self._change_json_fields_widgets(obj, form, can_change) + return form + + +class BaseConfigAdmin(ReadOnlyJsonFieldMixin, BaseAdmin): change_form_template = "admin/config/change_form.html" preview_template = None actions_on_bottom = True @@ -420,6 +479,7 @@ class Meta(BaseForm.Meta): class ConfigInline( + ReadOnlyJsonFieldMixin, DeactivatedDeviceReadOnlyMixin, MultitenantAdminMixin, TimeReadonlyAdminMixin, @@ -1265,7 +1325,7 @@ class Meta(BaseForm.Meta): widgets = {"meta_data": DeviceGroupJsonSchemaWidget, "context": FlatJsonWidget} -class DeviceGroupAdmin(MultitenantAdminMixin, BaseAdmin): +class DeviceGroupAdmin(MultitenantAdminMixin, ReadOnlyJsonFieldMixin, BaseAdmin): change_form_template = "admin/device_group/change_form.html" form = DeviceGroupForm list_display = [ @@ -1359,7 +1419,7 @@ class ConfigSettingsForm(AlwaysHasChangedMixin, forms.ModelForm): class Meta: widgets = {"context": FlatJsonWidget} - class ConfigSettingsInline(admin.StackedInline): + class ConfigSettingsInline(ReadOnlyJsonFieldMixin, admin.StackedInline): model = OrganizationConfigSettings form = ConfigSettingsForm diff --git a/openwisp_controller/config/static/config/css/readonly-json-widget.css b/openwisp_controller/config/static/config/css/readonly-json-widget.css new file mode 100644 index 000000000..a4cf7c99e --- /dev/null +++ b/openwisp_controller/config/static/config/css/readonly-json-widget.css @@ -0,0 +1,13 @@ +pre.readonly-json-widget { + margin-top: 0px; + color: var(--body-loud-color); + background: var(--darkened-bg); + padding: 6px 10px; + font-size: inherit; + margin-top: 0; +} +.readonly:has(pre.readonly-json-widget) { + width: 100%; + overflow: auto; + padding-top: 0 !important; +} diff --git a/openwisp_controller/config/tests/test_selenium.py b/openwisp_controller/config/tests/test_selenium.py index ae9061d39..6e4556974 100644 --- a/openwisp_controller/config/tests/test_selenium.py +++ b/openwisp_controller/config/tests/test_selenium.py @@ -1,5 +1,7 @@ +import json import time +from django.contrib.auth.models import Permission from django.contrib.staticfiles.testing import StaticLiveServerTestCase from django.test import tag from django.urls.base import reverse @@ -13,10 +15,16 @@ from openwisp_utils.tests import SeleniumTestMixin as BaseSeleniumTestMixin -from .utils import CreateConfigTemplateMixin, TestVpnX509Mixin, TestWireguardVpnMixin +from .utils import ( + CreateConfigTemplateMixin, + CreateDeviceGroupMixin, + TestVpnX509Mixin, + TestWireguardVpnMixin, +) Device = load_model("config", "Device") DeviceGroup = load_model("config", "DeviceGroup") +OrganizationConfigSettings = load_model("config", "OrganizationConfigSettings") Cert = load_model("django_x509", "Cert") @@ -44,6 +52,80 @@ def _verify_templates_visibility(self, hidden=None, visible=None): for template in visible: self.wait_for_visibility(By.XPATH, f'//*[@value="{template.id}"]') + def _create_readonly_user( + self, username="readonly_user", email="readonly@openwisp.org", organization=None + ): + """ + Creates a readonly user with staff privileges and view-only permissions. + Returns the user object. + """ + readonly_user = self._create_user(username=username, email=email, is_staff=True) + org = organization or self._get_org() + self._create_org_user(user=readonly_user, organization=org, is_admin=True) + readonly_user.user_permissions.add( + *Permission.objects.filter( + codename__in=[ + "view_device", + "view_template", + "view_vpn", + "view_config", + "view_devicegroup", + "view_organization", + "view_organizationconfigsettings", + ] + ) + ) + return readonly_user + + def _test_readonly_json_fields( + self, + url, + field_selectors, + scroll_to_bottom=True, + hide_loading_overlay=True, + user=None, + ): + """ + Reusable method to test readonly JSON fields rendering. + + Args: + url: The URL to open for testing + field_selectors: Dictionary where key is CSS selector and value is + expected text content + scroll_to_bottom: Whether to scroll to bottom of page (default: True) + user: User object to login as. If None, creates a readonly user + (default: None) + """ + if user is None: + org = self._get_org() + user = self._create_readonly_user(organization=org) + + self.login(username=user.username, password="tester") + self.open(url) + if hide_loading_overlay: + self.hide_loading_overlay() + + if scroll_to_bottom: + self.web_driver.execute_script( + "window.scrollTo(0, document.body.scrollHeight);" + ) + + for css_selector, expected_content in field_selectors.items(): + readonly_element = self.find_element( + by=By.CSS_SELECTOR, + value=css_selector, + ) + self.assertEqual(readonly_element.is_displayed(), True) + if isinstance(expected_content, dict): + # If expected_content is a dict, format it as JSON + self.assertEqual( + readonly_element.text, + json.dumps(expected_content, indent=4), + ) + else: + # Otherwise, check if the text contains the expected content + self.assertIn(expected_content, readonly_element.text) + @tag("selenium_tests") class TestDeviceAdmin( @@ -373,11 +455,73 @@ def test_add_remove_templates(self): self.assertEqual(config.templates.count(), 0) self.assertEqual(config.status, "modified") + def test_readonly_config_fields(self): + """ + Test that configuration variables and configuration render properly + when the device only has read only permission. + """ + org = self._get_org() + readonly_user = self._create_readonly_user(organization=org) + + template = self._create_template( + organization=org, + default_values={"mac_address": "00:00:00:00:00:00", "ssid": "OpenWisp"}, + config={ + "interfaces": [ + { + "name": "wlan0", + "network": "br-lan", + "type": "wireless", + "wireless": { + "mode": "access_point", + "radio": "radio0", + "ssid": "{{ ssid }}", + }, + } + ] + }, + ) + device = self._create_device(organization=org) + config = self._create_config( + device=device, + context={"hostname": "readonly-device", "ssid": "ReadOnlyWiFi"}, + ) + config.templates.add(template) + + with self.subTest("Template default values and config rendered as readonly"): + template_url = reverse("admin:config_template_change", args=[template.id]) + template_selectors = { + ".field-default_values .readonly pre.readonly-json-widget": ( + template.default_values + ), + ".field-config .readonly pre.readonly-json-widget": template.config, + } + self._test_readonly_json_fields( + url=template_url, field_selectors=template_selectors, user=readonly_user + ) + + with self.subTest("Device configuration variables rendered as readonly"): + device_url = ( + reverse("admin:config_device_change", args=[device.id]) + + "#config-group" + ) + device_selectors = { + ".field-context .readonly pre.readonly-json-widget": { + "hostname": "readonly-device", + "ssid": "ReadOnlyWiFi", + }, + ".field-config .readonly pre.readonly-json-widget": config.config, + } + self._test_readonly_json_fields( + url=device_url, field_selectors=device_selectors, user=readonly_user + ) + @tag("selenium_tests") class TestDeviceGroupAdmin( SeleniumTestMixin, CreateConfigTemplateMixin, + CreateDeviceGroupMixin, StaticLiveServerTestCase, ): def test_show_relevant_templates(self): @@ -476,6 +620,35 @@ def test_show_relevant_templates(self): False, ) + def test_readonly_devicegroup(self): + """ + Test that device group context renders properly + when the user only has read only permission. + """ + org = self._get_org() + readonly_user = self._create_readonly_user(organization=org) + device_group = self._create_device_group( + name="readonly-group", + organization=org, + context={"mesh_id": "readonly-mesh", "vni": "100"}, + ) + + device_group_url = reverse( + "admin:config_devicegroup_change", args=[device_group.id] + ) + device_group_selectors = { + ".field-context .readonly pre.readonly-json-widget": device_group.context, + ".field-meta_data .readonly pre.readonly-json-widget": ( + device_group.meta_data + ), + } + self._test_readonly_json_fields( + url=device_group_url, + field_selectors=device_group_selectors, + user=readonly_user, + hide_loading_overlay=False, + ) + @tag("selenium_tests") class TestDeviceAdminUnsavedChanges( @@ -618,3 +791,43 @@ def test_vpn_edit(self): backend.select_by_visible_text("OpenVPN") self.wait_for_invisibility(by=By.CLASS_NAME, value="field-webhook_endpoint") self.wait_for_invisibility(by=By.CLASS_NAME, value="field-auth_token") + + def test_readonly_vpn_config(self): + """ + Test that VPN configuration renders properly + when the user only has read only permission. + """ + org = self._get_org() + readonly_user = self._create_readonly_user(organization=org) + vpn = self._create_wireguard_vpn(organization=org) + + vpn_url = reverse("admin:config_vpn_change", args=[vpn.id]) + vpn_selectors = { + ".field-config .readonly pre.readonly-json-widget": vpn.config, + } + self._test_readonly_json_fields( + url=vpn_url, field_selectors=vpn_selectors, user=readonly_user + ) + + +@tag("selenium_tests") +class TestOrganizationConfigSettingsInlineAdmin( + SeleniumTestMixin, CreateConfigTemplateMixin, StaticLiveServerTestCase +): + def test_organization_config_settings_readonly_fields(self): + org = self._get_org() + config_settings = OrganizationConfigSettings.objects.create( + organization=org, + context={"key1": "value1", "key2": "value2"}, + ) + readonly_user = self._create_readonly_user(organization=org) + self._test_readonly_json_fields( + url=reverse("admin:openwisp_users_organization_change", args=[org.id]), + field_selectors={ + ".field-context .readonly pre.readonly-json-widget": ( + config_settings.context + ), + }, + user=readonly_user, + hide_loading_overlay=False, + ) diff --git a/openwisp_controller/config/widgets.py b/openwisp_controller/config/widgets.py index 302171108..819536318 100644 --- a/openwisp_controller/config/widgets.py +++ b/openwisp_controller/config/widgets.py @@ -1,12 +1,34 @@ +import json + from django import forms from django.contrib.admin.widgets import AdminTextareaWidget +from django.forms.widgets import Widget from django.template.loader import get_template from django.urls import reverse +from django.utils.safestring import mark_safe from swapper import load_model DeviceGroup = load_model("config", "DeviceGroup") +class ReadOnlyJsonWidget(Widget): + read_only = True + + @property + def media(self): + css = { + "all": [ + "config/css/readonly-json-widget.css", + ] + } + return forms.Media(css=css) + + def render(self, name, value, attrs=None, renderer=None): + value = value or {} + value = json.dumps(value, indent=4, sort_keys=True) + return mark_safe(f'
{value}
') + + class JsonSchemaWidget(AdminTextareaWidget): """ JSON Schema Editor widget From c28442a7ab3834cf0cbf5347c7935fc45f1dab65 Mon Sep 17 00:00:00 2001 From: Gagan Deep Date: Thu, 7 Aug 2025 17:36:01 +0530 Subject: [PATCH 2/2] [tests] Fixed failing tests --- openwisp_controller/config/admin.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/openwisp_controller/config/admin.py b/openwisp_controller/config/admin.py index fe4f67723..5070470b2 100644 --- a/openwisp_controller/config/admin.py +++ b/openwisp_controller/config/admin.py @@ -120,7 +120,8 @@ class ReadOnlyJsonFieldMixin(object): def _change_json_fields_widgets(self, obj, form, can_change): if obj and not can_change: - for field in form._meta.fields: + form_fields = form.base_fields.keys() or form._meta.fields + for field in form_fields: try: if isinstance( self.model._meta.get_field(field),