From 747cf23966860bad42874665916d974fd2701757 Mon Sep 17 00:00:00 2001 From: "Nicholas H.Tollervey" Date: Mon, 5 Nov 2018 12:03:05 +0000 Subject: [PATCH] Opens downloaded files when double-clicked as per Qubes recipe. Fixes #20. --- securedrop_client/gui/widgets.py | 10 +++++-- securedrop_client/logic.py | 27 +++++++++++++++---- tests/gui/test_widgets.py | 23 +++++++++++++--- tests/test_logic.py | 45 +++++++++++++++++++++++--------- 4 files changed, 82 insertions(+), 23 deletions(-) diff --git a/securedrop_client/gui/widgets.py b/securedrop_client/gui/widgets.py index d0d44a3279..86483ce4de 100644 --- a/securedrop_client/gui/widgets.py +++ b/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): diff --git a/securedrop_client/logic.py b/securedrop_client/logic.py index 59479d98a9..aa66825cdb 100644 --- a/securedrop_client/logic.py +++ b/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. diff --git a/tests/gui/test_widgets.py b/tests/gui/test_widgets.py index a83488c895..750b49ab94 100644 --- a/tests/gui/test_widgets.py +++ b/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(): diff --git a/tests/test_logic.py b/tests/test_logic.py index 71f974c4c1..ae014c3236 100644 --- a/tests/test_logic.py +++ b/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', ])