Skip to content

Commit d723890

Browse files
committed
[admin] Disallow changing configuration backend from UI #789
Make the backend read-only on Django admin change forms for device configuration, template, and VPN server, while keeping it selectable on create forms. Fixes #789
1 parent 8cf6733 commit d723890

File tree

2 files changed

+121
-5
lines changed

2 files changed

+121
-5
lines changed

openwisp_controller/config/admin.py

Lines changed: 29 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -66,6 +66,15 @@
6666
from django.contrib.admin import ModelAdmin
6767

6868

69+
def is_recover_view(request):
70+
resolver_match = getattr(request, "resolver_match", None)
71+
return getattr(request, "_recover_view", False) or bool(
72+
resolver_match
73+
and resolver_match.url_name
74+
and resolver_match.url_name.endswith("_recover")
75+
)
76+
77+
6978
class SystemDefinedVariableMixin(object):
7079
def system_context(self, obj):
7180
system_context = obj.get_system_context()
@@ -468,7 +477,14 @@ def _error_reason_field_conditional(self, obj, fields):
468477
return fields
469478

470479
def get_readonly_fields(self, request, obj):
471-
fields = super().get_readonly_fields(request, obj)
480+
fields = list(super().get_readonly_fields(request, obj))
481+
if (
482+
obj
483+
and obj._has_config()
484+
and not is_recover_view(request)
485+
and "backend" not in fields
486+
):
487+
fields.append("backend")
472488
return self._error_reason_field_conditional(obj, fields)
473489

