Merge branch 'fix/wave0-authorization-matrix' into wave0-integration

# Conflicts:
#	datasource/views.py

console/novncd

@@ -86,53 +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 accounts.models import UserInstance
     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)
-                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")
+        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,

console/tests.py

@@ -172,11 +172,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
@@ -189,7 +191,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/")
@@ -540,6 +543,57 @@ 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")
+        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(

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