1
0
Fork 0
mirror of https://github.com/retspen/webvirtcloud synced 2026-11-08 11:16:47 +00:00

fix(admin): a default VM that is not exactly one VM is refused on the form

NEW_USER_DEFAULT_INSTANCES names VMs, and a name may repeat across hosts.
A name with no VM, or with one on two hosts, made creating any user a 500.
The create form now looks the names up and says which one is not exactly
one VM; the user and its default VMs are saved together.
This commit is contained in:
catborise 2026-10-26 10:51:05 +03:00
parent 2e5ef34473
commit addff12652
4 changed files with 47 additions and 21 deletions

View file

@ -1,4 +1,5 @@
from django import forms
from django.conf import settings
from django.contrib.auth import password_validation
from django.contrib.auth.forms import AdminPasswordChangeForm
from django.contrib.auth.models import Group, User
@ -7,6 +8,7 @@ from django.utils.translation import gettext_lazy as _
from accounts.forms import UniqueEmailMixin
from accounts.models import UserAttributes
from instances.models import Instance
from .models import Permission
@ -85,6 +87,17 @@ class UserCreateForm(UserForm):
content_type__app_label="accounts", codename="change_password"
)
def clean(self):
# the VMs a new user owns, by name: a name may repeat across hosts
cleaned = super().clean()
self.default_instances = []
for name in settings.NEW_USER_DEFAULT_INSTANCES:
found = list(Instance.objects.filter(name=name)[:2])
if len(found) != 1:
raise ValidationError(_("NEW_USER_DEFAULT_INSTANCES: %(name)s is not exactly one VM") % {"name": name})
self.default_instances += found
return cleaned
def _post_clean(self):
# The validators in AUTH_PASSWORD_VALIDATORS, as on the password change
# forms; after the instance has the form's values (username, email).

View file

@ -1,7 +1,7 @@
"""Changes to users and groups write rows on the Logs page."""
from django.contrib.auth.models import Group, Permission, User
from django.test import TestCase, override_settings
from django.test import TestCase
from django.urls import reverse
from logs.models import Logs
@ -77,14 +77,6 @@ class UserRowTests(TestCase):
])
self.assertFalse(Logs.objects.filter(message__contains="New-carol-pass-1").exists())
@override_settings(NEW_USER_DEFAULT_INSTANCES=["no-such-vm"])
def test_a_failing_default_vm_leaves_no_user(self):
from instances.models import Instance
with self.assertRaises(Instance.DoesNotExist):
self.create()
self.assertFalse(User.objects.filter(username="carol").exists())
self.assertEqual(rows(), [("User carol: add: DoesNotExist", False)])
def test_the_row_has_actor_and_ip(self):
self.client.post(
reverse("admin:user_create"), {"username": "carol", "password": "Carol-pass-2026", **ATTRIBUTES},

View file

@ -6,6 +6,10 @@ from django.db import connection
from django.test import TestCase, override_settings
from django.urls import reverse
from accounts.models import UserInstance
from computes.models import Compute
from instances.models import Instance
RAW = "Raw-Passw0rd-in-the-form"
LIMITS = {"max_instances": 1, "max_cpus": 1, "max_memory": 1024, "max_disk_size": 4}
@ -63,3 +67,30 @@ class UserCreateTests(TestCase):
)
self.assertEqual(response.status_code, 200)
self.assertContains(response, "too similar to the username")
class DefaultInstancesTests(TestCase):
"""NEW_USER_DEFAULT_INSTANCES names VMs, and a name may repeat across hosts."""
def setUp(self):
self.client.force_login(User.objects.create_superuser("uc-admin", "uc@example.com", "pw"))
one, two = (Compute.objects.create(name=name, hostname="localhost", login="", password="", details="", type=4)
for name in ("uc-one", "uc-two"))
self.web = Instance.objects.create(compute=one, name="web", uuid="00000000-0000-0000-0000-000000000001")
for compute, uuid in ((one, "00000000-0000-0000-0000-000000000002"), (two, "00000000-0000-0000-0000-000000000003")):
Instance.objects.create(compute=compute, name="db", uuid=uuid)
def create(self):
return self.client.post(reverse("admin:user_create"), {"username": "newbie", "password": RAW, **LIMITS})
@override_settings(NEW_USER_DEFAULT_INSTANCES=["web"])
def test_the_new_user_owns_the_named_vm(self):
self.assertRedirects(self.create(), reverse("admin:user_list"))
self.assertEqual([o.instance for o in UserInstance.objects.filter(user__username="newbie")], [self.web])
def test_a_name_that_is_not_one_vm_is_refused_on_the_form(self):
for name in ("no-such-vm", "db"):
with self.subTest(name), override_settings(NEW_USER_DEFAULT_INSTANCES=["web", name]):
response = self.create()
self.assertContains(response, f"NEW_USER_DEFAULT_INSTANCES: {name} is not exactly one VM")
self.assertFalse(User.objects.filter(username="newbie").exists())

View file

@ -127,7 +127,8 @@ def user_create(request):
attributes = attributes_form.save(commit=False)
attributes.user = user
attributes.save()
add_default_instances(user)
UserInstance.objects.bulk_create(UserInstance(user=user, instance=instance)
for instance in user_form.default_instances)
return redirect("admin:user_list")
return render(
@ -346,14 +347,3 @@ def tombstone_delete(request, pk):
"consequence": _("Its owners no longer get the VM back if its UUID shows up again.")},
)
def add_default_instances(user):
"""
Adds instances listed in NEW_USER_DEFAULT_INSTANCES to user
"""
existing_instances = UserInstance.objects.filter(user=user)
if not existing_instances:
for instance_name in settings.NEW_USER_DEFAULT_INSTANCES:
instance = Instance.objects.get(name=instance_name)
user_instance = UserInstance(user=user, instance=instance)
user_instance.save()