From e04adec87be2dc3c0a198f8d419d2d27526f173d Mon Sep 17 00:00:00 2001 From: catborise Date: Fri, 23 Oct 2026 10:55:55 +0300 Subject: [PATCH] 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 | 2 +- accounts/tests/test_audit_rows.py | 103 +++++++++++++++++++++++ accounts/views.py | 41 ++++++--- admin/tests/test_audit_rows.py | 121 +++++++++++++++++++++++++++ admin/views.py | 61 ++++++++++---- appsettings/tests/test_audit_rows.py | 44 ++++++++++ appsettings/views.py | 18 ++-- instances/api/viewsets.py | 18 ++++ instances/tests/test_audit_rows.py | 76 +++++++++++++++++ instances/views.py | 30 ++++--- 10 files changed, 468 insertions(+), 46 deletions(-) create mode 100644 accounts/tests/test_audit_rows.py create mode 100644 admin/tests/test_audit_rows.py create mode 100644 appsettings/tests/test_audit_rows.py create mode 100644 instances/tests/test_audit_rows.py diff --git a/README.md b/README.md index 8f715f2d..1ee61495 100644 --- a/README.md +++ b/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. diff --git a/accounts/tests/test_audit_rows.py b/accounts/tests/test_audit_rows.py new file mode 100644 index 00000000..775287b3 --- /dev/null +++ b/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()) diff --git a/accounts/views.py b/accounts/views.py index c903f840..e7983136 100644 --- a/accounts/views.py +++ b/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) diff --git a/admin/tests/test_audit_rows.py b/admin/tests/test_audit_rows.py new file mode 100644 index 00000000..48ae9d2b --- /dev/null +++ b/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)]) diff --git a/admin/views.py b/admin/views.py index 1424f588..d37ccda0 100644 --- a/admin/views.py +++ b/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( diff --git a/appsettings/tests/test_audit_rows.py b/appsettings/tests/test_audit_rows.py new file mode 100644 index 00000000..5fe6f48f --- /dev/null +++ b/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, "-")]) diff --git a/appsettings/views.py b/appsettings/views.py index 2ebb4c70..4a2f63fa 100644 --- a/appsettings/views.py +++ b/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()) diff --git a/instances/api/viewsets.py b/instances/api/viewsets.py index 6300c304..967d3343 100644 --- a/instances/api/viewsets.py +++ b/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): """ diff --git a/instances/tests/test_audit_rows.py b/instances/tests/test_audit_rows.py new file mode 100644 index 00000000..5f1edc55 --- /dev/null +++ b/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", + ]) diff --git a/instances/views.py b/instances/views.py index 15dd59ae..2a6a2f91 100755 --- a/instances/views.py +++ b/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"))