474490
def get_fields(self, request, obj):
@@ -1082,6 +1098,12 @@ class TemplateAdmin(MultitenantAdminMixin, BaseConfigAdmin, SystemDefinedVariabl
10821098
readonly_fields = ["system_context"]
10831099
autocomplete_fields = ["vpn"]
10841100

1101+
def get_readonly_fields(self, request, obj=None):
1102+
fields = list(super().get_readonly_fields(request, obj))
1103+
if obj and not is_recover_view(request) and "backend" not in fields:
1104+
fields.append("backend")
1105+
return fields
1106+
10851107
@admin.action(permissions=["add"])
10861108
def clone_selected_templates(self, request, queryset):
10871109
selectable_orgs = None
@@ -1261,6 +1283,12 @@ class VpnAdmin(
12611283
"modified",
12621284
]
12631285

1286+
def get_readonly_fields(self, request, obj=None):
1287+
fields = list(super().get_readonly_fields(request, obj))
1288+
if obj and not is_recover_view(request) and "backend" not in fields:
1289+
fields.append("backend")
1290+
return fields
1291+
12641292
class Media(BaseConfigAdmin):
12651293
js = list(BaseConfigAdmin.Media.js) + [f"{prefix}js/vpn.js"]
12661294

openwisp_controller/config/tests/test_admin.py

Lines changed: 92 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1435,12 +1435,40 @@ def test_default_device_backend(self):
14351435
response = self.client.get(path)
14361436
self.assertContains(response, '<option value="netjsonconfig.OpenWrt" selected')
14371437

1438-
def test_existing_device_backend(self):
1438+
def test_device_backend_readonly_on_change(self):
14391439
d = self._create_device()
14401440
self._create_config(device=d, backend="netjsonconfig.OpenWisp")
14411441
path = reverse(f"admin:{self.app_label}_device_change", args=[d.pk])
14421442
response = self.client.get(path)
1443-
self.assertContains(response, '<option value="netjsonconfig.OpenWisp" selected')
1443+
self.assertContains(response, "field-backend")
1444+
self.assertNotContains(response, 'name="config-0-backend"')
1445+
1446+
def test_device_backend_cannot_be_changed_from_change_form(self):
1447+
template = Template.objects.first()
1448+
device = self._create_device()
1449+
config = self._create_config(
1450+
device=device,
1451+
backend=template.backend,
1452+
config=template.config,
1453+
)
1454+
path = reverse(f"admin:{self.app_label}_device_change", args=[device.pk])
1455+
params = self._get_device_params(org=device.organization)
1456+
params.update(
1457+
{
1458+
"name": "device-backend-unchanged",
1459+
"config-0-id": str(config.pk),
1460+
"config-0-device": str(device.pk),
1461+
"config-0-backend": "totally.invalid.backend",
1462+
"config-0-templates": str(template.pk),
1463+
"config-INITIAL_FORMS": 1,
1464+
}
1465+
)
1466+
response = self.client.post(path, params)
1467+
self.assertNotContains(response, "errors field-backend", status_code=302)
1468+
config.refresh_from_db()
1469+
self.assertEqual(config.backend, template.backend)
1470+
device.refresh_from_db(fields=["name"])
1471+
self.assertEqual(device.name, "device-backend-unchanged")
14441472

14451473
def test_device_search(self):
14461474
d = self._create_device(name="admin-search-test")
@@ -1502,7 +1530,7 @@ def test_default_template_backend(self):
15021530
response = self.client.get(path)
15031531
self.assertContains(response, '<option value="netjsonconfig.OpenWrt" selected')
15041532

1505-
def test_existing_template_backend(self):
1533+
def test_template_backend_readonly_on_change(self):
15061534
t = Template.objects.first()
15071535
t.backend = "netjsonconfig.OpenWisp"
15081536
t.config = {
@@ -1513,7 +1541,34 @@ def test_existing_template_backend(self):
15131541
t.save()
15141542
path = reverse(f"admin:{self.app_label}_template_change", args=[t.pk])
15151543
response = self.client.get(path)
1516-
self.assertContains(response, '<option value="netjsonconfig.OpenWisp" selected')
1544+
self.assertContains(response, "field-backend")
1545+
self.assertNotContains(response, 'name="backend"')
1546+
1547+
def test_template_backend_cannot_be_changed_from_change_form(self):
1548+
template = self._create_template()
1549+
original_backend = template.backend
1550+
path = reverse(f"admin:{self.app_label}_template_change", args=[template.pk])
1551+
params = {
1552+
"name": "template-backend-unchanged",
1553+
"organization": str(template.organization_id or ""),
1554+
"type": template.type,
1555+
"backend": "totally.invalid.backend",
1556+
"vpn": str(template.vpn_id or ""),
1557+
"tags": ",".join(template.tags.names()),
1558+
"default_values": json.dumps(template.default_values or {}),
1559+
"config": json.dumps(template.config),
1560+
}
1561+
if template.auto_cert:
1562+
params["auto_cert"] = "on"
1563+
if template.default:
1564+
params["default"] = "on"
1565+
if template.required:
1566+
params["required"] = "on"
1567+
response = self.client.post(path, params)
1568+
self.assertNotContains(response, "errors field-backend", status_code=302)
1569+
template.refresh_from_db()
1570+
self.assertEqual(template.backend, original_backend)
1571+
self.assertEqual(template.name, "template-backend-unchanged")
15171572

15181573
def test_preview_variables(self):
15191574
path = reverse(f"admin:{self.app_label}_device_preview")
@@ -1626,6 +1681,39 @@ def test_add_vpn(self):
16261681
response, 'value="openwisp_controller.vpn_backends.OpenVpn" selected'
16271682
)
16281683

1684+
def test_vpn_backend_readonly_on_change(self):
1685+
vpn = self._create_vpn()
1686+
path = reverse(f"admin:{self.app_label}_vpn_change", args=[vpn.pk])
1687+
response = self.client.get(path)
1688+
self.assertContains(response, "field-backend")
1689+
self.assertNotContains(response, 'name="backend"')
1690+
1691+
def test_vpn_backend_cannot_be_changed_from_change_form(self):
1692+
vpn = self._create_vpn()
1693+
original_backend = vpn.backend
1694+
path = reverse(f"admin:{self.app_label}_vpn_change", args=[vpn.pk])
1695+
params = {
1696+
"organization": str(vpn.organization_id or ""),
1697+
"name": "vpn-backend-unchanged",
1698+
"host": vpn.host,
1699+
"key": vpn.key,
1700+
"backend": "totally.invalid.backend",
1701+
"ca": str(vpn.ca_id or ""),
1702+
"cert": str(vpn.cert_id or ""),
1703+
"subnet": str(vpn.subnet_id or ""),
1704+
"ip": str(vpn.ip_id or ""),
1705+
"webhook_endpoint": vpn.webhook_endpoint or "",
1706+
"auth_token": vpn.auth_token or "",
1707+
"notes": vpn.notes or "",
1708+
"dh": vpn.dh or "",
1709+
"config": json.dumps(vpn.config),
1710+
}
1711+
response = self.client.post(path, params)
1712+
self.assertNotContains(response, "errors field-backend", status_code=302)
1713+
vpn.refresh_from_db()
1714+
self.assertEqual(vpn.backend, original_backend)
1715+
self.assertEqual(vpn.name, "vpn-backend-unchanged")
1716+
16291717
def test_vpn_clients_deleted(self):
16301718
def _update_template(templates):
16311719
params.update(

0 commit comments

Comments
 (0)