Opens downloaded files when double-clicked as per Qubes recipe. Fixes #…

…20.

securedrop_client/gui/widgets.py

@@ -466,9 +466,15 @@ def __init__(self, source_db_object, submission_db_object,
 
     def mouseDoubleClickEvent(self, e):
         """
-        Handle a double-click via the program logic.
+        Handle a double-click via the program logic. The download state of the
+        file distinguishes which function in the logic layet to call.
         """
-        self.controller.on_file_click(self.source, self.submission)
+        if self.submission.is_downloaded:
+            # Open the already downloaded file.
+            self.controller.on_file_open(self.submission)
+        else:
+            # Download the file.
+            self.controller.on_file_download(self.source, self.submission)
 
 
 class ConversationView(QWidget):

securedrop_client/logic.py

@@ -29,10 +29,9 @@
 from securedrop_client import models
 from securedrop_client.utils import check_dir_permissions
 from securedrop_client.data import Data
-from PyQt5.QtCore import QObject, QThread, pyqtSignal, QTimer
+from PyQt5.QtCore import QObject, QThread, pyqtSignal, QTimer, QProcess
 from securedrop_client.message_sync import MessageSync
 
-
 logger = logging.getLogger(__name__)
 
 
@@ -442,7 +441,25 @@ def set_status(self, message, duration=5000):
         """
         self.gui.set_status(message, duration)
 
-    def on_file_click(self, source_db_object, message):
+    def on_file_open(self, source_db_object):
+        """
+        Open the already doanloaded file associated with the message (which
+        may be a Submission or a Repl).
+        """
+        if self.proxy:
+            # Running on Qubes.
+            command = "qvm-open-in-vm"
+            args = ["'$dispvm:sd-svs-disp'", source_db_object.filename]
+            # QProcess (Qt) or Python's subprocess? Who cares? They do the
+            # same thing. :-)
+            process = QProcess(self)
+            process.start(command, args)
+            # TODO: Set marked as read?
+        else:  # pragma: no cover
+            # Non Qubes OS. Just log the event for now.
+            logger.info('Opening file "{}".'.format(message.filename))
+
+    def on_file_download(self, source_db_object, message):
         """
         Download the file associated with the associated message (which may
         be a Submission or Reply).
@@ -460,11 +477,11 @@ def on_file_click(self, source_db_object, message):
             sdk_object.filename = message.filename
             sdk_object.source_uuid = source_db_object.uuid
         self.set_status(_('Downloading {}'.format(sdk_object.filename)))
-        self.call_api(func, self.on_file_download,
+        self.call_api(func, self.on_file_downloaded,
                       self.on_download_timeout, sdk_object, self.data_dir,
                       current_object=message)
 
-    def on_file_download(self, result, current_object):
+    def on_file_downloaded(self, result, current_object):
         """
         Called when a file has downloaded. Cause a refresh to the conversation
         view to display the contents of the new file.

tests/gui/test_widgets.py

@@ -413,9 +413,26 @@ def test_FileWidget_init_right():
     assert fw.controller == mock_controller
 
 
-def test_FileWidget_mouseDoubleClickEvent():
+def test_FileWidget_mouseDoubleClickEvent_download():
     """
-    Should fire the expected event handler in the logic layer.
+    Should fire the expected download event handler in the logic layer.
+    """
+    mock_message = mock.MagicMock()
+    mock_controller = mock.MagicMock()
+    source = models.Source('source-uuid', 'testy-mctestface', False,
+                           'mah pub key', 1, False, datetime.now())
+    submission = models.Submission(source, 'submission-uuid', 123,
+                                   'mah-reply.gpg')
+    submission.is_downloaded = False
+
+    fw = FileWidget(source, submission, mock_controller)
+    fw.mouseDoubleClickEvent(None)
+    fw.controller.on_file_download.assert_called_once_with(source, submission)
+
+
+def test_FileWidget_mouseDoubleClickEvent_open():
+    """
+    Should fire the expected open event handler in the logic layer.
     """
     mock_message = mock.MagicMock()
     mock_controller = mock.MagicMock()
@@ -428,7 +445,7 @@ def test_FileWidget_mouseDoubleClickEvent():
 
     fw = FileWidget(source, submission, mock_controller)
     fw.mouseDoubleClickEvent(None)
-    fw.controller.on_file_click.assert_called_once_with(source, submission)
+    fw.controller.on_file_open.assert_called_once_with(submission)
 
 
 def test_ConversationView_init():

tests/test_logic.py

@@ -694,7 +694,7 @@ def func() -> None:
                 func()
 
 
-def test_Client_on_file_click_Submission(safe_tmpdir):
+def test_Client_on_file_download_Submission(safe_tmpdir):
     """
     If the handler is passed a submission, check the download_submission
     function is the one called against the API.
@@ -711,14 +711,14 @@ def test_Client_on_file_click_Submission(safe_tmpdir):
     submission_sdk_object = mock.MagicMock()
     with mock.patch('sdclientapi.Submission') as mock_submission:
         mock_submission.return_value = submission_sdk_object
-        cl.on_file_click(source, submission)
+        cl.on_file_download(source, submission)
     cl.call_api.assert_called_once_with(
-        cl.api.download_submission, cl.on_file_download,
+        cl.api.download_submission, cl.on_file_downloaded,
         cl.on_download_timeout, submission_sdk_object,
         cl.data_dir, current_object=submission)
 
 
-def test_Client_on_file_download_success(safe_tmpdir):
+def test_Client_on_file_downloaded_success(safe_tmpdir):
     mock_gui = mock.MagicMock()
     mock_session = mock.MagicMock()
     cl = Client('http://localhost', mock_gui, mock_session, str(safe_tmpdir))
@@ -733,11 +733,12 @@ def test_Client_on_file_download_success(safe_tmpdir):
     submission_db_object.filename = test_filename
     with mock.patch('securedrop_client.logic.storage') as mock_storage, \
             mock.patch('shutil.move'):
-        cl.on_file_download(result_data, current_object=submission_db_object)
+        cl.on_file_downloaded(result_data, current_object=submission_db_object)
         mock_storage.mark_file_as_downloaded.assert_called_once_with(
             test_object_uuid, mock_session)
 
-def test_Client_on_file_download_failure(safe_tmpdir):
+
+def test_Client_on_file_downloaded_failure(safe_tmpdir):
     mock_gui = mock.MagicMock()
     mock_session = mock.MagicMock()
     cl = Client('http://localhost', mock_gui, mock_session, str(safe_tmpdir))
@@ -751,7 +752,7 @@ def test_Client_on_file_download_failure(safe_tmpdir):
     submission_db_object = mock.MagicMock()
     submission_db_object.uuid = 'myuuid'
     submission_db_object.filename = 'filename'
-    cl.on_file_download(result_data, current_object=submission_db_object)
+    cl.on_file_downloaded(result_data, current_object=submission_db_object)
     cl.set_status.assert_called_once_with(
         "The file download failed. Please try again.")
 
@@ -772,10 +773,7 @@ def test_Client_on_download_timeout(safe_tmpdir):
         "The connection to the SecureDrop server timed out. Please try again.")
 
 
-# This can be unfailed when this SDK change is merged and released:
-# https://github.com/freedomofpress/securedrop-sdk/pull/32
-@pytest.mark.xfail(reason="needs SDK change merged")
-def test_Client_on_file_click_Reply(safe_tmpdir):
+def test_Client_on_file_download_Reply(safe_tmpdir):
     """
     If the handler is passed a reply, check the download_reply
     function is the one called against the API.
@@ -793,9 +791,30 @@ def test_Client_on_file_click_Reply(safe_tmpdir):
     reply_sdk_object = mock.MagicMock()
     with mock.patch('sdclientapi.Reply') as mock_reply:
         mock_reply.return_value = reply_sdk_object
-        cl.on_file_click(source, reply)
+        cl.on_file_download(source, reply)
     cl.call_api.assert_called_once_with(cl.api.download_reply,
-                                        cl.on_file_download,
+                                        cl.on_file_downloaded,
                                         cl.on_download_timeout,
                                         reply_sdk_object,
                                         cl.data_dir, current_object=reply)
+
+
+def test_Client_on_file_open(safe_tmpdir):
+    """
+    If running on Qubes, a new QProcess with the expected command and args
+    should be started.
+    """
+    mock_gui = mock.MagicMock()
+    mock_session = mock.MagicMock()
+    cl = Client('http://localhost', mock_gui, mock_session, str(safe_tmpdir))
+    cl.proxy = True
+    mock_submission = mock.MagicMock()
+    mock_submission.filename = 'test.pdf'
+    mock_subprocess = mock.MagicMock()
+    mock_process = mock.MagicMock(return_value=mock_subprocess)
+    with mock.patch('securedrop_client.logic.QProcess', mock_process):
+        cl.on_file_open(mock_submission)
+        mock_process.assert_called_once_with(cl)
+        mock_subprocess.start.assert_called_once_with("qvm-open-in-vm",
+                                                      ["'$dispvm:sd-svs-disp'",
+                                                       'test.pdf', ])