diff --git a/console/tests/test_views.py b/console/tests/test_views.py index 0f67a6f0..b319caa1 100644 --- a/console/tests/test_views.py +++ b/console/tests/test_views.py @@ -69,7 +69,7 @@ class ConsoleViewsTestCase(TestCase): instance=cls.instance, ) - cls.token = f"{cls.compute.id}-{cls.instance.uuid}" + cls.url = reverse("console", args=[cls.compute.id, cls.instance.uuid]) @patch("console.views.wvmInstance") def test_console_superuser_vnc_lite(self, mock_wvm): @@ -80,7 +80,7 @@ class ConsoleViewsTestCase(TestCase): mock_wvm.return_value = mock_conn self.client.force_login(self.admin_user) - url = reverse("console") + f"?token={self.token}&view=lite" + url = self.url + "?view=lite" response = self.client.get(url) self.assertEqual(response.status_code, 200) @@ -100,7 +100,7 @@ class ConsoleViewsTestCase(TestCase): mock_wvm.return_value = mock_conn self.client.force_login(self.admin_user) - url = reverse("console") + f"?token={self.token}&view=full" + url = self.url + "?view=full" response = self.client.get(url) self.assertEqual(response.status_code, 200) @@ -114,11 +114,11 @@ class ConsoleViewsTestCase(TestCase): self.client.force_login(self.admin_user) for query, pressed in (("", "false"), ("&scale=true", "true")): with self.subTest(query=query): - page = html.fromstring(self.client.get(reverse("console") + f"?token={self.token}&view=lite{query}").content) + page = html.fromstring(self.client.get(self.url + f"?view=lite{query}").content) switch = page.xpath("//*[@id='scale_button']//a")[0] self.assertEqual(switch.get("aria-pressed"), pressed) self.assertEqual("active" in switch.get("class").split(), pressed == "true") - page = html.fromstring(self.client.get(reverse("console") + f"?token={self.token}&view=full").content) + page = html.fromstring(self.client.get(self.url + "?view=full").content) self.assertEqual(page.xpath("//*[@id='scale_button']"), []) # noVNC's own settings scale the full view @patch("console.views.wvmInstance") @@ -130,7 +130,7 @@ class ConsoleViewsTestCase(TestCase): mock_wvm.return_value = mock_conn self.client.force_login(self.admin_user) - url = reverse("console") + f"?token={self.token}&view=full" + url = self.url + "?view=full" response = self.client.get(url) self.assertEqual(response.status_code, 200) @@ -145,7 +145,7 @@ class ConsoleViewsTestCase(TestCase): mock_wvm.return_value = mock_conn self.client.force_login(self.admin_user) - url = reverse("console") + f"?token={self.token}" + url = self.url response = self.client.get(url) self.assertEqual(response.status_code, 200) @@ -159,7 +159,7 @@ class ConsoleViewsTestCase(TestCase): mock_wvm.return_value = mock_conn self.client.force_login(self.admin_user) - url = reverse("console") + f"?token={self.token}" + url = self.url response = self.client.get(url) self.assertEqual(response.status_code, 200) @@ -177,7 +177,7 @@ class ConsoleViewsTestCase(TestCase): mock_wvm.return_value = mock_conn self.client.force_login(self.alice) - url = reverse("console") + f"?token={self.token}" + url = self.url response = self.client.get(url) self.assertEqual(response.status_code, 200) @@ -200,13 +200,13 @@ class ConsoleViewsTestCase(TestCase): for view in ("lite", "full"): with self.subTest(user=user.username, permission=holds, view=view): self.client.force_login(get_user_model().objects.get(pk=user.pk)) # fresh permission cache - response = self.client.get(reverse("console") + f"?token={self.token}&view={view}") + response = self.client.get(self.url + f"?view={view}") self.assertEqual(response.status_code, 200) self.assertEqual("vncpw7f3a" in response.content.decode(), holds) def test_console_unauthorized_user_forbidden(self): self.client.force_login(self.bob) - url = reverse("console") + f"?token={self.token}" + url = self.url response = self.client.get(url) # Non-permitted user attempting to view other user's instance @@ -214,17 +214,18 @@ class ConsoleViewsTestCase(TestCase): self.assertIn("permission", response.content.decode("utf-8").lower()) @patch("console.views.wvmInstance") - def test_console_lite_uses_the_signed_token_not_the_url_token(self, mock_wvm): + def test_console_lite_uses_the_signed_token(self, mock_wvm): mock_conn = MagicMock() mock_conn.get_console_type.return_value = "vnc" mock_conn.get_console_websocket_port.return_value = None mock_wvm.return_value = mock_conn self.client.force_login(self.admin_user) - response = self.client.get(reverse("console") + f"?token={self.token}") + response = self.client.get(self.url) signed = response.context["token"] - self.assertNotEqual(signed, self.token) + self.assertEqual(TimestampSigner(salt="console.novnc").unsign(signed), + f"{self.compute.id}:{self.instance.uuid}:{self.admin_user.id}") self.assertNotContains(response, "readQueryVariable('token'") self.assertContains(response, f"const token = '{escapejs(signed)}'") @@ -233,7 +234,7 @@ class ConsoleViewsTestCase(TestCase): mock_wvm.return_value.get_console_type.return_value = "vnc" mock_wvm.return_value.instance.isActive.return_value = 0 self.client.force_login(self.alice) - response = self.client.get(reverse("console") + f"?token={self.token}") + response = self.client.get(self.url) self.assertContains(response, "The VM is not running") @patch("console.views.wvmInstance") @@ -242,7 +243,7 @@ class ConsoleViewsTestCase(TestCase): mock_wvm.return_value.get_console_type.return_value = "vnc" mock_wvm.return_value.get_console_websocket_port.return_value = None self.client.force_login(self.alice) - response = self.client.get(reverse("console") + f"?token={self.token}&host=attacker.example&port=443&path=x") + response = self.client.get(self.url + "?host=attacker.example&port=443&path=x") self.assertNotContains(response, "attacker.example") for name in ("host", "port", "path"): self.assertNotContains(response, f"readQueryVariable('{name}'") @@ -254,7 +255,7 @@ class ConsoleViewsTestCase(TestCase): mock_wvm.return_value.get_console_type.return_value = "vnc" mock_wvm.return_value.get_console_websocket_port.return_value = None self.client.force_login(self.admin_user) - response = self.client.get(reverse("console") + f"?token={self.token}") + response = self.client.get(self.url) self.assertNotIn("token", response.cookies) @patch("console.views.wvmInstance") @@ -270,7 +271,7 @@ class ConsoleViewsTestCase(TestCase): self.bob.user_permissions.add(perm) self.client.force_login(self.bob) - url = reverse("console") + f"?token={self.token}" + url = self.url response = self.client.get(url) self.assertEqual(response.status_code, 403) @@ -282,7 +283,7 @@ class ConsoleViewsTestCase(TestCase): mock_wvm.return_value.get_console_type.return_value = "vnc" mock_wvm.return_value.console_listens_locally.return_value = True self.client.force_login(self.admin_user) - response = self.client.get(reverse("console") + f"?token={self.token}") + response = self.client.get(self.url) self.assertContains(response, "This console listens only on its host") @patch("console.views.wvmInstance") @@ -292,10 +293,10 @@ class ConsoleViewsTestCase(TestCase): mock_wvm.return_value.get_console_type.return_value = "vnc" mock_wvm.return_value.get_console_websocket_port.return_value = None self.client.force_login(self.admin_user) - lite = self.client.get(reverse("console") + f"?token={self.token}&view=lite") + lite = self.client.get(self.url + "?view=lite") self.assertContains(lite, "const host = window.location.hostname;") self.assertContains(lite, "const path = 'novncd/';") - full = self.client.get(reverse("console") + f"?token={self.token}&view=full") + full = self.client.get(self.url + "?view=full") self.assertContains(full, "host: window.location.hostname,") self.assertContains(full, "encrypt: https,") @@ -309,9 +310,8 @@ class ConsoleViewsTestCase(TestCase): self.client.force_login(self.admin_user) # Attempt XSS injection via query parameters url = ( - reverse("console") - + f"?token={self.token}" - + "&view=lite&view_only=alert('xss')&scale=true&clip_viewport=1" + self.url + + "?view=lite&view_only=alert('xss')&scale=true&clip_viewport=1" ) response = self.client.get(url) @@ -336,7 +336,7 @@ class ConsoleViewsTestCase(TestCase): self.client.force_login(self.admin_user) # Test Lite mode - url_lite = reverse("console") + f"?token={self.token}&view=lite" + url_lite = self.url + "?view=lite" resp_lite = self.client.get(url_lite) self.assertEqual(resp_lite.status_code, 200) content_lite = resp_lite.content.decode("utf-8") @@ -344,7 +344,7 @@ class ConsoleViewsTestCase(TestCase): self.assertNotIn("'", content_lite) # Test Full mode - url_full = reverse("console") + f"?token={self.token}&view=full" + url_full = self.url + "?view=full" resp_full = self.client.get(url_full) self.assertEqual(resp_full.status_code, 200) content_full = resp_full.content.decode("utf-8") @@ -368,7 +368,7 @@ class ConsoleViewsTestCase(TestCase): self.client.force_login(self.admin_user) # Default: scale=False, resize_session=False -> 'off' - url = reverse("console") + f"?token={self.token}&view=full" + url = self.url + "?view=full" resp = self.client.get(url) self.assertEqual(resp.status_code, 200) content = resp.content.decode("utf-8") @@ -377,7 +377,7 @@ class ConsoleViewsTestCase(TestCase): self.assertNotIn("'off'", content) # scale=True, resize_session=False -> 'scale' - url_scale = reverse("console") + f"?token={self.token}&view=full&scale=true" + url_scale = self.url + "?view=full&scale=true" resp_scale = self.client.get(url_scale) self.assertEqual(resp_scale.status_code, 200) content_scale = resp_scale.content.decode("utf-8") @@ -385,7 +385,7 @@ class ConsoleViewsTestCase(TestCase): # resize_session=True -> 'remote' url_remote = ( - reverse("console") + f"?token={self.token}&view=full&resize_session=true" + self.url + "?view=full&resize_session=true" ) resp_remote = self.client.get(url_remote) self.assertEqual(resp_remote.status_code, 200) @@ -397,16 +397,16 @@ class ConsoleViewsTestCase(TestCase): mock_wvm.side_effect = libvirtError("Connection to libvirtd failed") self.client.force_login(self.admin_user) - url = reverse("console") + f"?token={self.token}" + url = self.url response = self.client.get(url) self.assertEqual(response.status_code, 200) self.assertTemplateUsed(response, "console-vnc-lite.html") self.assertIsNotNone(response.context["console_error"]) - def test_console_missing_token_safe_fallback(self): + def test_an_unknown_vm_gives_the_superuser_an_error_page(self): self.client.force_login(self.admin_user) - url = reverse("console") + url = reverse("console", args=[self.compute.id, "no-such-uuid"]) response = self.client.get(url) self.assertEqual(response.status_code, 200) @@ -420,7 +420,7 @@ class ConsoleViewsTestCase(TestCase): mock_wvm.return_value = mock_conn self.client.force_login(self.admin_user) - url = reverse("console") + f"?token={self.token}&view=lite" + url = self.url + "?view=lite" response = self.client.get(url) self.assertEqual(response.status_code, 200) diff --git a/console/views.py b/console/views.py index cb3a1183..93979a69 100644 --- a/console/views.py +++ b/console/views.py @@ -3,7 +3,6 @@ from appsettings.settings import app_settings from console import tokens from django.conf import settings -from django.core.signing import BadSignature, SignatureExpired from django.http.response import HttpResponseForbidden from django.shortcuts import render from django.utils.translation import gettext_lazy as _ @@ -20,67 +19,19 @@ def to_bool(val, default=False): return str(val).strip().lower() in ("true", "1", "yes") -def console(request): - """ - :param request: - :return: - """ +def console(request, compute_id, uuid): + """The console page of the VM with this UUID on this compute. The page's + query gives the view (lite or full) and its display options.""" console_error = None token = "" - view_type = "lite" - view_only = to_bool(app_settings.CONSOLE_VIEW_ONLY, default=False) - scale = to_bool(app_settings.CONSOLE_SCALE, default=False) - resize_session = to_bool(app_settings.CONSOLE_RESIZE_SESSION, default=False) - clip_viewport = to_bool(app_settings.CONSOLE_CLIP_VIEWPORT, default=False) - - if request.method == "GET": - token = request.GET.get("token", token) - view_type = ( - "full" - if str(request.GET.get("view", view_type)).strip().lower() == "full" - else "lite" - ) - if "view_only" in request.GET: - view_only = to_bool(request.GET.get("view_only"), default=view_only) - if "scale" in request.GET: - scale = to_bool(request.GET.get("scale"), default=scale) - if "resize_session" in request.GET: - resize_session = to_bool( - request.GET.get("resize_session"), default=resize_session - ) - if "clip_viewport" in request.GET: - clip_viewport = to_bool( - request.GET.get("clip_viewport"), default=clip_viewport - ) - - signer = tokens.SIGNER - host = None - uuid = None - - if token: - try: - unsigned = signer.unsign(token, max_age=120) - parts = unsigned.split(":") - if len(parts) >= 2: - host = int(parts[0]) - uuid = parts[1] - except (BadSignature, SignatureExpired, ValueError, TypeError): - pass - - if host is None or uuid is None: - try: - temptoken = token.split("-", 1) - host = int(temptoken[0]) - uuid = temptoken[1] - except (ValueError, IndexError): - host = None - uuid = None + view_type = "full" if request.GET.get("view", "").strip().lower() == "full" else "lite" + view_only = to_bool(request.GET.get("view_only"), default=to_bool(app_settings.CONSOLE_VIEW_ONLY)) + scale = to_bool(request.GET.get("scale"), default=to_bool(app_settings.CONSOLE_SCALE)) + resize_session = to_bool(request.GET.get("resize_session"), default=to_bool(app_settings.CONSOLE_RESIZE_SESSION)) + clip_viewport = to_bool(request.GET.get("clip_viewport"), default=to_bool(app_settings.CONSOLE_CLIP_VIEWPORT)) try: - if host is None or uuid is None: - raise ValueError("Invalid token") - - instance = Instance.objects.filter(compute_id=host, uuid=uuid).first() + instance = Instance.objects.filter(compute_id=compute_id, 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 @@ -92,7 +43,7 @@ def console(request): return HttpResponseForbidden(console_error) # Generate secure signed token for noVNC daemon - token = signer.sign(f"{host}:{uuid}:{request.user.id}") + token = tokens.SIGNER.sign(f"{compute_id}:{uuid}:{request.user.id}") conn = wvmInstance( instance.compute.hostname, diff --git a/instances/templates/instance_actions.html b/instances/templates/instance_actions.html index 8ee27d74..36ee94c3 100644 --- a/instances/templates/instance_actions.html +++ b/instances/templates/instance_actions.html @@ -59,16 +59,11 @@ {% endif %} {% if "console" in actions %} - {% else %} {% endif %} - {% endif %} diff --git a/instances/templates/instances/access_tab.html b/instances/templates/instances/access_tab.html index 295ef621..63d04700 100644 --- a/instances/templates/instances/access_tab.html +++ b/instances/templates/instances/access_tab.html @@ -161,8 +161,8 @@ const vo = $("#view_only").is(':checked'); const rs = $("#resize_session").is(':checked'); const cv = $("#clip_viewport").is(':checked'); - url = '{% url 'console' %}?token={{ compute.id }}-{{ instance.get_uuid }}&view=' + view_style + '' + '&view_only=' + vo + '' + '&scale=' + sc + '' + '&clip_viewport=' + cv + '' + '&resize_session=' + rs + ''; - window.open(url, '', 'width=850,height=600') + const options = new URLSearchParams({view: view_style, view_only: vo, scale: sc, clip_viewport: cv, resize_session: rs}); + window.open('{% url 'console' compute.id instance.uuid %}?' + options, '', 'width=850,height=600') } {% endif %} diff --git a/webvirtcloud/urls.py b/webvirtcloud/urls.py index 05d5ba54..5407b726 100644 --- a/webvirtcloud/urls.py +++ b/webvirtcloud/urls.py @@ -26,7 +26,7 @@ urlpatterns = [ path("accounts/", include("accounts.urls")), path("appsettings/", appsettings, name="appsettings"), path("computes/", include("computes.urls")), - path("console/", console, name="console"), + path("console///", console, name="console"), path("datasource/", include("datasource.urls")), path("instances/", include("instances.urls")), path("i18n/", include("django.conf.urls.i18n")),