j3d1/webvirtcloud
1
0
Fork 0
mirror of https://github.com/retspen/webvirtcloud synced 2026-11-08 11:16:47 +00:00
Code Issues Projects Releases Wiki Activity

fix(instances): change the edited NIC instead of adding a new one

Browse source 
The edit form posts net-*-<n> fields for NIC n and carries only the
new MAC. The backend read index 0 and looked the NIC up by the new MAC,
so editing a MAC detached nothing and attached a second NIC, and edits
of any NIC but the first were applied with empty values.

The form now also posts the current MAC (net-old-mac-<n>). The view
reads the submitted NIC's fields, validates the new MAC and reports
errors. The backend replaces the NIC with the old MAC, refuses an
unknown one, and on a failed attach restores the old NIC and raises
instead of reporting success.

The view crashed with a TypeError without a Referer header; it now
returns to the instance page, and a Referer from another site is
ignored.
This commit is contained in:
catborise 2026-10-03 17:56:47 +03:00
parent 9e92d208db
commit fbc6c4976f
6 changed files with 216 additions and 40 deletions
Download patch file Download diff file Expand all files Collapse all files

2
instances/templates/instances/settings_tab.html
View file

@ -412,7 +412,7 @@
<div class="row">
<label class="col-form-label">{% trans "MAC" %}</label>
<div class="input-group">
<input class="form-control" type="text" value="{{ network.mac }}" readonly/>
<input class="form-control" type="text" name="net-old-mac-{{ forloop.counter0 }}" value="{{ network.mac }}" readonly/>
<input class="form-control" type="text" name="net-mac-{{ forloop.counter0 }}" value="{{ network.mac }}"/>
</div>
</div>

57
instances/test_change_network.py Normal file
View file

@ -0,0 +1,57 @@
from unittest.mock import patch
from computes.models import Compute
from django.contrib.auth import get_user_model
from django.test import TestCase
from django.urls import reverse
from instances.models import Instance
class ChangeNetworkViewTestCase(TestCase):
def setUp(self):
admin = get_user_model().objects.create_superuser("net_admin", "n@example.com", "pw")
self.client.force_login(admin)
compute = Compute.objects.create(name="net-compute", hostname="127.0.0.1", login="", password="", type=4)
self.instance = Instance.objects.create(
compute=compute, name="net-vm", uuid="11111111-2222-3333-4444-555555555555"
)
self.url = reverse("instances:change_network", args=[self.instance.id])
def post(self, data):
with patch("instances.models.wvmInstance") as wvm:
wvm.return_value.get_status.return_value = 5
self.client.post(self.url, data, HTTP_REFERER="/instances/%d/" % self.instance.id)
return wvm.return_value.change_network
def test_changes_the_nic_of_the_submitted_form(self):
change = self.post({
"net-old-mac-1": "52:54:00:00:00:02",
"net-mac-1": "52:54:00:00:00:03",
"net-source-1": "net:default",
"net-nwfilter-1": "",
"net-model-1": "e1000",
})
change.assert_called_once_with("52:54:00:00:00:02", "52:54:00:00:00:03", "default", "net", "e1000", "")
def test_invalid_mac_is_rejected(self):
change = self.post({
"net-old-mac-0": "52:54:00:00:00:01",
"net-mac-0": "52:54:00:00:00:01'/><evil/>",
"net-source-0": "net:default",
})
change.assert_not_called()
def test_missing_old_mac_is_rejected(self):
change = self.post({"net-mac-0": "52:54:00:00:00:03", "net-source-0": "net:default"})
change.assert_not_called()
def test_missing_referer_redirects_to_the_instance(self):
with patch("instances.models.wvmInstance") as wvm:
wvm.return_value.get_status.return_value = 5
response = self.client.post(self.url, {
"net-old-mac-0": "52:54:00:00:00:01",
"net-source-0": "net:default",
"net-model-0": "virtio",
})
self.assertEqual(response.status_code, 302)
self.assertEqual(response.url, f"/instances/{self.instance.id}/#network")

42
instances/tests.py
View file

