feat(logs): changes to users, groups, owners, flavors and settings are logged

The Logs page now records the panel's own records too: users (add,
change with the changed fields, delete, block, unblock, an admin's
password set, a user's own password change including the first-login
one, own profile edits), groups with their members and permissions,
VM owners from the accounts pages and from the VM page, OTP QR code
mails, flavors from web and API, app settings and the theme,
removed-VM records and SSH keys. No row carries a password or a key.

- A user is created in one transaction: when a VM named in
  NEW_USER_DEFAULT_INSTANCES is missing, no half-made user stays.
- The OTP mail checks the email address before it creates a device;
  without one it writes a failed row and makes no device.
- The VM page's owner rows read "VM web-1 owner bob: add" and
  "...: remove" instead of "Add owner: bob" and "Delete owner: 17"; a
  second owner refused by the single-owner setting is a failed row.
  Owner rows keep the VM's host and name, so the VM's log tab shows
  them.
- A setting or theme posted with its current value is no change and
  writes no row; the old free-text setting rows are gone.

Host-side objects (networks, storage, interfaces, secrets, filters)
follow in the next layer.

README.md

@@ -583,7 +583,7 @@ A version from before v3.0.0 has no worker: `systemctl disable --now webvirt-wor
 - A disk can be grown while its VM runs or is paused, not only when it is shut off. A disk on an LVM pool cannot be grown (libvirt cannot resize its volumes): the resize says so instead of failing, and the VM's other disks still grow.
 - Enabling vCPU hot plug keeps the VM's current vCPU count (it enabled all of them up to the maximum).
 - A host's CPU graph shows the average since its previous point (normally 5 s) instead of a 1 s sample taken while a worker waits.
-- The Logs page records who added, changed or deleted a compute, and every login, failed login, lockout and logout, with the address it came from and whether it worked (Result: Done, Failed, Unknown for older rows); a compute's VMs leaving the panel get a row each. Other admin and host changes follow.
+- The Logs page records who added, changed or deleted a compute, user, group (with its members and permissions), VM owner, flavor, setting, removed-VM record or SSH key, who mailed an OTP QR code, and every login, failed login, lockout and logout, with the address it came from and whether it worked (Result: Done, Failed, Unknown for older rows); a compute's VMs leaving the panel get a row each. Changes of networks, storage and other host objects follow.
 - The CPU usage in the new-VM dialog is the current load (over 1 s), read when the dialog opens; it was the average since the host booted.
 - A VM's stats are polled every 5 s like the host graph, the first point as soon as the tab opens; each point is the average since the previous one, disk rates in MB/s (they were MiB/s labelled Mb/s); a counter the host does not report shows a gap instead of 0.
 - An ISO uploads to an SSH compute about 3.5 times faster (SFTP writes no longer wait for each 32 KiB); a failed write still fails the upload.

accounts/tests/test_audit_rows.py

@@ -0,0 +1,103 @@
+"""Changes to a user's own account, VM ownership, OTP mails and SSH keys write rows on the Logs page."""
+
+from django.contrib.auth import get_user_model
+from django.test import TestCase
+from django.urls import reverse
+
+from accounts.models import UserAttributes
+from logs.models import Logs
+
+
+def rows():
+    return list(Logs.objects.order_by("id").values_list("message", "success"))
+
+
+class OwnAccountRowTests(TestCase):
+    def setUp(self):
+        self.bob = get_user_model().objects.create_user("bob", password="Bob-pass-2026")
+        self.client.force_login(self.bob)
+        Logs.objects.all().delete()  # the login's row
+
+    def test_own_profile_and_password(self):
+        self.client.post(reverse("accounts:profile"), {"username": "bob", "email": "bob@example.com"})
+        self.client.post(reverse("accounts:change_password"), {
+            "old_password": "Bob-pass-2026", "new_password1": "Bob-new-pass-1", "new_password2": "Bob-new-pass-1"})
+        self.assertEqual(rows(), [("User bob: change email", True), ("User bob: change password", True)])
+        self.assertFalse(Logs.objects.filter(message__contains="pass-").exists())
+
+    def test_the_first_login_password_change(self):
+        self.bob.user_permissions.clear()
+        UserAttributes.objects.update_or_create(user=self.bob, defaults={"must_change_password": True})
+        self.client.post(reverse("accounts:change_password"), {
+            "old_password": "Bob-pass-2026", "new_password1": "Bob-new-pass-1", "new_password2": "Bob-new-pass-1"})
+        self.assertEqual(rows(), [("User bob: change password", True)])
+
+
+SSH_KEY = ("ssh-rsa AAAAB3NzaC1yc2EAAAADAQABAAAAgQC6OOdbfv27QVnSC6sKxGaHb6YFc+3gxCkyVR3cTSXE/n5BEGf8aOgBpepULWa1RZfx"
+           "YHY14PlKULDygdXSdrrR2kNSwoKz/Oo4d+3EE92L7ocl1+djZbptzgWgtw1OseLwbFik+iKlIdqPsH+IUQvX7yV545ZQtAP8Qj1R+uCqkw=="
+           " test@test")
+
+
+class AdminAccountRowTests(TestCase):
+    def setUp(self):
+        from computes.models import Compute
+        from instances.models import Instance
+
+        self.admin = get_user_model().objects.create_superuser("audit-admin", "audit@example.com", "pw")
+        self.bob = get_user_model().objects.create_user("bob", password="Bob-pass-2026")
+        compute = Compute.objects.create(name="c1", hostname="127.0.0.1", login="", password="", type=4)
+        self.vm = Instance.objects.create(compute=compute, name="vm", uuid="66666666-6666-6666-6666-666666666666")
+        self.client.force_login(self.admin)
+        Logs.objects.all().delete()  # the login's row
+
+    def test_ownership_from_the_accounts_pages(self):
+        from accounts.models import UserInstance
+
+        self.client.post(reverse("accounts:user_instance_create", args=[self.bob.pk]),
+                         {"user": self.bob.pk, "instance": self.vm.pk})
+        ui = UserInstance.objects.get(user=self.bob, instance=self.vm)
+        url = reverse("accounts:user_instance_update", args=[ui.pk])
+        self.client.post(url, {"is_vnc": "on"})
+        self.client.post(url, {"is_vnc": "on"})  # no change
+        self.client.post(reverse("accounts:user_instance_delete", args=[ui.pk]))
+        self.assertEqual(rows(), [
+            ("VM vm owner bob: add", True),
+            ("VM vm owner bob: change is_vnc", True),
+            ("VM vm owner bob: remove", True),
+        ])
+        self.assertEqual(set(Logs.objects.values_list("user", "host", "instance")), {("audit-admin", "c1", "vm")})
+
+    def test_otp_mail_without_an_email(self):
+        from smtplib import SMTPException
+        from unittest.mock import patch
+
+        from django_otp.plugins.otp_totp.models import TOTPDevice
+
+        url = reverse("accounts:admin_email_otp", args=[self.bob.pk])
+        with patch("accounts.views.send_email_with_otp") as send:
+            self.client.post(url)
+            send.assert_not_called()
+        self.assertFalse(TOTPDevice.objects.filter(user=self.bob).exists())
+
+        self.bob.email = "bob@example.com"
+        self.bob.save()
+        with patch("accounts.views.send_email_with_otp") as send:
+            self.client.post(url)
+            send.assert_called_once()
+        with patch("accounts.views.send_email_with_otp", side_effect=SMTPException("refused")), \
+                self.assertRaises(SMTPException):
+            self.client.post(url)
+        self.assertEqual(rows(), [
+            ("User bob: mail OTP QR code: no email address", False),
+            ("User bob: mail OTP QR code", True),
+            ("User bob: mail OTP QR code: SMTPException", False),
+        ])
+
+    def test_ssh_keys(self):
+        from accounts.models import UserSSHKey
+
+        self.client.post(reverse("accounts:ssh_key_create"), {"keyname": "laptop", "keypublic": SSH_KEY})
+        key = UserSSHKey.objects.get(keyname="laptop")
+        self.client.post(reverse("accounts:ssh_key_delete", args=[key.pk]))
+        self.assertEqual(rows(), [("SSH key laptop: add", True), ("SSH key laptop: delete", True)])
+        self.assertFalse(Logs.objects.filter(message__contains="AAAA").exists())

accounts/views.py

@@ -19,6 +19,7 @@ from django.views.decorators.http import require_POST
 
 from accounts.apps import admin_password_path
 from instances.models import Instance
+from logs.audit import audit, changed_fields, failed, snapshot
 
 from accounts.forms import EmailOTPForm, ProfileForm, UserSSHKeyForm
 from accounts.models import User, UserAttributes, UserInstance, UserSSHKey
@@ -29,11 +30,16 @@ from .utils import get_existing_totp_device, get_user_totp_device, send_email_wi
 
 def profile(request):
     publickeys = UserSSHKey.objects.filter(user_id=request.user.id)
+    before = snapshot(request.user)
     profile_form = ProfileForm(request.POST or None, instance=request.user)
     ssh_key_form = UserSSHKeyForm()
 
     if profile_form.is_valid():
-        profile_form.save()
+        with audit(request, None, f"User {request.user.username}", "change") as entry:
+            profile_form.save()
+            fields = changed_fields(before, snapshot(request.user))
+            entry.action = "change " + ", ".join(fields)
+            entry.skip = not fields
         messages.success(request, _("Profile updated"))
         return redirect("accounts:profile")
 
@@ -51,7 +57,8 @@ def profile(request):
 def ssh_key_create(request):
     key_form = UserSSHKeyForm(request.POST or None, user=request.user)
     if key_form.is_valid():
-        key_form.save()
+        with audit(request, None, f"SSH key {key_form.cleaned_data['keyname']}", "add"):
+            key_form.save()
         messages.success(request, _("SSH key added"))
         return redirect("accounts:profile")
 
@@ -68,7 +75,8 @@ def ssh_key_create(request):
 def ssh_key_delete(request, pk):
     ssh_key = get_object_or_404(UserSSHKey, pk=pk, user=request.user)
     if request.method == "POST":
-        ssh_key.delete()
+        with audit(request, None, f"SSH key {ssh_key.keyname}", "delete"):
+            ssh_key.delete()
         messages.success(request, _("SSH key deleted"))
         return redirect("accounts:profile")
 
@@ -113,7 +121,7 @@ def change_password(request):
     form = PasswordChangeForm(request.user, request.POST or None)
 
     if form.is_valid():
-        with transaction.atomic():
+        with audit(request, None, f"User {request.user.username}", "change password"), transaction.atomic():
             user = form.save()
             UserAttributes.objects.filter(user=user).update(must_change_password=False)
         update_session_auth_hash(request, user)
@@ -137,7 +145,10 @@ def user_instance_create(request, user_id):
 
     form = forms.UserInstanceForm(request.POST or None, initial={"user": user})
     if form.is_valid():
-        form.save()
+        instance = form.cleaned_data["instance"]
+        subject = f"VM {instance.name} owner {form.cleaned_data['user'].username}"
+        with audit(request, instance.compute, subject, "add", instance=instance.name):
+            form.save()
         return redirect(reverse("accounts:account", args=[user.id]))
 
     return render(
@@ -153,9 +164,15 @@ def user_instance_create(request, user_id):
 @superuser_only
 def user_instance_update(request, pk):
     user_instance = get_object_or_404(UserInstance, pk=pk)
+    instance, before = user_instance.instance, snapshot(user_instance)
     form = forms.UserInstanceForm(request.POST or None, instance=user_instance)
     if form.is_valid():
-        form.save()
+        subject = f"VM {instance.name} owner {user_instance.user.username}"
+        with audit(request, instance.compute, subject, "change", instance=instance.name) as entry:
+            form.save()
+            fields = changed_fields(before, snapshot(user_instance))
+            entry.action = "change " + ", ".join(fields)
+            entry.skip = not fields
         return redirect(reverse("accounts:account", args=[user_instance.user.id]))
 
     return render(
@@ -172,8 +189,10 @@ def user_instance_update(request, pk):
 def user_instance_delete(request, pk):
     user_instance = get_object_or_404(UserInstance, pk=pk)
     if request.method == "POST":
-        user = user_instance.user
-        user_instance.delete()
+        user, instance = user_instance.user, user_instance.instance
+        with audit(request, instance.compute, f"VM {instance.name} owner {user.username}", "remove",
+                   instance=instance.name):
+            user_instance.delete()
         next_url = request.GET.get("next", None)
         if next_url and url_has_allowed_host_and_scheme(
             url=next_url,
@@ -238,12 +257,14 @@ def email_otp(request):
 @superuser_only
 def admin_email_otp(request, user_id):
     user = get_object_or_404(get_user_model(), pk=user_id)
-    device = get_user_totp_device(user)
+    subject = f"User {user.username}"
     if user.email != "":
-        send_email_with_otp(user, device)
+        with audit(request, None, subject, "mail OTP QR code"):
+            send_email_with_otp(user, get_user_totp_device(user))
         messages.success(
             request, _("OTP QR code was emailed to user %(user)s") % {"user": user}
         )
     else:
+        failed(request, None, subject, "mail OTP QR code", "no email address")
         messages.error(request, _("User email not set, failed to send QR code"))
     return redirect("accounts:account", user.id)

admin/tests/test_audit_rows.py

@@ -0,0 +1,121 @@
+"""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.urls import reverse
+
+from logs.models import Logs
+
+ATTRIBUTES = {"max_instances": 2, "max_cpus": 2, "max_memory": 2048, "max_disk_size": 20}
+
+
+def rows():
+    return list(Logs.objects.order_by("id").values_list("message", "success"))
+
+
+class UserRowTests(TestCase):
+    def setUp(self):
+        self.admin = User.objects.create_superuser("audit-admin", "audit@example.com", "pw")
+        self.client.force_login(self.admin)
+        Logs.objects.all().delete()  # the login's row
+
+    def create(self, username="carol", **extra):
+        return self.client.post(reverse("admin:user_create"),
+                                {"username": username, "password": "Carol-pass-2026", "is_active": "on", **ATTRIBUTES, **extra})
+
+    def edit(self, user, **changes):
+        data = {"username": user.username, "email": user.email, "is_active": "on", **ATTRIBUTES,
+                "user_permissions": [p.pk for p in user.user_permissions.all()],
+                "groups": [g.pk for g in user.groups.all()], **changes}
+        return self.client.post(reverse("admin:user_update", args=[user.pk]), data)
+
+    def test_user_add_change_delete(self):
+        self.create()
+        carol = User.objects.get(username="carol")
+        self.edit(carol, email="carol@example.com")
+        self.edit(User.objects.get(pk=carol.pk))  # nothing changed
+        self.client.post(reverse("admin:user_delete", args=[carol.pk]))
+        self.assertEqual(rows(), [
+            ("User carol: add", True),
+            ("User carol: change email", True),
+            ("User carol: delete", True),
+        ])
+
+    def test_attributes_and_groups_are_named(self):
+        self.create()
+        carol = User.objects.get(username="carol")
+        permission = Permission.objects.get(content_type__app_label="accounts", codename="change_password")
+        carol.user_permissions.remove(permission)
+        Logs.objects.all().delete()
+
+        self.edit(carol, max_instances=5)
+        group = Group.objects.create(name="ops")
+        self.edit(carol, max_instances=5, groups=[group.pk], user_permissions=[permission.pk])
+        self.edit(carol, max_instances=5, groups=[group.pk], user_permissions=[permission.pk], username="carla")
+        self.edit(User.objects.get(pk=carol.pk), username="")  # invalid: no row
+        self.assertEqual([m for m, _ in rows()], [
+            "User carol: change max_instances",
+            "User carol: change groups, user_permissions",
+            "User carol: change username",
+        ])
+        self.assertTrue(User.objects.get(pk=carol.pk).user_permissions.filter(pk=permission.pk).exists())
+
+    def test_block_unblock_and_password(self):
+        self.create()
+        carol = User.objects.get(username="carol")
+        Logs.objects.all().delete()
+        self.client.post(reverse("admin:user_block", args=[carol.pk]))
+        self.client.post(reverse("admin:user_unblock", args=[carol.pk]))
+        self.client.post(reverse("admin:user_update_password", args=[carol.pk]),
+                         {"password1": "New-carol-pass-1", "password2": "New-carol-pass-1"})
+        self.assertEqual(rows(), [
+            ("User carol: block", True),
+            ("User carol: unblock", True),
+            ("User carol: set password", True),
+        ])
+        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},
+            REMOTE_ADDR="192.0.2.9")
+        row = Logs.objects.get()
+        self.assertEqual((row.user, row.host, row.instance, row.ip), ("audit-admin", "-", "", "192.0.2.9"))
+
+
+class GroupAndRemovedVMRowTests(TestCase):
+    def setUp(self):
+        self.client.force_login(User.objects.create_superuser("audit-admin", "audit@example.com", "pw"))
+        Logs.objects.all().delete()  # the login's row
+
+    def test_group_add_change_delete(self):
+        member = User.objects.create_user("dave")
+        permission = Permission.objects.get(content_type__app_label="accounts", codename="change_password")
+        self.client.post(reverse("admin:group_create"), {"name": "ops"})
+        group = Group.objects.get(name="ops")
+        url = reverse("admin:group_update", args=[group.pk])
+        self.client.post(url, {"name": "ops", "users": [member.pk]})
+        self.client.post(url, {"name": "ops", "users": [member.pk], "permissions": [permission.pk]})
+        self.client.post(url, {"name": "ops", "users": [member.pk], "permissions": [permission.pk]})  # no change
+        self.client.post(reverse("admin:group_delete", args=[group.pk]))
+        self.assertEqual(rows(), [
+            ("Group ops: add", True),
+            ("Group ops: change users", True),
+            ("Group ops: change permissions", True),
+            ("Group ops: delete", True),
+        ])
+
+    def test_a_removed_vm_record_delete(self):
+        from instances.models import InstanceTombstone
+        uuid = "55555555-5555-5555-5555-555555555555"
+        tombstone = InstanceTombstone.objects.create(uuid=uuid, name="vm", owners=[])
+        self.client.post(reverse("admin:tombstone_delete", args=[tombstone.pk]))
+        self.assertEqual(rows(), [(f"Removed VM vm ({uuid}): delete", True)])

admin/views.py

@@ -8,17 +8,24 @@ from django.contrib.auth import update_session_auth_hash
 from django.contrib.auth.forms import AdminPasswordChangeForm
 from django.contrib.auth.models import Group, User
 from django.core.paginator import Paginator
+from django.db import transaction
 from django.shortcuts import get_object_or_404, redirect, render
 from django.utils.http import url_has_allowed_host_and_scheme
 from django.utils.translation import gettext_lazy as _
 from django.views.decorators.http import require_POST
 from instances.models import InstanceTombstone
+from logs.audit import audit, changed_fields, snapshot
 from logs.models import Logs
 
 from . import forms
 from .decorators import superuser_only
 
 
+def group_snapshot(group):
+    """snapshot() of a group with its members, which are a reverse relation snapshot() does not see."""
+    return snapshot(group) | {"users": frozenset(group.user_set.values_list("pk", flat=True))}
+
+
 @superuser_only
 def group_list(request):
     groups = Group.objects.all()
@@ -35,7 +42,8 @@ def group_list(request):
 def group_create(request):
     form = forms.GroupForm(request.POST or None)
     if form.is_valid():
-        form.save()
+        with audit(request, None, f"Group {form.cleaned_data['name']}", "add"):
+            form.save()
         return redirect("admin:group_list")
 
     return render(
@@ -51,9 +59,14 @@ def group_create(request):
 @superuser_only
 def group_update(request, pk):
     group = get_object_or_404(Group, pk=pk)
+    subject, before = f"Group {group.name}", group_snapshot(group)
     form = forms.GroupForm(request.POST or None, instance=group)
     if form.is_valid():
-        form.save()
+        with audit(request, None, subject, "change") as entry:
+            form.save()
+            fields = changed_fields(before, group_snapshot(group))
+            entry.action = "change " + ", ".join(fields)
+            entry.skip = not fields
         return redirect("admin:group_list")
 
     return render(
@@ -70,7 +83,8 @@ def group_update(request, pk):
 def group_delete(request, pk):
     group = get_object_or_404(Group, pk=pk)
     if request.method == "POST":
-        group.delete()
+        with audit(request, None, f"Group {group.name}", "delete"):
+            group.delete()
         return redirect("admin:group_list")
 
     return render(
@@ -98,11 +112,12 @@ def user_create(request):
     user_form = forms.UserCreateForm(request.POST or None)
     attributes_form = forms.UserAttributesForm(request.POST or None)
     if user_form.is_valid() and attributes_form.is_valid():
-        user = user_form.save()
-        attributes = attributes_form.save(commit=False)
-        attributes.user = user
-        attributes.save()
-        add_default_instances(user)
+        with audit(request, None, f"User {user_form.cleaned_data['username']}", "add"), transaction.atomic():
+            user = user_form.save()
+            attributes = attributes_form.save(commit=False)
+            attributes.user = user
+            attributes.save()
+            add_default_instances(user)
         return redirect("admin:user_list")
 
     return render(
@@ -120,13 +135,20 @@ def user_create(request):
 def user_update(request, pk):
     user = get_object_or_404(User, pk=pk)
     attributes, attributes_created = UserAttributes.objects.get_or_create(user=user)
+    subject = f"User {user.username}"
+    before, before_attributes = snapshot(user), snapshot(attributes)
     user_form = forms.UserForm(request.POST or None, instance=user)
     attributes_form = forms.UserAttributesForm(
         request.POST or None, instance=attributes
     )
     if user_form.is_valid() and attributes_form.is_valid():
-        user_form.save()
-        attributes_form.save()
+        with audit(request, None, subject, "change") as entry:
+            user_form.save()
+            attributes_form.save()
+            fields = set(changed_fields(before, snapshot(user))) | set(changed_fields(before_attributes, snapshot(attributes)))
+            fields = sorted(fields - {"password", "last_login"})
+            entry.action = "change " + ", ".join(fields)
+            entry.skip = not fields
         next_url = request.GET.get("next")
         if next_url and url_has_allowed_host_and_scheme(
             url=next_url, allowed_hosts={request.get_host()}, require_https=request.is_secure()
@@ -151,7 +173,8 @@ def user_update_password(request, pk):
     if request.method == "POST":
         form = AdminPasswordChangeForm(user, request.POST)
         if form.is_valid():
-            user = form.save()
+            with audit(request, None, f"User {user.username}", "set password"):
+                user = form.save()
             update_session_auth_hash(request, user)  # Important!
             messages.success(
                 request, _("Password changed for %(user)s") % {"user": user.username}
@@ -176,7 +199,8 @@ def user_update_password(request, pk):
 def user_delete(request, pk):
     user = get_object_or_404(User, pk=pk)
     if request.method == "POST":
-        user.delete()
+        with audit(request, None, f"User {user.username}", "delete"):
+            user.delete()
         return redirect("admin:user_list")
 
     return render(
@@ -191,8 +215,9 @@ def user_delete(request, pk):
 @superuser_only
 def user_block(request, pk):
     user: User = get_object_or_404(User, pk=pk)
-    user.is_active = False
-    user.save()
+    with audit(request, None, f"User {user.username}", "block"):
+        user.is_active = False
+        user.save()
     return redirect("admin:user_list")
 
 
@@ -200,8 +225,9 @@ def user_block(request, pk):
 @superuser_only
 def user_unblock(request, pk):
     user: User = get_object_or_404(User, pk=pk)
-    user.is_active = True
-    user.save()
+    with audit(request, None, f"User {user.username}", "unblock"):
+        user.is_active = True
+        user.save()
     return redirect("admin:user_list")
 
 
@@ -230,7 +256,8 @@ def tombstone_list(request):
 def tombstone_delete(request, pk):
     tombstone = get_object_or_404(InstanceTombstone, pk=pk)
     if request.method == "POST":
-        tombstone.delete()
+        with audit(request, None, f"Removed VM {tombstone.name} ({tombstone.uuid})", "delete"):
+            tombstone.delete()
         return redirect("admin:tombstone_list")
 
     return render(

appsettings/tests/test_audit_rows.py

@@ -0,0 +1,44 @@
+"""App setting and theme changes write rows on the Logs page."""
+
+from unittest.mock import patch
+
+from django.contrib.auth import get_user_model
+from django.db import DatabaseError
+from django.test import TestCase
+from django.urls import reverse
+
+from appsettings.models import AppSettings
+from logs.models import Logs
+
+
+def rows():
+    return list(Logs.objects.order_by("id").values_list("message", "success", "host"))
+
+
+class SettingRowTests(TestCase):
+    def setUp(self):
+        self.client.force_login(get_user_model().objects.create_superuser("audit-admin", "audit@example.com", "pw"))
+        Logs.objects.all().delete()  # the login's row
+        self.url = reverse("appsettings")
+
+    def test_setting_and_theme_rows(self):
+        name = AppSettings.objects.get(key="LOGS_PER_PAGE").name
+        self.client.post(self.url, {"LOGS_PER_PAGE": "50"})
+        self.client.post(self.url, {"LOGS_PER_PAGE": "50"})  # no change
+        before = AppSettings.objects.get(key="WORKER_CONCURRENCY").value
+        self.client.post(self.url, {"WORKER_CONCURRENCY": "0"})  # out of bounds
+        self.assertEqual(AppSettings.objects.get(key="WORKER_CONCURRENCY").value, before)
+        theme = "cerulean" if AppSettings.objects.get(key="BOOTSTRAP_THEME").value != "cerulean" else "brite"
+        self.client.post(self.url, {"BOOTSTRAP_THEME": theme})
+        self.client.post(self.url, {"BOOTSTRAP_THEME": theme})  # no change
+        self.assertEqual(rows(), [
+            (f"Setting {name}: change to 50", True, "-"),
+            (f"Setting Theme: change to {theme}", True, "-"),
+        ])
+
+    def test_a_failing_save_is_a_failed_row(self):
+        name = AppSettings.objects.get(key="LOGS_PER_PAGE").name
+        with patch.object(AppSettings, "save", side_effect=DatabaseError("disk full")):
+            response = self.client.post(self.url, {"LOGS_PER_PAGE": "50"}, follow=True)
+        self.assertIn("error", [m.level_tag for m in response.context["messages"]])
+        self.assertEqual(rows(), [(f"Setting {name}: change to 50: DatabaseError", False, "-")])

appsettings/views.py

@@ -6,7 +6,7 @@ from django.contrib import messages
 from django.http import HttpResponseRedirect
 from django.shortcuts import render
 from django.utils.translation import gettext_noop as _
-from logs.views import addlogmsg
+from logs.audit import audit
 from operations.utils import BOUNDS
 
 from appsettings.models import AppSettings
@@ -36,11 +36,12 @@ def appsettings(request):
             if theme not in themes_list:
                 messages.error(request, _("Unknown theme: %(theme)s") % {"theme": theme})
                 return HttpResponseRedirect(request.get_full_path())
-            bootstrap_theme.value = theme
-            bootstrap_theme.save()
+            if theme != bootstrap_theme.value:
+                with audit(request, None, "Setting Theme", f"change to {theme}"):
+                    bootstrap_theme.value = theme
+                    bootstrap_theme.save()
             msg = _("Theme is changed. Now: %(theme)s") % {"theme": theme}
             messages.success(request, msg)
-            addlogmsg(request.user.username, "-", "", msg)
             return HttpResponseRedirect(request.get_full_path())
 
         for setting in appsettings:
@@ -52,9 +53,12 @@ def appsettings(request):
                         messages.error(request, _("%(setting)s must be a whole number between %(low)s and %(high)s")
                                        % {"setting": setting.name, "low": low, "high": high})
                         return HttpResponseRedirect(request.get_full_path())
+                value = request.POST.get(setting.key, "")
                 try:
-                    setting.value = request.POST.get(setting.key, "")
-                    setting.save()
+                    if value != setting.value:
+                        with audit(request, None, f"Setting {setting.name}", f"change to {value}"):
+                            setting.value = value
+                            setting.save()
 
                     msg = _("%(setting)s is changed. Now: %(value)s") % {
                         "setting": setting.name,
@@ -64,8 +68,6 @@ def appsettings(request):
                 except Exception as err:
                     msg = err
                     messages.error(request, msg)
-
-                addlogmsg(request.user.username, "-", "", msg)
                 return HttpResponseRedirect(request.get_full_path())
 
     return render(request, "appsettings.html", locals())

instances/api/viewsets.py

@@ -16,6 +16,7 @@ from instances.views import (
     suspend,
 )
 from drf_spectacular.utils import extend_schema
+from logs.audit import audit, changed_fields, snapshot
 from operations import jobs
 from operations.api.serializers import OperationSerializer
 from rest_framework import permissions, status, viewsets
@@ -210,6 +211,23 @@ class FlavorViewSet(viewsets.ModelViewSet):
             return [permissions.IsAuthenticated()]
         return [IsSuperUser()]
 
+    def perform_create(self, serializer):
+        with audit(self.request, None, f"Flavor {serializer.validated_data['label']}", "add"):
+            serializer.save()
+
+    def perform_update(self, serializer):
+        flavor = serializer.instance
+        subject, before = f"Flavor {flavor.label}", snapshot(flavor)
+        with audit(self.request, None, subject, "change") as entry:
+            serializer.save()
+            fields = changed_fields(before, snapshot(flavor))
+            entry.action = "change " + ", ".join(fields)
+            entry.skip = not fields
+
+    def perform_destroy(self, instance):
+        with audit(self.request, None, f"Flavor {instance.label}", "delete"):
+            instance.delete()
+
 
 class CreateInstanceViewSet(viewsets.ViewSet):
     """

instances/tests/test_audit_rows.py

@@ -0,0 +1,76 @@
+"""VM page owner changes and flavor changes write rows on the Logs page."""
+
+from accounts.models import UserInstance
+from appsettings.models import AppSettings
+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
+from logs.models import Logs
+
+
+def rows():
+    return list(Logs.objects.order_by("id").values_list("message", "success"))
+
+
+class VMOwnerRowTests(TestCase):
+    def setUp(self):
+        self.client.force_login(get_user_model().objects.create_superuser("audit-admin", "audit@example.com", "pw"))
+        self.bob = get_user_model().objects.create_user("bob")
+        self.carol = get_user_model().objects.create_user("carol")
+        compute = Compute.objects.create(name="c1", hostname="127.0.0.1", login="", password="", type=4)
+        self.vm = Instance.objects.create(compute=compute, name="vm", uuid="77777777-7777-7777-7777-777777777777")
+        Logs.objects.all().delete()  # the login's row
+
+    def test_vm_page_owners(self):
+        add = reverse("instances:add_owner", args=[self.vm.pk])
+        self.client.post(add, {"user_id": self.bob.pk})
+        self.client.post(add, {"user_id": self.bob.pk})  # already an owner: nothing changed
+        AppSettings.objects.filter(key="ALLOW_INSTANCE_MULTIPLE_OWNER").update(value="False")
+        self.client.post(add, {"user_id": self.carol.pk})
+        self.assertFalse(UserInstance.objects.filter(user=self.carol).exists())
+        ui = UserInstance.objects.get(user=self.bob)
+        self.client.post(reverse("instances:del_owner", args=[self.vm.pk]), {"userinstance": ui.pk})
+        self.assertEqual(rows(), [
+            ("VM vm owner bob: add", True),
+            ("VM vm owner carol: add: only one owner is allowed", False),
+            ("VM vm owner bob: remove", True),
+        ])
+        shown = [log["message"] for log in self.client.get(reverse("vm_logs", args=["vm"])).json()]
+        self.assertCountEqual(shown, [m for m, _ in rows()])
+
+
+class FlavorRowTests(TestCase):
+    def setUp(self):
+        self.admin = get_user_model().objects.create_superuser("audit-admin", "audit@example.com", "pw")
+        self.client.force_login(self.admin)
+        Logs.objects.all().delete()  # the login's row
+
+    def test_flavor_web_and_api(self):
+        from instances.models import Flavor
+        from rest_framework.test import APIClient
+
+        data = {"label": "tiny-a", "memory": 512, "vcpu": 1, "disk": 10}
+        self.client.post(reverse("instances:flavor_create"), data)
+        flavor = Flavor.objects.get(label="tiny-a")
+        self.client.post(reverse("instances:flavor_update", args=[flavor.pk]), {**data, "vcpu": 2})
+        self.client.post(reverse("instances:flavor_update", args=[flavor.pk]), {**data, "vcpu": 2})  # no change
+        self.client.post(reverse("instances:flavor_delete", args=[flavor.pk]))
+        self.assertFalse(Flavor.objects.filter(label="tiny-a").exists())
+
+        api = APIClient()
+        api.force_authenticate(self.admin)
+        data = {"label": "huge-a", "memory": 4096, "vcpu": 4, "disk": 40}
+        self.assertEqual(api.post(reverse("instance-flavor-list"), data, format="json").status_code, 201)
+        detail = reverse("instance-flavor-detail", args=[Flavor.objects.get(label="huge-a").pk])
+        self.assertEqual(api.patch(detail, {"vcpu": 8}, format="json").status_code, 200)
+        self.assertEqual(api.put(detail, {**data, "vcpu": 8, "memory": 8192}, format="json").status_code, 200)
+        self.assertEqual(Flavor.objects.get(label="huge-a").memory, 8192)
+        self.assertEqual(api.delete(detail).status_code, 204)
+        self.assertFalse(Flavor.objects.filter(label="huge-a").exists())
+
+        self.assertEqual([m for m, _ in rows()], [
+            "Flavor tiny-a: add", "Flavor tiny-a: change vcpu", "Flavor tiny-a: delete",
+            "Flavor huge-a: add", "Flavor huge-a: change vcpu", "Flavor huge-a: change memory", "Flavor huge-a: delete",
+        ])

instances/views.py

@@ -36,6 +36,8 @@ from libvirt import (VIR_DOMAIN_UNDEFINE_KEEP_NVRAM,
                      VIR_DOMAIN_UNDEFINE_NVRAM,
                      VIR_ERR_NO_SUPPORT,
                      libvirtError)
+from logs.audit import audit, changed_fields, failed
+from logs.audit import snapshot as audit_snapshot
 from logs.views import addlogmsg
 from operations import claims, jobs
 from operations.utils import busy_message, summary
@@ -1731,15 +1733,16 @@ def add_owner(request, pk):
     if app_settings.ALLOW_INSTANCE_MULTIPLE_OWNER == "False":
         check_inst = UserInstance.objects.filter(instance=instance).count()
 
+    subject = f"VM {instance.name} owner {user.username}"
     if check_inst > 0:
+        failed(request, instance.compute, subject, "add", "only one owner is allowed", instance=instance.name)
         messages.error(
             request, _("Only one owner is allowed and the one already added")
         )
     else:
-        if UserInstance.objects.get_or_create(instance=instance, user=user)[1]:
-            msg = _("Add owner: %(user)s") % {"user": user}
-            addlogmsg(request.user.username, instance.compute.name, instance.name, msg)
-        else:
+        with audit(request, instance.compute, subject, "add", instance=instance.name) as entry:
+            entry.skip = not UserInstance.objects.get_or_create(instance=instance, user=user)[1]
+        if entry.skip:
             messages.warning(request, _("%(user)s is already an owner") % {"user": user})
     return _back(request, pk, "users")
 
@@ -1752,9 +1755,9 @@ def del_owner(request, pk):
     if not userinstance_id.isdigit():
         raise Http404
     userinstance = get_object_or_404(UserInstance, pk=userinstance_id, instance=instance)
-    userinstance.delete()
-    msg = _("Delete owner: %(userinstance_id)s ") % {"userinstance_id": userinstance_id}
-    addlogmsg(request.user.username, instance.compute.name, instance.name, msg)
+    with audit(request, instance.compute, f"VM {instance.name} owner {userinstance.user.username}", "remove",
+               instance=instance.name):
+        userinstance.delete()
     return _back(request, pk, "users")
 
 
@@ -2164,7 +2167,8 @@ def create_instance(request, compute_id, arch, machine):
 def flavor_create(request):
     form = FlavorForm(request.POST or None)
     if form.is_valid():
-        form.save()
+        with audit(request, None, f"Flavor {form.cleaned_data['label']}", "add"):
+            form.save()
         messages.success(request, _("Flavor Created"))
         return get_safe_redirect(request, default=reverse("instances:index"))
 
@@ -2178,9 +2182,14 @@ def flavor_create(request):
 @superuser_only
 def flavor_update(request, pk):
     flavor = get_object_or_404(Flavor, pk=pk)
+    subject, before = f"Flavor {flavor.label}", audit_snapshot(flavor)
     form = FlavorForm(request.POST or None, instance=flavor)
     if form.is_valid():
-        form.save()
+        with audit(request, None, subject, "change") as entry:
+            form.save()
+            fields = changed_fields(before, audit_snapshot(flavor))
+            entry.action = "change " + ", ".join(fields)
+            entry.skip = not fields
         messages.success(request, _("Flavor Updated"))
         return get_safe_redirect(request, default=reverse("instances:index"))
 
@@ -2195,7 +2204,8 @@ def flavor_update(request, pk):
 def flavor_delete(request, pk):
     flavor = get_object_or_404(Flavor, pk=pk)
     if request.method == "POST":
-        flavor.delete()
+        with audit(request, None, f"Flavor {flavor.label}", "delete"):
+            flavor.delete()
         messages.success(request, _("Flavor Deleted"))
         return get_safe_redirect(request, default=reverse("instances:index"))