Merge branch 'fix/wave0-staff-console-password' into wave0-integration

instances/templates/instances/settings_tab.html

@@ -23,7 +23,7 @@
                 </button>
             </li>
         {% endif %}
-        {% if request.user.is_superuser or request.user.is_staff or userinstance.is_vnc %}
+        {% if can_manage_console %}
             <li class="nav-item" role="presentation">
                 <button class="nav-link" data-bs-toggle="tab" data-bs-target="#vncsettings" type="button" role="tab" aria-controls="vncsettings" aria-selected="false">
                     {% trans "Console" %}
@@ -711,7 +711,7 @@
             <div class="clearfix"></div>
         </div>
         {% endif %}
-        {% if request.user.is_superuser or request.user.is_staff or userinstance.is_vnc %}
+        {% if can_manage_console %}
         <div role="tabpanel" class="tab-pane tab-pane-bordered" id="vncsettings">
             <form method="post" action="{% url 'instances:update_console' instance.id %}">
                 {% csrf_token %}

instances/test_console_settings_access.py

@@ -0,0 +1,87 @@
+"""
+The console settings form on the VM detail page carries the VNC password
+(PasswordInput(render_value=True)). It must only be rendered for users who may
+change console settings, which is the rule update_console enforces: a
+superuser, or an owner with both is_change and is_vnc. In particular, is_staff
+and the global view_instances permission must not reveal the VNC password.
+"""
+from unittest.mock import 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
+
+VNC_PASSWORD = "s3cretVNCpass"
+
+
+class ConsoleSettingsAccessTestCase(TestCase):
+    def setUp(self):
+        User = get_user_model()
+        view_perm = Permission.objects.get(codename="view_instances")
+        self.superuser = User.objects.create_superuser(
+            username="vncpw_super", email="vncpw_super@example.com", password="x"
+        )
+        self.staff_viewer = User.objects.create_user(
+            username="vncpw_staff_viewer", password="x", is_staff=True
+        )
+        self.staff_viewer.user_permissions.add(view_perm)
+        self.staff_owner = User.objects.create_user(
+            username="vncpw_staff_owner", password="x", is_staff=True
+        )
+        self.vnc_owner = User.objects.create_user(username="vncpw_vnc_owner", password="x")
+
+        self.compute = Compute.objects.create(
+            name="vncpw-compute", hostname="127.0.0.1:1", login="root", password="", type=1
+        )
+        self.instance = Instance.objects.create(
+            compute=self.compute,
+            name="vncpw-vm",
+            uuid="cccccccc-dddd-eeee-ffff-000000000000",
+        )
+        UserInstance.objects.create(
+            instance=self.instance, user=self.staff_owner, is_change=True, is_vnc=False
+        )
+        UserInstance.objects.create(
+            instance=self.instance, user=self.vnc_owner, is_change=True, is_vnc=True
+        )
+
+    def _detail_page(self, user):
+        self.client.force_login(user)
+        with patch("instances.models.wvmInstance") as mock_wvm, patch(
+            "instances.views.utils.check_user_quota", return_value=""
+        ):
+            proxy = mock_wvm.return_value
+            proxy.get_memory.return_value = 1024
+            proxy.get_cur_memory.return_value = 1024
+            proxy.get_max_memory.return_value = 4096
+            proxy.get_networks.return_value = []
+            proxy.get_ifaces.return_value = []
+            proxy.get_storages.return_value = []
+            proxy.get_console_passwd.return_value = VNC_PASSWORD
+            proxy.get_console_type.return_value = "vnc"
+            proxy.get_status.return_value = 5
+            proxy.get_disk_devices.return_value = []
+            proxy.get_media_devices.return_value = []
+            proxy.get_net_devices.return_value = []
+            response = self.client.get(
+                reverse("instances:instance", args=[self.instance.id])
+            )
+        self.assertEqual(response.status_code, 200)
+        return response.content.decode()
+
+    def test_staff_with_view_instances_does_not_see_vnc_password(self):
+        self.assertFalse(VNC_PASSWORD in self._detail_page(self.staff_viewer), "VNC password leaked")
+
+    def test_staff_owner_without_is_vnc_does_not_see_vnc_password(self):
+        self.assertFalse(VNC_PASSWORD in self._detail_page(self.staff_owner), "VNC password leaked")
+
+    def test_owner_with_is_change_and_is_vnc_sees_console_settings(self):
+        self.assertTrue(VNC_PASSWORD in self._detail_page(self.vnc_owner), "console settings missing")
+
+    def test_superuser_sees_console_settings(self):
+        self.assertTrue(VNC_PASSWORD in self._detail_page(self.superuser), "console settings missing")

instances/utils.py

@@ -264,3 +264,18 @@ def get_clone_disk_name(disk, prefix, clone_name=""):
     else:
         image = f"{disk['image']}-clone"
     return image
+
+
+def can_manage_console(user, instance):
+    """
+    Who may see and change a VM's console settings, including its VNC
+    password: an active superuser, or an active owner with both is_change and
+    is_vnc. is_staff and the global view_instances permission do not count.
+    """
+    if not user.is_active:
+        return False
+    if user.is_superuser:
+        return True
+    return UserInstance.objects.filter(
+        instance=instance, user=user, is_change=True, is_vnc=True
+    ).exists()

instances/views.py

@@ -79,14 +79,19 @@ def instance(request, pk):
     console_types = AppSettings.objects.get(
         key="QEMU_CONSOLE_DEFAULT_TYPE"
     ).choices_as_list()
-    console_form = ConsoleForm(
-        initial={
-            "type": instance.console_type,
-            "listen_on": instance.console_listener_address,
-            "password": instance.console_passwd,
-            "keymap": instance.console_keymap,
-        }
-    )
+    # The console form renders the VNC password, so only users who may manage
+    # console settings get the form (and the password) at all.
+    can_manage_console = utils.can_manage_console(request.user, instance)
+    console_form = None
+    if can_manage_console:
+        console_form = ConsoleForm(
+            initial={
+                "type": instance.console_type,
+                "listen_on": instance.console_listener_address,
+                "password": instance.console_passwd,
+                "keymap": instance.console_keymap,
+            }
+        )
     console_listener_addresses = settings.QEMU_CONSOLE_LISTENER_ADDRESSES
     bottom_bar = app_settings.VIEW_INSTANCE_DETAIL_BOTTOM_BAR
     allow_admin_or_not_template = (
@@ -1734,12 +1739,8 @@ def clone(request, pk):
 @serialize_instance_mutation
 def update_console(request, pk):
     instance = get_instance(request.user, pk, perm_type="change")
-    try:
-        userinstance = instance.userinstance_set.get(user=request.user)
-    except Exception:
-        userinstance = UserInstance(is_vnc=False)
 
-    if request.user.is_superuser or userinstance.is_vnc:
+    if utils.can_manage_console(request.user, instance):
         form = ConsoleForm(request.POST or None)
         if form.is_valid():
             if (