Skip to content

Commit 6298af4

Browse files
committed
[admin] Disallow changing configuration backend from UI #789
Make backend readonly on admin change forms for device config inline, template, and VPN while keeping backend selection available on add forms.\n\nAdd focused admin tests to verify backend immutability on change forms and that attempted backend edits through UI change requests do not alter persisted backend values.\n\nFixes #789
1 parent 8cf6733 commit 6298af4

File tree

2 files changed

+103
-5
lines changed

2 files changed

+103
-5
lines changed

openwisp_controller/config/admin.py

Lines changed: 15 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -468,7 +468,9 @@ def _error_reason_field_conditional(self, obj, fields):
468468
return fields
469469

470470
def get_readonly_fields(self, request, obj):
471-
fields = super().get_readonly_fields(request, obj)
471+
fields = list(super().get_readonly_fields(request, obj))
472+
if obj and obj._has_config() and "backend" not in fields:
473+
fields.append("backend")
472474
return self._error_reason_field_conditional(obj, fields)
473475

474476
def get_fields(self, request, obj):
@@ -1082,6 +1084,12 @@ class TemplateAdmin(MultitenantAdminMixin, BaseConfigAdmin, SystemDefinedVariabl
10821084
readonly_fields = ["system_context"]
10831085
autocomplete_fields = ["vpn"]
10841086

1087+
def get_readonly_fields(self, request, obj=None):
1088+
fields = list(super().get_readonly_fields(request, obj))
1089+
if obj and "backend" not in fields:
1090+
fields.append("backend")
1091+
return fields
1092+
10851093
@admin.action(permissions=["add"])
10861094
def clone_selected_templates(self, request, queryset):
10871095
selectable_orgs = None
@@ -1261,6 +1269,12 @@ class VpnAdmin(
12611269
"modified",
12621270
]
12631271

1272+
def get_readonly_fields(self, request, obj=None):
1273+
fields = list(super().get_readonly_fields(request, obj))
1274+
if obj and "backend" not in fields:
1275+
fields.append("backend")
1276+
return fields
1277+
12641278
class Media(BaseConfigAdmin):
12651279
js = list(BaseConfigAdmin.Media.js) + [f"{prefix}js/vpn.js"]
12661280

openwisp_controller/config/tests/test_admin.py

Lines changed: 88 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1435,12 +1435,38 @@ 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")
1468+
config.refresh_from_db()
1469+
self.assertEqual(config.backend, template.backend)
14441470

14451471
def test_device_search(self):
14461472
d = self._create_device(name="admin-search-test")
@@ -1502,7 +1528,7 @@ def test_default_template_backend(self):
15021528
response = self.client.get(path)
15031529
self.assertContains(response, '<option value="netjsonconfig.OpenWrt" selected')
15041530

1505-
def test_existing_template_backend(self):
1531+
def test_template_backend_readonly_on_change(self):
15061532
t = Template.objects.first()
15071533
t.backend = "netjsonconfig.OpenWisp"
15081534
t.config = {
@@ -1513,7 +1539,33 @@ def test_existing_template_backend(self):
15131539
t.save()
15141540
path = reverse(f"admin:{self.app_label}_template_change", args=[t.pk])
15151541
response = self.client.get(path)
1516-
self.assertContains(response, '<option value="netjsonconfig.OpenWisp" selected')
1542+
self.assertContains(response, "field-backend")
1543+
self.assertNotContains(response, 'name="backend"')
1544+
1545+
def test_template_backend_cannot_be_changed_from_change_form(self):
1546+
template = self._create_template()
1547+
path = reverse(f"admin:{self.app_label}_template_change", args=[template.pk])
1548+
params = {
1549+
"name": "template-backend-unchanged",
1550+
"organization": str(template.organization_id or ""),
1551+
"type": template.type,
1552+
"backend": "totally.invalid.backend",
1553+
"vpn": str(template.vpn_id or ""),
1554+
"tags": ",".join(template.tags.names()),
1555+
"default_values": json.dumps(template.default_values or {}),
1556+
"config": json.dumps(template.config),
1557+
}
1558+
if template.auto_cert:
1559+
params["auto_cert"] = "on"
1560+
if template.default:
1561+
params["default"] = "on"
1562+
if template.required:
1563+
params["required"] = "on"
1564+
response = self.client.post(path, params)
1565+
self.assertNotContains(response, "errors field-backend", status_code=302)
1566+
template.refresh_from_db()
1567+
self.assertEqual(template.backend, "netjsonconfig.OpenWrt")
1568+
self.assertEqual(template.name, "template-backend-unchanged")
15171569

15181570
def test_preview_variables(self):
15191571
path = reverse(f"admin:{self.app_label}_device_preview")
@@ -1626,6 +1678,38 @@ def test_add_vpn(self):
16261678
response, 'value="openwisp_controller.vpn_backends.OpenVpn" selected'
16271679
)
16281680

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

0 commit comments

Comments
 (0)