@ -917,6 +917,7 @@ class InstancesTestCase(TestCase):
response = self.client.post(
reverse("instances:change_network", args=[self.instance.id]),
{
"net-old-mac-0": "52:54:00:a2:3c:e7",
"net-mac-0": "52:54:00:a2:3c:e8",
"net-source-0": "net:default",
"net-nwfilter-0": "",
@ -927,8 +928,49 @@ class InstancesTestCase(TestCase):
self.assertEqual(response.status_code, 302)
del self.instance.networks
self.assertEqual(len(self.instance.networks), 1)
self.assertEqual(self.instance.networks[0]["mac"], "52:54:00:a2:3c:e8")
# The second NIC's form changes only the second NIC
self.client.post(
reverse("instances:add_network", args=[self.instance.id]),
{"add-net-mac": "52:54:00:a2:3c:e9", "add-net-network": "net:default"},
HTTP_REFERER=reverse("index"),
)
self.client.post(
reverse("instances:change_network", args=[self.instance.id]),
{
"net-old-mac-1": "52:54:00:a2:3c:e9",
"net-mac-1": "52:54:00:a2:3c:ea",
"net-source-1": "net:default",
"net-nwfilter-1": "",
"net-model-1": "e1000",
},
HTTP_REFERER=reverse("index"),
)
del self.instance.networks
self.assertEqual([n["mac"] for n in self.instance.networks], ["52:54:00:a2:3c:e8", "52:54:00:a2:3c:ea"])
self.assertEqual(self.instance.networks[1]["model"], "e1000")
# Restore the shared test VM
self.client.post(
reverse("instances:delete_network", args=[self.instance.id]),
{"delete_network": "52:54:00:a2:3c:ea"},
HTTP_REFERER=reverse("index"),
)
self.client.post(
reverse("instances:change_network", args=[self.instance.id]),
{
"net-old-mac-0": "52:54:00:a2:3c:e8",
"net-mac-0": "52:54:00:a2:3c:e7",
"net-source-0": "net:default",
"net-model-0": "virtio",
},
HTTP_REFERER=reverse("index"),
)
del self.instance.networks
self.assertEqual([n["mac"] for n in self.instance.networks], ["52:54:00:a2:3c:e7"])
def test_add_delete_network(self):
self.assertEqual(len(self.instance.networks), 1)
net_mac = self.instance.networks[0]["mac"]

68
instances/views.py
View file

@ -317,6 +317,16 @@ def sshkeys(request, pk):
return JsonResponse(instance_keys, safe=False)
def _same_origin_referer(request, default):
"""The Referer without its fragment if it is on this site, else default."""
referer = request.META.get("HTTP_REFERER")
if referer and url_has_allowed_host_and_scheme(
url=referer, allowed_hosts={request.get_host()}, require_https=request.is_secure()
):
return referer.split("#")[0]
return default
def get_safe_redirect(request, default=None):
referer = request.META.get("HTTP_REFERER")
if referer and url_has_allowed_host_and_scheme(
@ -1441,34 +1451,52 @@ def set_video_model(request, pk):
@serialize_instance_mutation
def change_network(request, pk):
instance = get_instance(request.user, pk)
back = _same_origin_referer(request, reverse("instances:instance", args=[pk])) + "#network"
msg = _("Change network")
network_data = {}
# Each NIC has its own form; its fields end with that NIC's index.
nums = [key[len("net-old-mac-"):] for key in request.POST if key.startswith("net-old-mac-")]
if len(nums) != 1:
messages.error(request, _("Select one network interface to change"))
return redirect(back)
num = nums[0]
old_mac = request.POST[f"net-old-mac-{num}"]
mac = request.POST.get(f"net-mac-{num}", "").strip() or old_mac
try:
util.validate_macaddr(mac)
except ValueError as err:
messages.error(request, err)
return redirect(back)
for post in request.POST:
if post.startswith("net-source-"):
(source, source_type) = utils.get_network_tuple(request.POST.get(post))
network_data[post] = source
network_data[post + "-type"] = source_type
(source, source_type) = utils.get_network_tuple(request.POST.get(f"net-source-{num}", ""))
if source_type == "iface":
iface = wvmInterface(
instance.compute.hostname,
instance.compute.login,
instance.compute.password,
instance.compute.type,
source,
)
source_type = iface.get_type()
if source_type == "iface":
iface = wvmInterface(
instance.compute.hostname,
instance.compute.login,
instance.compute.password,
instance.compute.type,
source,
)
network_data[post + "-type"] = iface.get_type()
elif post.startswith("net-"):
network_data[post] = request.POST.get(post, "")
try:
instance.proxy.change_network(
old_mac,
mac,
source,
source_type,
request.POST.get(f"net-model-{num}"),
request.POST.get(f"net-nwfilter-{num}", ""),
)
except libvirtError as err:
messages.error(request, err)
return redirect(back)
instance.proxy.change_network(network_data)
msg = _("Change network: %(mac)s") % {"mac": old_mac}
addlogmsg(request.user.username, instance.compute.name, instance.name, msg)
msg = _("Network Device Config is changed. Please shutdown instance to activate.")
if instance.proxy.get_status() != 5:
messages.success(request, msg)
return redirect(request.META.get("HTTP_REFERER") + "#network")
return redirect(back)
@require_POST

31
vrtManager/instance.py
View file

@ -1707,27 +1707,20 @@ class wvmInstance(wvmConnect):
return new_xml
return None
def change_network(self, network_data):
net_mac = network_data.get("net-mac-0")
net_source = network_data.get("net-source-0")
net_source_type = network_data.get("net-source-0-type")
net_filter = network_data.get("net-nwfilter-0")
net_model = network_data.get("net-model-0")
# Remove interface first, but keep network interface XML definition
# If there is an error happened while adding changed one, then add removed one to back.
status = self.delete_network(net_mac)
def change_network(self, old_mac, mac, source, source_type, model, nwfilter):
"""Replace the NIC with old_mac; put it back if the new one cannot be added."""
old_xml = self.delete_network(old_mac)
if old_xml is None:
raise libvirtError(f"No network interface with MAC {old_mac}")
try:
self.add_network(
net_mac, net_source, net_source_type, net_model, net_filter
)
self.add_network(mac, source, source_type, model, nwfilter)
except libvirtError:
if status is not None:
if self.get_status() == 1:
self.instance.attachDeviceFlags(status, VIR_DOMAIN_AFFECT_LIVE)
self.instance.attachDeviceFlags(status, VIR_DOMAIN_AFFECT_CONFIG)
if self.get_status() == 5:
self.instance.attachDeviceFlags(status, VIR_DOMAIN_AFFECT_CONFIG)
if self.get_status() == 1:
self.instance.attachDeviceFlags(old_xml, VIR_DOMAIN_AFFECT_LIVE)
self.instance.attachDeviceFlags(old_xml, VIR_DOMAIN_AFFECT_CONFIG)
if self.get_status() == 5:
self.instance.attachDeviceFlags(old_xml, VIR_DOMAIN_AFFECT_CONFIG)
raise
def change_network_oldway(self, network_data):
"""

56
vrtManager/test_change_network.py Normal file
View file

@ -0,0 +1,56 @@
import unittest
from unittest.mock import MagicMock
from django.conf import settings
if not settings.configured:
settings.configure(MAC_OUI="52:54:10")
from libvirt import VIR_DOMAIN_AFFECT_CONFIG, libvirtError
from vrtManager.instance import wvmInstance
DOMAIN_XML = """<domain><devices>
<interface type='network'><mac address='52:54:00:00:00:01'/><source network='default'/><model type='virtio'/></interface>
<interface type='network'><mac address='52:54:00:00:00:02'/><source network='default'/><model type='virtio'/></interface>
</devices></domain>"""
def stopped_instance():
inst = wvmInstance.__new__(wvmInstance)
inst.instance = MagicMock()
inst._XMLDesc = MagicMock(return_value=DOMAIN_XML)
inst.get_status = MagicMock(return_value=5)
return inst
class TestChangeNetwork(unittest.TestCase):
def test_replaces_the_nic_with_the_old_mac(self):
inst = stopped_instance()
inst.change_network("52:54:00:00:00:02", "52:54:00:00:00:03", "default", "net", "e1000", "")
detached = inst.instance.detachDeviceFlags.call_args[0][0]
self.assertIn("52:54:00:00:00:02", detached)
attached = inst.instance.attachDeviceFlags.call_args[0][0]
self.assertIn("52:54:00:00:00:03", attached)
self.assertIn("e1000", attached)
def test_unknown_old_mac_adds_nothing(self):
inst = stopped_instance()
with self.assertRaises(libvirtError):
inst.change_network("52:54:00:00:00:09", "52:54:00:00:00:03", "default", "net", "virtio", "")
inst.instance.detachDeviceFlags.assert_not_called()
inst.instance.attachDeviceFlags.assert_not_called()
def test_failed_add_restores_the_old_nic(self):
inst = stopped_instance()
inst.instance.attachDeviceFlags.side_effect = [libvirtError("bad source"), 0]
with self.assertRaises(libvirtError):
inst.change_network("52:54:00:00:00:01", "52:54:00:00:00:03", "missing", "net", "virtio", "")
restored, flags = inst.instance.attachDeviceFlags.call_args[0]
self.assertIn("52:54:00:00:00:01", restored)
self.assertEqual(flags, VIR_DOMAIN_AFFECT_CONFIG)
if __name__ == "__main__":
unittest.main()