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"))