fix(jobs): VM create and volume clone check the user and the name again in the worker

Every other job re-checks its permission and state in precheck before any
effect; these two checked only when queued. A queued create of a user who
is no longer a superuser, or whose VM name a host took meanwhile, ran; so
did a volume clone whose name a volume made outside the panel took.

instances/jobs.py

@@ -792,6 +792,16 @@ class CreateInstance(Kind):
                     label=f"{compute.name}/{params['name']}",
                     intent={"compute": compute.pk, "uuid": util.randomUUID(), "name": params["name"]})
 
+    def precheck(self, op, ctx):
+        if op.user is None or not op.user.is_active or not op.user.is_superuser:
+            raise PermissionDenied
+        # a VM found on a host since the job was queued may have taken the name
+        if Instance.objects.filter(name__exact=op.params["name"]).exists():
+            raise util.OperationError(_("There is an instance with same name. Remove it and try again!"))
+        c = Compute.objects.get(pk=op.params["compute"])
+        if op.params["name"] in wvmCreate(c.hostname, c.login, c.password, c.type).get_instances():
+            raise util.OperationError(_("A virtual machine with this name already exists"))
+
     def run(self, op, ctx):
         data = op.params
         c = Compute.objects.get(pk=data["compute"])

instances/tests/test_create_job.py

@@ -6,6 +6,7 @@ from unittest.mock import MagicMock, patch
 import libvirt
 
 from django.contrib.auth import get_user_model
+from django.core.exceptions import PermissionDenied
 from django.test import TransactionTestCase
 
 from appsettings.settings import get_settings
@@ -114,3 +115,19 @@ class CreateJobTestCase(TransactionTestCase):
         self.assertEqual(CreateInstance().reconcile(op).state, Operation.FAILED)
         pool.listVolumes.return_value = ["api-vm.qcow2"]
         self.assertEqual(CreateInstance().reconcile(op).state, Operation.UNCERTAIN)
+
+    def test_the_worker_checks_the_user_and_the_name_again(self):
+        response = self.client.post(f"/api/v1/computes/{self.compute.pk}/instances/create/x86_64/q35/", BODY,
+                                    content_type="application/json")
+        self.assertEqual(response.status_code, 202)
+        op = Operation.objects.get()
+        CreateInstance().precheck(op, None)
+        # a VM of that name found on a host meanwhile
+        self.conn.get_instances.return_value = ["api-vm"]
+        with self.assertRaisesMessage(libvirt.libvirtError, "A virtual machine with this name already exists"):
+            CreateInstance().precheck(op, None)
+        # the user is no longer a superuser
+        op.user.is_superuser = False
+        op.user.save()
+        with self.assertRaises(PermissionDenied):
+            CreateInstance().precheck(op, None)

storages/jobs.py

@@ -51,6 +51,13 @@ class CloneVolume(Kind):
         if not user.is_superuser:
             raise PermissionDenied
 
+    def precheck(self, op, ctx):
+        if op.user is None or not op.user.is_active or not op.user.is_superuser:
+            raise PermissionDenied
+        # a volume made outside the panel since the job was queued may have taken the name
+        if op.intent["target"] in _storage(op.params)[1].get_volumes():
+            raise util.OperationError(gettext_noop("Name of volume already in use"))
+
     def plan(self, user, params):
         compute, stg = _storage(params)
         volumes = stg.get_volumes()

storages/tests/test_volume_clone_job.py

@@ -4,12 +4,14 @@ from types import SimpleNamespace
 from unittest.mock import patch
 
 from django.contrib.auth import get_user_model
+from django.core.exceptions import PermissionDenied
 from django.test import TransactionTestCase
 from django.urls import reverse
 
 from computes.models import Compute
 from operations.jobs import Context
 from operations.models import Claim, Operation, acquire
+from libvirt import libvirtError
 from storages.jobs import CloneVolume
 
 
@@ -72,3 +74,16 @@ class VolumeCloneJobTestCase(TransactionTestCase):
         self.assertEqual(CloneVolume().reconcile(op).state, Operation.FAILED)
         op.intent["created"] = "copy.qcow2"
         self.assertEqual(CloneVolume().reconcile(op).state, Operation.SUCCEEDED)
+
+    def test_the_worker_checks_the_user_and_the_name_again(self):
+        user = get_user_model().objects.get(username="vol_admin")
+        op = Operation(kind="volume.clone", user=user, intent={"target": "copy.qcow2"},
+                       params={"compute": self.compute.pk, "pool": "pool-a", "image": "base.qcow2", "name": "copy",
+                               "format": "", "prealloc": False})
+        CloneVolume().precheck(op, None)
+        self.stg.get_volumes.return_value = ["base.qcow2", "copy.qcow2"]  # made outside the panel meanwhile
+        with self.assertRaisesMessage(libvirtError, "Name of volume already in use"):
+            CloneVolume().precheck(op, None)
+        user.is_superuser = False
+        with self.assertRaises(PermissionDenied):
+            CloneVolume().precheck(op, None)