Skip check for permission to change for related_via attrs (#351)

Attributes on servertypes which are configured to get their value from
another servertype (related_via) should be excluded from the permission
check to change them because the actual change takes place on the target
servertype and is also checked there.

Thus the check would be redundant and annoying because you have to
whitelist the attributes in two places in ACLs.

serveradmin/access_control/fixtures/access_control.json

@@ -0,0 +1,19 @@
+[
+  {
+    "model": "access_control.accesscontrolgroup",
+    "pk": 1,
+    "fields": {
+      "name": "test",
+      "description": "",
+      "query": "servertype=vm",
+      "is_whitelist": true,
+      "members": [],
+      "applications": [
+        2
+      ],
+      "attributes": [
+        "hv"
+      ]
+    }
+  }
+]

serveradmin/access_control/fixtures/apps.json

@@ -0,0 +1,32 @@
+[
+  {
+    "model": "apps.application",
+    "pk": 1,
+    "fields": {
+      "name": "default app for serveradmin",
+      "app_id": "421cbe6b6e6044f785e77267f7f403dd8646be9f",
+      "auth_token": "8igR21kG0yspOm9KRwUwozlu",
+      "owner": 1,
+      "location": "",
+      "disabled": false,
+      "last_login": null,
+      "superuser": true,
+      "allowed_methods": ""
+    }
+  },
+  {
+    "model": "apps.application",
+    "pk": 2,
+    "fields": {
+      "name": "test",
+      "app_id": "52095437c8551641a736fe64c70177e0a32f02c7",
+      "auth_token": "dyiNqQVXS2tjpDAI5iCMlIhb",
+      "owner": 1,
+      "location": "local",
+      "disabled": false,
+      "last_login": null,
+      "superuser": false,
+      "allowed_methods": ""
+    }
+  }
+]

serveradmin/access_control/fixtures/auth_user.json

@@ -0,0 +1,20 @@
+[
+  {
+    "model": "auth.user",
+    "pk": 1,
+    "fields": {
+      "password": "pbkdf2_sha256$260000$JBmjhDUKmEpd9vhvLIClaz$Dp71dvjzlrOL4yWVrKwen1oTy/HHz8Mzio7xt+HoK2g=",
+      "last_login": "2024-04-09T14:36:04.043Z",
+      "is_superuser": true,
+      "username": "serveradmin",
+      "first_name": "",
+      "last_name": "",
+      "email": "",
+      "is_staff": true,
+      "is_active": true,
+      "date_joined": "2024-04-09T14:34:49.985Z",
+      "groups": [],
+      "user_permissions": []
+    }
+  }
+]

serveradmin/access_control/fixtures/serverdb.json

@@ -0,0 +1,93 @@
+[
+  {
+    "model": "serverdb.servertype",
+    "pk": "hv",
+    "fields": {
+      "description": "Hypervisor",
+      "ip_addr_type": "null"
+    }
+  },
+  {
+    "model": "serverdb.servertype",
+    "pk": "vm",
+    "fields": {
+      "description": "Virtual Machine",
+      "ip_addr_type": "null"
+    }
+  },
+  {
+    "model": "serverdb.attribute",
+    "pk": "hv",
+    "fields": {
+      "type": "relation",
+      "multi": false,
+      "hovertext": "",
+      "group": "other",
+      "help_link": null,
+      "inet_address_family": "",
+      "readonly": false,
+      "target_servertype": "hv",
+      "reversed_attribute": null,
+      "clone": false,
+      "history": true,
+      "regexp": "\\A.*\\Z"
+    }
+  },
+  {
+    "model": "serverdb.attribute",
+    "pk": "nic",
+    "fields": {
+      "type": "string",
+      "multi": false,
+      "hovertext": "",
+      "group": "other",
+      "help_link": null,
+      "inet_address_family": "",
+      "readonly": false,
+      "target_servertype": null,
+      "reversed_attribute": null,
+      "clone": false,
+      "history": true,
+      "regexp": "\\A.*\\Z"
+    }
+  },
+  {
+    "model": "serverdb.servertypeattribute",
+    "pk": 1,
+    "fields": {
+      "servertype": "vm",
+      "attribute": "hv",
+      "related_via_attribute": null,
+      "consistent_via_attribute": null,
+      "required": false,
+      "default_value": null,
+      "default_visible": false
+    }
+  },
+  {
+    "model": "serverdb.servertypeattribute",
+    "pk": 2,
+    "fields": {
+      "servertype": "vm",
+      "attribute": "nic",
+      "related_via_attribute": "hv",
+      "consistent_via_attribute": null,
+      "required": false,
+      "default_value": null,
+      "default_visible": false
+    }
+  },
+  {
+    "model": "serverdb.servertypeattribute",
+    "pk": 3,
+    "fields": {
+      "servertype": "hv",
+      "attribute": "nic",
+      "related_via_attribute": null,
+      "consistent_via_attribute": null,
+      "required": false,
+      "default_value": null,
+      "default_visible": false
+    }
+  }
+]

serveradmin/access_control/tests/test_acl.py

@@ -0,0 +1,51 @@
+from django.core.exceptions import PermissionDenied
+from django.test import TransactionTestCase
+
+from serveradmin.apps.models import Application
+from serveradmin.dataset import Query
+
+
+class TestAttributeRelatedViaPermissions(TransactionTestCase):
+    fixtures = ['auth_user.json', 'apps.json', 'serverdb.json', 'access_control.json']
+
+    # See https://github.com/innogames/serveradmin/pull/351
+    def test_can_commit_related_via_attribute(self):
+        hv_1 = Query().new_object("hv")
+        hv_1["hostname"] = "hv-1"
+        hv_1["nic"] = "nic-1"
+        hv_1.commit(app=Application.objects.filter(superuser=True).first())
+
+        hv_2 = Query().new_object("hv")
+        hv_2["hostname"] = "hv-2"
+        hv_2["nic"] = "nic-2"
+        hv_2.commit(app=Application.objects.filter(superuser=True).first())
+
+        vm = Query().new_object("vm")
+        vm["hostname"] = "vm-1"
+        vm["hv"] = "hv-1"
+        vm.commit(app=Application.objects.filter(superuser=True).first())
+
+        vm = Query({"hostname": "vm-1"}, ["hostname", "hv"])
+        vm.update(hv="hv-2")
+        self.assertIsNone(vm.commit(app=Application.objects.get(name="test")))
+
+    # See https://github.com/innogames/serveradmin/pull/351
+    def test_cannot_commit_related_via_attribute_target(self):
+        hv_1 = Query().new_object("hv")
+        hv_1["hostname"] = "hv-1"
+        hv_1["nic"] = "nic-1"
+        hv_1.commit(app=Application.objects.filter(superuser=True).first())
+
+        hv_2 = Query().new_object("hv")
+        hv_2["hostname"] = "hv-2"
+        hv_2["nic"] = "nic-2"
+        hv_2.commit(app=Application.objects.filter(superuser=True).first())
+
+        vm = Query().new_object("vm")
+        vm["hostname"] = "vm-1"
+        vm["hv"] = "hv-1"
+        vm.commit(app=Application.objects.filter(superuser=True).first())
+
+        vm = Query({"hostname": "hv-1"}, ["hostname", "nic"])
+        vm.update(nic="hv-2")
+        self.assertRaises(PermissionDenied, vm.commit, app=Application.objects.get(name="test"))

serveradmin/serverdb/query_committer.py

@@ -18,7 +18,7 @@
     ServerAttribute,
     ServerRelationAttribute,
     ChangeCommit,
-    Change,
+    Change, ServertypeAttribute,
 )
 from serveradmin.serverdb.query_materializer import (
     QueryMaterializer,
@@ -422,6 +422,17 @@ def _acl_violations(changed_objects, obj, acl):
             attribute_id not in attribute_ids and
             attribute_value != old_object[attribute_id]
         ):
+            is_related_via: bool = ServertypeAttribute.objects.filter(
+                servertype_id=obj['servertype'],
+                attribute_id=attribute_id,
+                related_via_attribute__isnull=False
+            ).exists()
+            if is_related_via:
+                # Attributes which are related via another servertype can be
+                # skipped because permission to change the value is checked
+                # at the target servertype where the actual change takes place.
+                continue
+
             violations.append(
                 'Change is not covered by ACL "{}", Attribute "{}" was '
                 'modified despite not beeing whitelisted.'.format(