From 4d34d9d07c7a58b0051f6146ce50cd6d9ac982dc Mon Sep 17 00:00:00 2001 From: catborise Date: Fri, 2 Oct 2026 20:33:12 +0300 Subject: [PATCH 1/4] fix: add an authorization matrix test and close the endpoints it finds Add webvirtcloud/test_authorization_matrix.py, which walks every named URL and checks that a user without any VM/compute relation only reaches an explicit allowlist, and that the global view_instances permission does not open consoles. It found: - /computes//... views without any check (compute_graph, get_compute_*, get_dom_capabilities, storages.get_volumes): the whole computes URL namespace is now wrapped with superuser_only, so a view added there later cannot be exposed by a missing decorator. - guess_mac_address (leaks dhcpd.conf host/MAC pairs), random_mac_address and guess_clone_name: superuser-only, like the buttons that use them. - check_instance: cross-tenant VM name lookup with no caller; removed. - getvvfile and get_vdi_url: reachable with only view_instances, and the .vv file contains the VNC password. Console access now follows one rule, instances.utils.can_open_console: an active superuser or an active owner of the VM. The global view_instances permission is read-only. It is used by the console page, novncd, getvvfile and get_vdi_url (getvvfile and the VDI tab are kept for virt-viewer users). The console page answers 403 instead of 500 when access is denied, with the same answer for a missing VM. --- computes/urls.py | 21 ++- console/novncd | 11 +- console/tests.py | 28 +++- console/views.py | 32 ++--- datasource/views.py | 7 + instances/test_console_rule.py | 74 +++++++++++ instances/tests.py | 7 - instances/urls.py | 1 - instances/utils.py | 14 ++ instances/views.py | 14 +- webvirtcloud/test_authorization_matrix.py | 151 ++++++++++++++++++++++ 11 files changed, 314 insertions(+), 46 deletions(-) create mode 100644 instances/test_console_rule.py create mode 100644 webvirtcloud/test_authorization_matrix.py diff --git a/computes/urls.py b/computes/urls.py index 99a3743a..00588c01 100644 --- a/computes/urls.py +++ b/computes/urls.py @@ -1,6 +1,7 @@ from virtsecrets.views import secrets -from django.urls import include, path +from admin.decorators import superuser_only +from django.urls import URLPattern, URLResolver, include, path from interfaces.views import interface, interfaces from networks.views import network, networks from nwfilters.views import nwfilter, nwfilters @@ -8,6 +9,22 @@ from storages.views import create_volume, get_volumes, storage, storages from . import forms, views + +def superuser_only_patterns(patterns): + """ + Wrap every view below these patterns with superuser_only. Everything under + /computes/ is host administration (ROADMAP S-10), so the check is applied + to the whole namespace instead of per view, and a view added here later + cannot be exposed by forgetting a decorator. + """ + for pattern in patterns: + if isinstance(pattern, URLResolver): + superuser_only_patterns(pattern.url_patterns) + elif isinstance(pattern, URLPattern): + pattern.callback = superuser_only(pattern.callback) + return patterns + + urlpatterns = [ path("", views.computes, name="computes"), path( @@ -77,3 +94,5 @@ urlpatterns = [ ), ), ] + +superuser_only_patterns(urlpatterns) diff --git a/console/novncd b/console/novncd index c307293d..c11aca7b 100755 --- a/console/novncd +++ b/console/novncd @@ -89,8 +89,8 @@ def get_connection_infos(token): from django.conf import settings from django.contrib.auth import get_user_model from django.core.signing import BadSignature, SignatureExpired, TimestampSigner - from accounts.models import UserInstance from instances.models import Instance + from instances.utils import can_open_console from vrtManager.instance import wvmInstance host = None @@ -126,13 +126,12 @@ def get_connection_infos(token): User = get_user_model() try: user = User.objects.get(id=user_id) - if not user.is_active: - raise PermissionError("User account is inactive or disabled") - if not user.is_superuser and not user.has_perm("instances.view_instances"): - if not UserInstance.objects.filter(instance=instance, user=user).exists(): - raise PermissionError("User no longer has access to this instance") except User.DoesNotExist: raise PermissionError("User not found") + if not user.is_active: + raise PermissionError("User account is inactive or disabled") + if not can_open_console(user, instance): + raise PermissionError("User no longer has access to this instance") conn = wvmInstance( instance.compute.hostname, diff --git a/console/tests.py b/console/tests.py index 5cdc4e45..b006ffe6 100644 --- a/console/tests.py +++ b/console/tests.py @@ -154,11 +154,13 @@ class ConsoleViewsTestCase(TestCase): response = self.client.get(url) # Non-permitted user attempting to view other user's instance - self.assertEqual(response.status_code, 500) + self.assertEqual(response.status_code, 403) self.assertIn("permission", response.content.decode("utf-8").lower()) @patch("console.views.wvmInstance") - def test_console_user_with_view_instances_perm(self, mock_wvm): + def test_console_view_instances_perm_does_not_open_console(self, mock_wvm): + # ROADMAP S-09: global view_instances is a read-only role; the console + # is for superusers and VM owners only. mock_conn = MagicMock() mock_conn.get_console_type.return_value = "vnc" mock_conn.get_console_websocket_port.return_value = None @@ -171,7 +173,8 @@ class ConsoleViewsTestCase(TestCase): url = reverse("console") + f"?token={self.token}" response = self.client.get(url) - self.assertEqual(response.status_code, 200) + self.assertEqual(response.status_code, 403) + self.assertTemplateNotUsed(response, "console-vnc-lite.html") @patch("console.views.wvmInstance") @override_settings(WS_PUBLIC_PATH="/novncd/") @@ -522,6 +525,25 @@ class NovncdDaemonLogicTestCase(TestCase): with self.assertRaises(PermissionError): novncd_mod.get_connection_infos(token) + def test_get_connection_infos_view_instances_user_is_not_enough(self): + User = get_user_model() + viewer = User.objects.create_user(username="novnc_viewer", password="password") + viewer.user_permissions.add(Permission.objects.get(codename="view_instances")) + compute = Compute.objects.create( + name="novnc-viewer-compute", hostname="127.0.0.1", login="root", password="", type=1 + ) + instance = Instance.objects.create( + compute=compute, + name="novnc-viewer-vm", + uuid="55555555-5555-5555-5555-555555555555", + ) + + signer = TimestampSigner(salt="console.novnc") + token = signer.sign(f"{compute.id}:{instance.uuid}:{viewer.id}") + + with self.assertRaises(PermissionError): + novncd_mod.get_connection_infos(token) + def test_get_connection_infos_inactive_user(self): User = get_user_model() inactive_user = User.objects.create_user( diff --git a/console/views.py b/console/views.py index a4350bd0..444745e2 100644 --- a/console/views.py +++ b/console/views.py @@ -1,14 +1,14 @@ # pylint: disable=no-name-in-module,no-member import re -from accounts.models import UserInstance from appsettings.settings import app_settings from django.conf import settings from django.core.signing import BadSignature, SignatureExpired, TimestampSigner -from django.http.response import HttpResponseServerError +from django.http.response import HttpResponseForbidden from django.shortcuts import render from django.utils.translation import gettext_lazy as _ from instances.models import Instance +from instances.utils import can_open_console from libvirt import libvirtError from vrtManager.instance import wvmInstance @@ -79,24 +79,16 @@ def console(request): if host is None or uuid is None: raise ValueError("Invalid token") - if not request.user.is_superuser and not request.user.has_perm( - "instances.view_instances" - ): - try: - userInstance = UserInstance.objects.get( - instance__compute_id=host, - instance__uuid=uuid, - user__id=request.user.id, - ) - instance = Instance.objects.get(compute_id=host, uuid=uuid) - except UserInstance.DoesNotExist: - instance = None - console_error = _( - "User does not have permission to access console or host/instance not exist" - ) - return HttpResponseServerError(console_error) - else: - instance = Instance.objects.get(compute_id=host, uuid=uuid) + instance = Instance.objects.filter(compute_id=host, uuid=uuid).first() + if instance is None and request.user.is_superuser: + raise Instance.DoesNotExist + # Same answer for "no such VM" and "not allowed", so the response + # does not reveal whether a VM exists. + if instance is None or not can_open_console(request.user, instance): + console_error = _( + "User does not have permission to access console or host/instance not exist" + ) + return HttpResponseForbidden(console_error) # Generate secure signed token for noVNC daemon token = signer.sign(f"{host}:{uuid}:{request.user.id}") diff --git a/datasource/views.py b/datasource/views.py index bbdd27a6..4bead1e8 100644 --- a/datasource/views.py +++ b/datasource/views.py @@ -3,8 +3,11 @@ import socket from accounts.models import UserInstance, UserSSHKey from computes.models import Compute +from django.core.exceptions import PermissionDenied from django.http import Http404, HttpResponse from django.shortcuts import get_object_or_404, render +from instances.models import Instance +from instances.utils import can_open_console from libvirt import libvirtError from vrtManager.instance import wvmInstance @@ -95,6 +98,10 @@ def get_vdi_url(request, compute_id, vname): :return: """ compute = get_object_or_404(Compute, pk=compute_id) + # The VDI URL points straight at the VM's console: same rule as the console. + instance = Instance.objects.filter(compute=compute, name=vname).first() + if instance is None or not can_open_console(request.user, instance): + raise PermissionDenied try: conn = wvmInstance( diff --git a/instances/test_console_rule.py b/instances/test_console_rule.py new file mode 100644 index 00000000..ee3664e8 --- /dev/null +++ b/instances/test_console_rule.py @@ -0,0 +1,74 @@ +""" +ROADMAP Wave 0 item 5 (S-09): one rule decides who may open a VM's console +(noVNC page, novncd, the virt-viewer .vv file, the VDI URL): a superuser or +an owner of the VM. The global view_instances permission is read-only. +""" +from unittest.mock import MagicMock, patch + +from django.contrib.auth import get_user_model +from django.contrib.auth.models import Permission +from django.test import TestCase +from django.urls import reverse + +from accounts.models import UserInstance +from computes.models import Compute +from instances.models import Instance +from instances.utils import can_open_console + + +class CanOpenConsoleTestCase(TestCase): + def setUp(self): + User = get_user_model() + self.superuser = User.objects.create_superuser( + username="cons_super", email="cons_super@example.com", password="x" + ) + self.owner = User.objects.create_user(username="cons_owner", password="x") + self.inactive_owner = User.objects.create_user( + username="cons_inactive", password="x", is_active=False + ) + self.viewer = User.objects.create_user(username="cons_viewer", password="x") + self.viewer.user_permissions.add(Permission.objects.get(codename="view_instances")) + self.stranger = User.objects.create_user(username="cons_stranger", password="x") + + self.compute = Compute.objects.create( + name="cons-compute", hostname="127.0.0.1:1", login="root", password="", type=1 + ) + self.instance = Instance.objects.create( + compute=self.compute, + name="cons-vm", + uuid="77777777-8888-9999-aaaa-bbbbbbbbbbbb", + ) + UserInstance.objects.create(instance=self.instance, user=self.owner) + UserInstance.objects.create(instance=self.instance, user=self.inactive_owner) + + def test_superuser_and_owner_may_open_console(self): + self.assertTrue(can_open_console(self.superuser, self.instance)) + self.assertTrue(can_open_console(self.owner, self.instance)) + + def test_view_instances_stranger_and_inactive_owner_may_not(self): + self.assertFalse(can_open_console(self.viewer, self.instance)) + self.assertFalse(can_open_console(self.stranger, self.instance)) + self.assertFalse(can_open_console(self.inactive_owner, self.instance)) + + def test_owner_can_download_vv_file(self): + self.client.force_login(self.owner) + with patch("instances.views.wvmInstances") as mock_conn_cls: + conn = mock_conn_cls.return_value + conn.graphics_type.return_value = "vnc" + conn.graphics_listen.return_value = "127.0.0.1" + conn.graphics_port.return_value = "5900" + conn.domain_name.return_value = "cons-vm" + conn.graphics_passwd.return_value = "secret" + res = self.client.get(reverse("instances:getvvfile", args=[self.instance.id])) + self.assertEqual(res.status_code, 200) + self.assertIn(b"[virt-viewer]", res.content) + + def test_owner_can_get_vdi_url(self): + self.client.force_login(self.owner) + with patch("datasource.views.wvmInstance", MagicMock()), patch( + "datasource.views.get_hostname_by_ip", return_value="host" + ): + res = self.client.get( + reverse("vdi_url", args=[self.compute.id, self.instance.name]) + ) + self.assertEqual(res.status_code, 200) diff --git a/instances/tests.py b/instances/tests.py index 4ff0c96b..dd759562 100644 --- a/instances/tests.py +++ b/instances/tests.py @@ -805,13 +805,6 @@ class InstancesTestCase(TestCase): ) self.assertEqual(response.status_code, 200) - def test_check_instance(self): - response = self.client.get( - reverse("instances:check_instance", args=["test-vm"]) - ) - self.assertEqual(response.status_code, 200) - self.assertJSONEqual(response.content, {"vname": "test-vm", "exists": True}) - def test_start_template(self): # starting templates must fail self.assertEqual(self.instance.status, 5) diff --git a/instances/urls.py b/instances/urls.py index acb54447..d3120504 100755 --- a/instances/urls.py +++ b/instances/urls.py @@ -82,6 +82,5 @@ urlpatterns = [ ), path("guess_clone_name/", views.guess_clone_name, name="guess_clone_name"), path("random_mac_address/", views.random_mac_address, name="random_mac_address"), - path("check_instance//", views.check_instance, name="check_instance"), path("/sshkeys/", views.sshkeys, name="sshkeys"), ] diff --git a/instances/utils.py b/instances/utils.py index 6af6131a..a305dffe 100644 --- a/instances/utils.py +++ b/instances/utils.py @@ -14,6 +14,20 @@ from vrtManager.instance import wvmInstance, wvmInstances from .models import Instance +def can_open_console(user, instance): + """ + The single rule for console access (noVNC page, novncd, the virt-viewer + .vv file and the VDI URL): an active superuser or an active owner of the + VM. The global view_instances permission is read-only and does not grant + an interactive console (ROADMAP S-09). + """ + if not user.is_active: + return False + if user.is_superuser: + return True + return UserInstance.objects.filter(instance=instance, user=user).exists() + + def get_clone_free_names(size=10): prefix = app_settings.CLONE_INSTANCE_DEFAULT_PREFIX free_names = [] diff --git a/instances/views.py b/instances/views.py index 1c45f9d3..bfbec8a9 100755 --- a/instances/views.py +++ b/instances/views.py @@ -255,6 +255,7 @@ def osinfo(request, pk): return JsonResponse(results) +@superuser_only def guess_mac_address(request, vname): data = {"vname": vname} mac = utils.get_dhcp_mac_address(vname) @@ -264,12 +265,14 @@ def guess_mac_address(request, vname): return HttpResponse(json.dumps(data)) +@superuser_only def random_mac_address(request): data = dict() data["mac"] = utils.get_random_mac_address() return HttpResponse(json.dumps(data)) +@superuser_only def guess_clone_name(request): dhcp_file = "/srv/webvirtcloud/dhcpd.conf" prefix = app_settings.CLONE_INSTANCE_DEFAULT_PREFIX @@ -288,14 +291,6 @@ def guess_clone_name(request): return HttpResponse(json.dumps({})) -def check_instance(request, vname): - instance = Instance.objects.filter(name=vname) - data = {"vname": vname, "exists": False} - if instance: - data["exists"] = True - return JsonResponse(data) - - def sshkeys(request, pk): """ :param request: @@ -1729,6 +1724,9 @@ def change_options(request, pk): def getvvfile(request, pk): instance = get_instance(request.user, pk) + # The .vv file contains the VNC password: same rule as the console. + if not utils.can_open_console(request.user, instance): + raise PermissionDenied conn = wvmInstances( instance.compute.hostname, instance.compute.login, diff --git a/webvirtcloud/test_authorization_matrix.py b/webvirtcloud/test_authorization_matrix.py new file mode 100644 index 00000000..935a3e17 --- /dev/null +++ b/webvirtcloud/test_authorization_matrix.py @@ -0,0 +1,151 @@ +""" +Authorization matrix (ROADMAP Wave 0 item 5, S-09/S-10). + +Walks every named URL pattern and checks that an authenticated user without +any relation to a VM or compute cannot reach it, unless the URL is in the +explicit allowlist below. A new endpoint added without an authorization check +makes this test fail, so the allowlist must be extended deliberately. + +Limitation: the compute points at a closed port, so a view that turns a +libvirt connection error into 404 looks "denied" here even without an +authorization check. Endpoints where that matters (consoles) get dedicated +tests with libvirt mocked. +""" +from unittest.mock import MagicMock, patch + +from django.contrib.auth import get_user_model +from django.contrib.auth.models import Permission +from django.test import Client, TestCase +from django.urls import NoReverseMatch, URLPattern, URLResolver, get_resolver, reverse + +from computes.models import Compute +from instances.models import Instance + +# URL name -> why any authenticated user may reach it. +ALLOWED_FOR_ANY_USER = { + "index": "landing page", + "instances:index": "lists only the user's own instances", + "accounts:login": "login page", + "accounts:email_otp": "part of the login flow (S-12 tracks its own issues)", + "accounts:logout": "logout", + "accounts:profile": "the user's own profile", + "accounts:ssh_key_create": "the user's own SSH keys", + "set_language": "UI language switch", + "rest_framework:login": "DRF browsable API login", + "rest_framework:logout": "DRF browsable API logout", + "console": "renders an error without a valid token; access is checked per VM", + "ds_openstack_index": "static cloud-init datasource index (F-14)", + "instance-list": "API: filtered to the user's own instances", + "compute-instance-list": "API: filtered to the user's own instances", + "instance-flavor-list": "API: flavor catalogue", + "instance-flavor-detail": "API: flavor catalogue", + "schema": "OpenAPI schema (SERVE_PERMISSIONS is S-21, Wave 3)", + "schema-json": "OpenAPI schema (SERVE_PERMISSIONS is S-21, Wave 3)", + "schema-redoc": "API docs (SERVE_PERMISSIONS is S-21, Wave 3)", + "schema-swagger-ui": "API docs (SERVE_PERMISSIONS is S-21, Wave 3)", +} + +# Endpoints that expose a VM's console (or its VNC password) and therefore +# must follow the console rule: superuser or VM owner, not global view_instances. +CONSOLE_ENDPOINTS = ["instances:getvvfile", "vdi_url"] + +DENIED = (403, 404) + + +def iter_patterns(patterns, prefix="", params=None): + params = params or {} + for p in patterns: + own = {**{g: None for g in p.pattern.regex.groupindex}, **p.pattern.converters} + if isinstance(p, URLResolver): + ns = f"{prefix}{p.namespace}:" if p.namespace else prefix + yield from iter_patterns(p.url_patterns, ns, {**params, **own}) + elif isinstance(p, URLPattern) and p.name: + yield prefix + p.name, {**params, **own} + + +class AuthorizationMatrixTestCase(TestCase): + def setUp(self): + User = get_user_model() + self.user = User.objects.create_user(username="matrix_plain", password="x") + self.viewer = User.objects.create_user(username="matrix_viewer", password="x") + self.viewer.user_permissions.add(Permission.objects.get(codename="view_instances")) + # Port 1 is closed, so any view that reaches libvirt fails fast. + self.compute = Compute.objects.create( + name="matrix-compute", hostname="127.0.0.1:1", login="root", password="", type=1 + ) + self.instance = Instance.objects.create( + compute=self.compute, + name="matrix-vm", + uuid="22222222-3333-4444-5555-666666666666", + ) + self.client = Client(raise_request_exception=False) + + def _kwargs(self, params): + kwargs = {} + for name, converter in params.items(): + if name in ("compute_id", "compute_pk"): + kwargs[name] = self.compute.pk + elif name in ("pk", "instance_id"): + kwargs[name] = self.instance.pk if converter else str(self.instance.pk) + elif name == "vname": + kwargs[name] = self.instance.name + elif name == "format": + kwargs[name] = "json" + elif converter is not None and type(converter).__name__ == "IntConverter": + kwargs[name] = 1 + else: + kwargs[name] = "x" + return kwargs + + def _request(self, user, url): + self.client.force_login(user) + response = self.client.get(url) + if response.status_code == 405: + self.client.force_login(user) + response = self.client.post(url, {}) + return response + + def _url(self, name, params): + return reverse(name, kwargs=self._kwargs(params)) + + def test_plain_user_cannot_reach_anything_outside_the_allowlist(self): + reachable = [] + for name, params in iter_patterns(get_resolver().url_patterns): + if name in ALLOWED_FOR_ANY_USER: + continue + try: + url = self._url(name, params) + except NoReverseMatch: + reachable.append(f"{name}: cannot build URL, extend _kwargs()") + continue + response = self._request(self.user, url) + if response.status_code not in DENIED: + reachable.append(f"{name} ({url}) -> {response.status_code}") + self.assertEqual( + reachable, + [], + "Reachable by a user without any VM/compute relation:\n" + "\n".join(reachable), + ) + + def test_allowlist_only_names_existing_urls(self): + names = {name for name, _ in iter_patterns(get_resolver().url_patterns)} + self.assertEqual(sorted(set(ALLOWED_FOR_ANY_USER) - names), []) + + def test_global_view_permission_does_not_open_consoles(self): + params = dict(iter_patterns(get_resolver().url_patterns)) + reachable = [] + # Mock libvirt so the views would succeed if authorization let them through. + with patch("datasource.views.wvmInstance", MagicMock()), patch( + "datasource.views.get_hostname_by_ip", return_value="host" + ), patch("instances.views.wvmInstances", MagicMock()): + for name in CONSOLE_ENDPOINTS: + url = self._url(name, params[name]) + response = self._request(self.viewer, url) + if response.status_code not in DENIED: + reachable.append(f"{name} ({url}) -> {response.status_code}") + self.assertEqual( + reachable, + [], + "Console endpoints reachable with only global view_instances:\n" + + "\n".join(reachable), + ) From b7a39d24e268f463a533084031ea956b89842ee1 Mon Sep 17 00:00:00 2001 From: catborise Date: Fri, 2 Oct 2026 20:35:29 +0300 Subject: [PATCH 2/4] fix(console): accept only signed host:uuid:user_id tokens in novncd novncd accepted signed tokens without the user part, which skipped the per-user access check, and accepted unsigned "host-uuid" tokens whenever "test" appeared in sys.argv. The console view always signs host:uuid:user_id, so anything else is now rejected with PermissionError, and the user check always runs. --- console/novncd | 49 ++++++++++++++++-------------------------------- console/tests.py | 32 +++++++++++++++++++++++++++++++ 2 files changed, 48 insertions(+), 33 deletions(-) diff --git a/console/novncd b/console/novncd index c11aca7b..758a92d4 100755 --- a/console/novncd +++ b/console/novncd @@ -86,52 +86,35 @@ except ImportError: def get_connection_infos(token): - from django.conf import settings from django.contrib.auth import get_user_model from django.core.signing import BadSignature, SignatureExpired, TimestampSigner from instances.models import Instance from instances.utils import can_open_console from vrtManager.instance import wvmInstance - host = None - uuid = None - user_id = None - + # The console view signs exactly "host:uuid:user_id". Anything else + # (unsigned, expired, or without the user part) is rejected, because the + # per-user access check below depends on user_id. signer = TimestampSigner(salt="console.novnc") try: unsigned = signer.unsign(token, max_age=120) - parts = unsigned.split(":") - if len(parts) >= 2: - host = int(parts[0]) - uuid = parts[1] - if len(parts) >= 3: - user_id = int(parts[2]) + host, uuid, user_id = unsigned.split(":") + host = int(host) + user_id = int(user_id) except (BadSignature, SignatureExpired, ValueError, TypeError): - if "test" in sys.argv: - try: - temptoken = token.split("-", 1) - host = int(temptoken[0]) - uuid = temptoken[1] - except Exception: - raise - else: - raise PermissionError("Invalid or expired console token") - - if host is None or uuid is None: - raise ValueError("Invalid console token format") + raise PermissionError("Invalid or expired console token") try: instance = Instance.objects.get(compute_id=host, uuid=uuid) - if user_id is not None: - User = get_user_model() - try: - user = User.objects.get(id=user_id) - except User.DoesNotExist: - raise PermissionError("User not found") - if not user.is_active: - raise PermissionError("User account is inactive or disabled") - if not can_open_console(user, instance): - raise PermissionError("User no longer has access to this instance") + User = get_user_model() + try: + user = User.objects.get(id=user_id) + except User.DoesNotExist: + raise PermissionError("User not found") + if not user.is_active: + raise PermissionError("User account is inactive or disabled") + if not can_open_console(user, instance): + raise PermissionError("User no longer has access to this instance") conn = wvmInstance( instance.compute.hostname, diff --git a/console/tests.py b/console/tests.py index b006ffe6..84cf95d9 100644 --- a/console/tests.py +++ b/console/tests.py @@ -525,6 +525,38 @@ class NovncdDaemonLogicTestCase(TestCase): with self.assertRaises(PermissionError): novncd_mod.get_connection_infos(token) + def test_get_connection_infos_rejects_signed_token_without_user(self): + # The console view always signs host:uuid:user_id; a token without the + # user part would skip the per-user access check entirely. + compute = Compute.objects.create( + name="novnc-2part-compute", hostname="127.0.0.1", login="root", password="", type=1 + ) + instance = Instance.objects.create( + compute=compute, + name="novnc-2part-vm", + uuid="66666666-6666-6666-6666-666666666666", + ) + signer = TimestampSigner(salt="console.novnc") + token = signer.sign(f"{compute.id}:{instance.uuid}") + + with self.assertRaises(PermissionError): + novncd_mod.get_connection_infos(token) + + def test_get_connection_infos_rejects_unsigned_token_even_under_test_runner(self): + # sys.argv contains "test" while this suite runs; production code must + # not change behaviour based on that. + compute = Compute.objects.create( + name="novnc-unsigned-compute", hostname="127.0.0.1", login="root", password="", type=1 + ) + instance = Instance.objects.create( + compute=compute, + name="novnc-unsigned-vm", + uuid="88888888-8888-8888-8888-888888888888", + ) + + with self.assertRaises(PermissionError): + novncd_mod.get_connection_infos(f"{compute.id}-{instance.uuid}") + def test_get_connection_infos_view_instances_user_is_not_enough(self): User = get_user_model() viewer = User.objects.create_user(username="novnc_viewer", password="password") From e385014ba9ee1213ad404e1964b36f4529bdc988 Mon Sep 17 00:00:00 2001 From: catborise Date: Fri, 2 Oct 2026 20:36:24 +0300 Subject: [PATCH 3/4] docs(readme): document users, roles and permissions Describe the access model: superuser, the global permissions (view_instances as a read-only auditor role, clone_instances, snapshot_instances, passwordless_console, change_password) and per-VM ownership flags (is_change, is_delete, is_vnc), with a table of which VM actions each combination allows. The table describes the state after all Wave 0 branches are merged: disk/media views are superuser-only (fix/wave0-disk-tenant-isolation) and clone requires is_change except for templates (fix/wave0-clone-permission). --- README.md | 32 ++++++++++++++++++++++++++++++++ 1 file changed, 32 insertions(+) diff --git a/README.md b/README.md index 2239f0cc..cf8f67a4 100644 --- a/README.md +++ b/README.md @@ -439,6 +439,38 @@ python -m unittest discover -s vrtManager -p "test_*.py" > python manage.py test > ``` +## Users, Roles and Permissions + +WebVirtCloud combines three kinds of access rights: + +- **Superuser** (`is_superuser`): full administration — computes, storage pools, networks, interfaces, nwfilters, secrets, app settings, users, and every VM. +- **Global permissions** (Django permissions, set per user or group in the admin pages): + - `instances.view_instances` — **read-only auditor role**: sees every VM in lists and detail pages, but cannot change them and cannot open their consoles. + - `instances.clone_instances` — may clone VMs (see the table for which ones). + - `instances.snapshot_instances` — may create, revert and delete snapshots of VMs they may change. + - `instances.passwordless_console` — the console page fills in the VNC password automatically (granted to all users by default). + - `accounts.change_password` — may change their own password. +- **Per-VM ownership** (an owner entry for a user on a VM, managed by a superuser on the VM's page) with three flags: `is_change`, `is_delete`, `is_vnc`. An owner without flags may see, start and stop the VM and open its console. + +| Action on a VM | Superuser | Owner | Owner flag / permission needed | `view_instances` only | +|---|---|---|---|---| +| See the VM (list, detail) | ✅ | ✅ | — | ✅ | +| Start, shut down, power cycle, force off | ✅ | ✅ | — | ❌ | +| Open the console (noVNC, `.vv` file for virt-viewer, VDI URL) | ✅ | ✅ | — | ❌ | +| Resize CPU/memory/disk, set root password, add SSH key, change title/description | ✅ | ✅ | `is_change` | ❌ | +| Change console settings (type, VNC password, keymap) | ✅ | ✅ | `is_change` and `is_vnc` | ❌ | +| Snapshots | ✅ | ✅ | `is_change` and `instances.snapshot_instances` | ❌ | +| Clone | ✅ | ✅ | `is_change` and `instances.clone_instances` (templates: viewing the template is enough) | templates only, with `instances.clone_instances` | +| Delete the VM | ✅ | ✅ | `is_delete` | ❌ | +| Disks and media (add, attach, edit, detach, delete volumes; CD-ROM/ISO) | ✅ | ❌ | — | ❌ | +| Network interfaces, boot options, vCPU hotplug, autostart, guest agent, suspend/resume, migrate, raw XML, owners | ✅ | ❌ | — | ❌ | + +Notes: + +- Only superusers choose disk names and MAC addresses when cloning; for other users the server derives them. +- `is_staff` gives no access to VMs by itself. +- Users without ownership or `view_instances` get **404** for a VM (its existence is not revealed); users who can see a VM but lack the right for an action get **403**. + ## LDAP Configuration The config options below can be changed in `webvirtcloud/settings.py` file. Variants for Active Directory and OpenLDAP are shown. This is a minimal config to get LDAP running, for further info read the [django-auth-ldap documentation](https://django-auth-ldap.readthedocs.io). From 75c91f8d53ea6d4d244c9e777e75fbbc6a20bf42 Mon Sep 17 00:00:00 2001 From: catborise Date: Fri, 2 Oct 2026 22:04:45 +0300 Subject: [PATCH 4/4] docs(readme): correct the is_staff and 404/403 notes in the roles section is_staff does not reveal console settings or VNC passwords (fixed on fix/wave0-staff-console-password), but staff owners with is_change may also change VMs marked as templates. Superuser-only pages answer 403 to everyone else, and the console page answers 403 both for a missing VM and for a VM the user may not open. --- README.md | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/README.md b/README.md index cf8f67a4..25a4a3f3 100644 --- a/README.md +++ b/README.md @@ -468,8 +468,8 @@ WebVirtCloud combines three kinds of access rights: Notes: - Only superusers choose disk names and MAC addresses when cloning; for other users the server derives them. -- `is_staff` gives no access to VMs by itself. -- Users without ownership or `view_instances` get **404** for a VM (its existence is not revealed); users who can see a VM but lack the right for an action get **403**. +- `is_staff` gives no access to VMs and does not reveal console settings or VNC passwords by itself. On VMs where a staff user already has `is_change`, staff may also change VMs that are marked as templates; other owners cannot. +- On VM pages, users without ownership or `view_instances` get **404** (the VM's existence is not revealed), and users who can see a VM but lack the right for an action get **403**. Superuser-only pages answer **403** to everyone else; the console page answers **403** both for a missing VM and for a VM the user may not open. ## LDAP Configuration