[fix:ux] Fixed rendering of JSON fields in readonly mode #848

openwisp_controller/config/admin.py

@@ -2,19 +2,22 @@
 import logging
 from collections.abc import Iterable
 
+import jsonfield
 import reversion
 from django import forms
 from django.conf import settings
 from django.contrib import admin, messages
 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,64 @@ 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:
+            form_fields = form.base_fields.keys() or form._meta.fields
+            for field in form_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 +480,7 @@ class Meta(BaseForm.Meta):
 
 
 class ConfigInline(
+    ReadOnlyJsonFieldMixin,
     DeactivatedDeviceReadOnlyMixin,
     MultitenantAdminMixin,
     TimeReadonlyAdminMixin,
@@ -1265,7 +1326,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 +1420,7 @@ class ConfigSettingsForm(AlwaysHasChangedMixin, forms.ModelForm):
         class Meta:
             widgets = {"context": FlatJsonWidget}
 
-    class ConfigSettingsInline(admin.StackedInline):
+    class ConfigSettingsInline(ReadOnlyJsonFieldMixin, admin.StackedInline):
         model = OrganizationConfigSettings
         form = ConfigSettingsForm
 

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;
+}

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="[email protected]", 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,
+        )