From 319ab817d1a6e5cb3bda247e8795a3aac8aeb5d9 Mon Sep 17 00:00:00 2001 From: Jelle van der Waa Date: Wed, 4 Sep 2024 10:29:17 +0200 Subject: [PATCH] Re-add frame-src CSP to allow downloading the desktop viewer file This was removed 0dcf8640b04cebcca under the impression it wasn't needed as all integration tests but the tests never validated if the desktop viewer file could be downloaded. Additionally add an integration test which verifies that the file can be downloaded successfully. Hide the iframe to avoid showing a temporary scrollbar which flakes the pixel test. Closes: #1799 --- src/helpers.js | 3 +-- src/manifest.json | 1 + test/check-machines-consoles | 39 +++++++++++++++++++++++++++++------- 3 files changed, 34 insertions(+), 9 deletions(-) diff --git a/src/helpers.js b/src/helpers.js index 0718740ae..53e65e9bb 100644 --- a/src/helpers.js +++ b/src/helpers.js @@ -321,9 +321,8 @@ export function fileDownload({ data, fileName = 'myFile.dat', mimeType = 'applic } else */ { // do iframe dataURL download logDebug('fileDownload() is using IFRAME'); const f = document.createElement('iframe'); - f.width = '1'; - f.height = '1'; document.body.appendChild(f); + f.setAttribute("hidden", "hidden"); const nicerText = '\n[...............................GraphicsConsole]\n'; f.src = `data:${mimeType},${encodeURIComponent(data + nicerText)}`; window.setTimeout(() => document.body.removeChild(f), 333); diff --git a/src/manifest.json b/src/manifest.json index 695e51a53..2cab61693 100644 --- a/src/manifest.json +++ b/src/manifest.json @@ -24,6 +24,7 @@ ] } }, + "content-security-policy": "frame-src 'self' data:", "config": { "StorageMigrationSupported": { "rhel": false diff --git a/test/check-machines-consoles b/test/check-machines-consoles index 48648fe4a..cc728473f 100755 --- a/test/check-machines-consoles +++ b/test/check-machines-consoles @@ -26,6 +26,18 @@ import testlib @testlib.nondestructive class TestMachinesConsoles(machineslib.VirtualMachinesCase): + def waitDownloadFile(self, filename: str, expected_size: int | None = None, content: str | None = None) -> None: + b = self.browser + filepath = b.driver.download_dir / filename + + # Big downloads can take a while + testlib.wait(filepath.exists, tries=120) + if expected_size is not None: + testlib.wait(lambda: filepath.stat().st_size == expected_size) + + if content is not None: + self.assertEqual(filepath.read_text(), content) + @testlib.skipImage('SPICE not supported on RHEL', "rhel-*", "centos-*") def testExternalConsole(self): b = self.browser @@ -43,14 +55,27 @@ class TestMachinesConsoles(machineslib.VirtualMachinesCase): b.wait_in_text(".pf-v5-c-console__manual-connection dl > div:first-child dd", "127.0.0.1") b.wait_in_text(".pf-v5-c-console__manual-connection dl > div:nth-child(2) dd", "5900") + b.allow_download() b.click(".pf-v5-c-console__remote-viewer-launch-vv") # "Launch Remote Viewer" button - b.wait_visible("#dynamically-generated-file") # is .vv file generated for download? - self.assertEqual(b.attr("#dynamically-generated-file", "href"), - u"data:application/x-virt-viewer,%5Bvirt-viewer%5D%0Atype%3Dspice%0Ahost%3D127.0.0.1%0Aport%3D5900%0Adelete-this-file%3D1%0Afullscreen%3D0%0A") - - # HACK: clicking 'Launch Remote Viewer' kills execution context and thus CDP fails - b.reload() - b.enter_page("/machines") + content = """[virt-viewer] +type=spice +host=127.0.0.1 +port=5900 +delete-this-file=1 +fullscreen=0 + +[...............................GraphicsConsole] +""" + # HACK: Due to a bug in cockpit-machines, Firefox downloads the desktop + # viewer file as a randomly generated file while chromium as + # "download". As we want to download this as "$vmName.vv" in the end + # work around the issue for now. + if b.browser == "chromium": + self.waitDownloadFile("download", content=content) + else: + b.wait_visible("#dynamically-generated-file") # is .vv file generated for download? + self.assertEqual(b.attr("#dynamically-generated-file", "href"), + u"data:application/x-virt-viewer,%5Bvirt-viewer%5D%0Atype%3Dspice%0Ahost%3D127.0.0.1%0Aport%3D5900%0Adelete-this-file%3D1%0Afullscreen%3D0%0A") # Go to the expanded console view b.click("button:contains(Expand)")