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

fix(logs): blocking a blocked user writes no row; one upload submit path

Browse source 
Follow-ups from the review of the audit log layers:

- Blocking a user who is already blocked, or unblocking one who is
  not, changes nothing and now writes no row, like any other edit
  that changes nothing.
- write_chunk takes the request instead of the user and an optional
  request; the view always had one, so the branch only tests used
  is gone and every chunk 0 goes through the same audited submit.
- Tests: a busy clone creates no job and its row carries the page's
  message; a later chunk that fails writes no submit row; libvirt
  refusing a network or pool delete is a failed row.
This commit is contained in:
catborise 2026-10-23 14:09:26 +03:00
parent d5df187de4
commit b24746c1cb
7 changed files with 52 additions and 29 deletions
Download patch file Download diff file Expand all files Collapse all files

2
admin/tests/test_audit_rows.py
View file

@ -65,6 +65,8 @@ class UserRowTests(TestCase):
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_block", args=[carol.pk])) # already blocked: nothing changes
self.client.post(reverse("admin:user_unblock", 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"})

6
admin/views.py
View file

@ -215,7 +215,8 @@ def user_delete(request, pk):
@superuser_only
def user_block(request, pk):
user: User = get_object_or_404(User, pk=pk)
with audit(request, None, f"User {user.username}", "block"):
with audit(request, None, f"User {user.username}", "block") as entry:
entry.skip = not user.is_active # already blocked
user.is_active = False
user.save()
return redirect("admin:user_list")
@ -225,7 +226,8 @@ def user_block(request, pk):
@superuser_only
def user_unblock(request, pk):
user: User = get_object_or_404(User, pk=pk)
with audit(request, None, f"User {user.username}", "unblock"):
with audit(request, None, f"User {user.username}", "unblock") as entry:
entry.skip = user.is_active # not blocked
user.is_active = True
user.save()
return redirect("admin:user_list")

7
networks/tests/test_audit_rows.py
View file

@ -81,6 +81,13 @@ class NetworkRowTests(TestCase):
self.assertIn(error, [str(m) for m in get_messages(response.wsgi_request)])
self.assertEqual(rows(), [(f"Network lab: stop: {error}", False)])
def test_a_refused_network_delete(self):
error = "Requested operation is not valid: network is in use"
with patch("vrtManager.network.wvmNetwork.delete", side_effect=libvirt.libvirtError(error)):
response = self.post({"delete": ""})
self.assertIn(error, [str(m) for m in get_messages(response.wsgi_request)])
self.assertEqual(rows(), [(f"Network lab: delete: {error}", False)])
def test_network_create(self):
url = reverse("networks", args=[self.compute.id])
self.client.post(url, {"create": "", "name": "audit-net", "forward": "nat", "subnet": "198.51.100.0/24",

38
storages/tests/test_audit_rows.py
View file

@ -4,9 +4,8 @@ from types import SimpleNamespace
from unittest.mock import patch
from django.contrib.auth import get_user_model
from django.contrib.sessions.middleware import SessionMiddleware
from django.core.files.uploadedfile import SimpleUploadedFile
from django.test import RequestFactory, TransactionTestCase
from django.test import TransactionTestCase
from django.urls import reverse
from computes.models import Compute
@ -79,24 +78,17 @@ class CloneRowTests(TransactionTestCase):
def test_a_busy_pool_refuses_the_clone(self):
self.stg.get_volumes.return_value = ["base.qcow2"]
claims.admit({claims.pool_key(self.compute.pk, "default"): Claim.EXCLUSIVE}, holder="192.0.2.30:1:1")
self.client.post(self.url, {"cln_volume": "1", "image": "base.qcow2", "name": "copy",
"submission_key": "key-vol00005"})
[(message, success)] = rows()
self.assertFalse(success)
self.assertTrue(message.startswith("Storage pool default: clone volume base.qcow2 as copy: "), message)
response = self.client.post(self.url, {"cln_volume": "1", "image": "base.qcow2", "name": "copy",
"submission_key": "key-vol00005"}, follow=True)
[shown] = [str(m) for m in response.context["messages"]]
self.assertFalse(Operation.objects.exists())
self.assertEqual(rows(), [(f"Storage pool default: clone volume base.qcow2 as copy: {shown}", False)])
class UploadRowTests(upload_tests.LocalPool, upload_tests.UploadTestCase):
def request(self):
request = RequestFactory().post("/")
request.user = self.admin
SessionMiddleware(lambda r: None).process_request(request)
return request
def chunk(self, index, data, name=" disk.iso", total=2, size=6):
return write_chunk(self.admin, self.conn, self.compute, "default", upload_tests.KEY,
{"name": name, "size": size, "chunks": total}, index, SimpleUploadedFile("chunk", data),
request=self.request())
return write_chunk(self.request(), self.conn, self.compute, "default", upload_tests.KEY,
{"name": name, "size": size, "chunks": total}, index, SimpleUploadedFile("chunk", data))
def test_an_upload_writes_one_row(self):
self.chunk(0, b"abc")
@ -107,6 +99,12 @@ class UploadRowTests(upload_tests.LocalPool, upload_tests.UploadTestCase):
self.assertEqual(rows(), [("Storage pool default: upload volume disk.iso", True)])
self.assertEqual(Logs.objects.get().host, "up-c")
def test_a_failing_later_chunk_writes_no_submit_row(self):
self.chunk(0, b"abc")
with self.assertRaises(ValueError):
self.chunk(1, b"def", name="other.iso") # the session's parameters changed
self.assertEqual(rows(), [])
def test_a_refused_upload_is_a_failed_row(self):
open(self.path(), "wb").close()
with self.assertRaises(FileExistsError):
@ -157,6 +155,14 @@ class PoolAndVolumeRowTests(TransactionTestCase):
self.assertIn(reason, [str(m) for m in response.context["messages"]])
self.assertEqual(rows(), [(f"Storage pool default: stop: {reason}", False)])
def test_a_refused_pool_delete(self):
from vrtManager.util import OperationError
reason = "Pool default holds disks of the VMs web-1"
self.pool.delete.side_effect = OperationError(reason)
self.client.post(self.url, {"delete": "1"})
self.assertEqual(rows(), [(f"Storage pool default: delete: {reason}", False)])
def test_pool_create_and_a_duplicate_name(self):
url = reverse("storages", args=[self.compute.pk])
self.client.post(url, {"create": "1", "name": "audit-pool", "stg_type": "dir", "target": "/srv/audit-pool"})

14
storages/tests/test_upload_job.py
View file

@ -11,7 +11,8 @@ from unittest.mock import MagicMock, patch
from django.contrib.auth import get_user_model
from django.core.exceptions import PermissionDenied
from django.core.files.uploadedfile import SimpleUploadedFile
from django.test import TestCase
from django.contrib.sessions.middleware import SessionMiddleware
from django.test import RequestFactory, TestCase
from django.urls import reverse
from django.utils import timezone
from libvirt import VIR_ERR_NO_STORAGE_VOL, VIR_ERR_SYSTEM_ERROR, libvirtError
@ -50,8 +51,15 @@ class UploadTestCase(TestCase):
return patch.object(target_class, "write", cancelled)
def request(self, user=None):
"""A chunk request of user (the admin), with the session remember_job writes to."""
request = RequestFactory().post("/")
request.user = user or self.admin
SessionMiddleware(lambda r: None).process_request(request)
return request
def send(self, index, data, total=3, size=9, name="disk.iso", key=KEY, user=None):
return write_chunk(user or self.admin, self.conn, self.compute, "default", key,
return write_chunk(self.request(user), self.conn, self.compute, "default", key,
{"name": name, "size": size, "chunks": total}, index, SimpleUploadedFile("chunk", data))
def job(self):
@ -170,7 +178,7 @@ class LocalUploadTests(LocalPool, UploadTestCase):
chunk.chunks = interrupted
with self.assertRaises(OSError):
write_chunk(self.admin, self.conn, self.compute, "default", KEY,
write_chunk(self.request(), self.conn, self.compute, "default", KEY,
{"name": "disk.iso", "size": 9, "chunks": 3}, 0, chunk)
op = self.job()
self.assertEqual((op.intent, op.progress_done), ({"chunk": 0, "created": True}, None))

10
storages/upload.py
View file

@ -498,21 +498,19 @@ def end(op, state, error=""):
pass
def write_chunk(user, conn, compute, pool, upload_id, raw, chunk_index, chunk, request=None):
def write_chunk(request, conn, compute, pool, upload_id, raw, chunk_index, chunk):
"""Write chunk chunk_index of the upload upload_id (the job's submission
key) and commit it on the job; the last chunk renames the image."""
kind = jobs.get_kind("volume.upload")
raw = {**raw, "compute": compute.pk, "pool": pool}
if chunk_index == 0 and request is not None:
if chunk_index == 0:
name = os.path.basename(str(raw["name"])).strip()
with audit(request, compute, f"Storage pool {pool}", f"upload volume {name}") as entry:
op = jobs.submit(kind.name, user, raw, upload_id)
op = jobs.submit(kind.name, request.user, raw, upload_id)
entry.skip = True # the job writes its row when it ends
jobs.remember_job(request, op)
elif chunk_index == 0:
op = jobs.submit(kind.name, user, raw, upload_id)
else:
op = Operation.objects.filter(user=user, submission_key=upload_id).first()
op = Operation.objects.filter(user=request.user, submission_key=upload_id).first()
if op is None:
raise ValueError(_("Upload session not found or expired"))
if op.kind != kind.name or op.params != kind.normalize(raw):

4
storages/views.py
View file

@ -40,9 +40,9 @@ def _handle_iso_upload(request, conn, compute, pool):
return JsonResponse({"error": _("Invalid file size.")}, status=400)
try:
op = write_chunk(request.user, conn, compute, pool, request.POST.get("upload_id", ""),
op = write_chunk(request, conn, compute, pool, request.POST.get("upload_id", ""),
{"name": request.POST.get("file_name", ""), "size": file_size, "chunks": total_chunks},
chunk_index, file_chunk, request=request)
chunk_index, file_chunk)
except claims.Busy as busy:
if busy.blocker is not None: # another job holds the pool or the name
return JsonResponse({"error": busy_message(busy, request.user)}, status=400)