diff --git a/instances/jobs.py b/instances/jobs.py index 8dd43110..b654d267 100644 --- a/instances/jobs.py +++ b/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"]) diff --git a/instances/tests/test_create_job.py b/instances/tests/test_create_job.py index 99b6d00c..46d7ebea 100644 --- a/instances/tests/test_create_job.py +++ b/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) diff --git a/storages/jobs.py b/storages/jobs.py index c4ec1d3d..6430ac9e 100644 --- a/storages/jobs.py +++ b/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() diff --git a/storages/tests/test_volume_clone_job.py b/storages/tests/test_volume_clone_job.py index f495e5eb..247bcc17 100644 --- a/storages/tests/test_volume_clone_job.py +++ b/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)