From 46c085e2720fdb733d155a761ce44b10f1fd675f Mon Sep 17 00:00:00 2001 From: Ro Date: Fri, 26 Jan 2024 12:25:47 -0500 Subject: [PATCH] Export and print use single method signature across components. Device does not depend on filepaths. --- client/securedrop_client/gui/actions.py | 12 +- .../gui/conversation/export/device.py | 175 +++++++----------- .../gui/conversation/export/dialog.py | 8 +- .../gui/conversation/export/file_dialog.py | 8 +- .../gui/conversation/export/print_dialog.py | 8 +- .../export/print_transcript_dialog.py | 12 +- .../conversation/export/transcript_dialog.py | 10 +- client/securedrop_client/gui/widgets.py | 9 +- client/tests/conftest.py | 18 +- .../gui/conversation/export/test_device.py | 133 ++----------- .../gui/conversation/export/test_dialog.py | 2 +- .../conversation/export/test_file_dialog.py | 8 +- .../conversation/export/test_print_dialog.py | 4 +- .../export/test_transcript_dialog.py | 4 +- client/tests/gui/test_actions.py | 4 +- client/tests/gui/test_widgets.py | 8 +- client/tests/integration/conftest.py | 12 +- 17 files changed, 153 insertions(+), 282 deletions(-) diff --git a/client/securedrop_client/gui/actions.py b/client/securedrop_client/gui/actions.py index dc822ff02d..4cebcc4d11 100644 --- a/client/securedrop_client/gui/actions.py +++ b/client/securedrop_client/gui/actions.py @@ -187,8 +187,10 @@ def _on_triggered(self) -> None: # out of scope, any pending file removal will be performed # by the operating system. with open(file_path, "r") as f: - export = ExportDevice([file_path]) - dialog = PrintConversationTranscriptDialog(export, TRANSCRIPT_FILENAME, str(file_path)) + export = ExportDevice() + dialog = PrintConversationTranscriptDialog( + export, TRANSCRIPT_FILENAME, [str(file_path)] + ) dialog.exec() @@ -236,9 +238,9 @@ def _on_triggered(self) -> None: # out of scope, any pending file removal will be performed # by the operating system. with open(file_path, "r") as f: - export_device = ExportDevice([file_path]) + export_device = ExportDevice() dialog = ExportConversationTranscriptDialog( - export_device, TRANSCRIPT_FILENAME, str(file_path) + export_device, TRANSCRIPT_FILENAME, [str(file_path)] ) dialog.exec() @@ -325,7 +327,7 @@ def _prepare_to_export(self) -> None: # out of scope, any pending file removal will be performed # by the operating system. with ExitStack() as stack: - export_device = ExportDevice(file_locations) + export_device = ExportDevice() files = [ stack.enter_context(open(file_location, "r")) for file_location in file_locations ] diff --git a/client/securedrop_client/gui/conversation/export/device.py b/client/securedrop_client/gui/conversation/export/device.py index bae29311ee..0879ad0ca6 100644 --- a/client/securedrop_client/gui/conversation/export/device.py +++ b/client/securedrop_client/gui/conversation/export/device.py @@ -6,7 +6,7 @@ from io import BytesIO from shlex import quote from tempfile import TemporaryDirectory -from typing import List +from typing import List, Optional from PyQt5.QtCore import QObject, pyqtSignal @@ -48,6 +48,9 @@ class Device(QObject): print_preflight_check_succeeded = pyqtSignal(object) print_succeeded = pyqtSignal(object) + # Used for both print and export + export_completed = pyqtSignal(object) + # Emit ExportError(status=ExportStatus) export_preflight_check_failed = pyqtSignal(object) export_failed = pyqtSignal(object) @@ -55,21 +58,20 @@ class Device(QObject): print_preflight_check_failed = pyqtSignal(object) print_failed = pyqtSignal(object) - def __init__(self, filepaths: [str]) -> None: - super().__init__() - - self._filepaths_list = filepaths - def run_printer_preflight_checks(self) -> None: """ Make sure the Export VM is started. """ - logger.info("Running printer preflight check") + logger.info("Beginning printer preflight check") try: - status = self._build_archive_and_export( - metadata=self._PRINTER_PREFLIGHT_METADATA, filename=self._PRINTER_PREFLIGHT_FN - ) - self.print_preflight_check_succeeded.emit(status) + with TemporaryDirectory() as tmp_dir: + archive_path = self._create_archive( + archive_dir=tmp_dir, + archive_fn=self._PRINTER_PREFLIGHT_FN, + metadata=self._PRINTER_PREFLIGHT_METADATA, + ) + status = self._run_qrexec_export(archive_path) + self.print_preflight_check_succeeded.emit(status) except ExportError as e: logger.error("Print preflight failed") logger.debug(f"Print preflight failed: {e}") @@ -81,50 +83,77 @@ def run_export_preflight_checks(self) -> None: """ try: logger.debug("Beginning export preflight check") - status = self._build_archive_and_export( - metadata=self._USB_TEST_METADATA, filename=self._USB_TEST_FN - ) - self.export_preflight_check_succeeded.emit(status) + + with TemporaryDirectory() as tmp_dir: + archive_path = self._create_archive( + archive_dir=tmp_dir, + archive_fn=self._USB_TEST_FN, + metadata=self._USB_TEST_METADATA, + ) + status = self._run_qrexec_export(archive_path) + self.export_preflight_check_succeeded.emit(status) + except ExportError as e: logger.error("Export preflight failed") self.export_preflight_check_failed.emit(e) - def export_transcript(self, file_location: str, passphrase: str) -> None: + def export(self, filepaths: List[str], passphrase: Optional[str]) -> None: """ - Send the transcript specified by file_location to the Export VM. + Bundle filepaths into a tarball and send to encrypted USB via qrexec, + optionally supplying a passphrase to unlock encrypted drives. """ - logger.debug("Export transcript") - self._send_file_to_usb_device([file_location], passphrase) + try: + logger.debug(f"Begin exporting {len(filepaths)} item(s)") - def export_files(self, file_locations: List[str], passphrase: str) -> None: - """ - Send the files specified by file_locations to the Export VM. - """ - logger.debug(f"Export {len(file_locations)} files") - self._send_file_to_usb_device(file_locations, passphrase) + # Edit metadata template to include passphrase + metadata = self._DISK_METADATA.copy() + if passphrase: + metadata[self._DISK_ENCRYPTION_KEY_NAME] = passphrase - def export_file_to_usb_drive(self, file_uuid: str, passphrase: str) -> None: - """ - Send the file specified by file_uuid to the Export VM with the user-provided passphrase for - unlocking the attached transfer device. If the file is missing, update the db so that - is_downloaded is set to False. - """ + with TemporaryDirectory() as tmp_dir: + archive_path = self._create_archive( + archive_dir=tmp_dir, + archive_fn=self._DISK_FN, + metadata=metadata, + filepaths=filepaths, + ) + status = self._run_qrexec_export(archive_path) - self._send_file_to_usb_device(self._filepaths_list, passphrase) + self.export_succeeded.emit(status) + logger.debug(f"Status {status}") - def print_transcript(self, file_location: str) -> None: - """ - Send the transcript specified by file_location to the Export VM. - """ - self._print([file_location]) + except ExportError as e: + logger.error("Export failed") + logger.debug(f"Export failed: {e}") + self.export_failed.emit(e) + + self.export_completed.emit(filepaths) - def print_file(self, file_uuid: str) -> None: + def print(self, filepaths: List[str]) -> None: """ - Send the file specified by file_uuid to the Export VM. If the file is missing, update the db - so that is_downloaded is set to False. + Bundle files at self._filepaths_list into tarball and send for + printing via qrexec. """ + try: + logger.debug("Beginning print") + + with TemporaryDirectory() as tmp_dir: + archive_path = self._create_archive( + archive_dir=tmp_dir, + archive_fn=self._PRINT_FN, + metadata=self._PRINT_METADATA, + filepaths=filepaths, + ) + status = self._run_qrexec_export(archive_path) + self.print_succeeded.emit(status) + logger.debug(f"Status {status}") - self._print(self._filepaths_list) + except ExportError as e: + logger.error("Export failed") + logger.debug(f"Export failed: {e}") + self.print_failed.emit(e) + + self.export_completed.emit(filepaths) def _run_qrexec_export(self, archive_path: str) -> ExportStatus: """ @@ -174,7 +203,7 @@ def _run_qrexec_export(self, archive_path: str) -> ExportStatus: raise ExportError(ExportStatus.CALLED_PROCESS_ERROR) def _create_archive( - self, archive_dir: str, archive_fn: str, metadata: dict, filepaths: List[str] + self, archive_dir: str, archive_fn: str, metadata: dict, filepaths: List[str] = [] ) -> str: """ Create the archive to be sent to the Export VM. @@ -210,7 +239,7 @@ def _create_archive( self._add_file_to_archive( archive, filepath, prevent_name_collisions=is_one_of_multiple_files ) - if missing_count == len(filepaths): + if missing_count == len(filepaths) and missing_count > 0: # Context manager will delete archive even if an exception occurs # since the archive is in a TemporaryDirectory logger.error("Files were moved or missing") @@ -258,63 +287,3 @@ def _add_file_to_archive( arcname = os.path.join("export_data", parent_name, filename) archive.add(filepath, arcname=arcname, recursive=False) - - def _build_archive_and_export( - self, metadata: dict, filename: str, filepaths: List[str] = [] - ) -> ExportStatus: - """ - Build archive, run qrexec command and return resulting ExportStatus. - - ExportError may be raised during underlying _run_qrexec_export call, - and is handled by the calling method. - """ - with TemporaryDirectory() as tmp_dir: - archive_path = self._create_archive( - archive_dir=tmp_dir, archive_fn=filename, metadata=metadata, filepaths=filepaths - ) - return self._run_qrexec_export(archive_path) - - def _send_file_to_usb_device(self, filepaths: List[str], passphrase: str) -> None: - """ - Export the file to the luks-encrypted usb disk drive attached to the Export VM. - - Args: - filepath: The path of file to export. - passphrase: The passphrase to unlock the luks-encrypted usb disk drive. - """ - try: - logger.debug("beginning export") - # Edit metadata template to include passphrase - metadata = self._DISK_METADATA.copy() - metadata[self._DISK_ENCRYPTION_KEY_NAME] = passphrase - status = self._build_archive_and_export( - metadata=metadata, filename=self._DISK_FN, filepaths=filepaths - ) - - self.export_succeeded.emit(status) - logger.debug(f"Status {status}") - except ExportError as e: - logger.error("Export failed") - logger.debug(f"Export failed: {e}") - self.export_failed.emit(e) - - def _print(self, filepaths: List[str]) -> None: - """ - Print the file to the printer attached to the Export VM. - - Args: - filepath: The path of file to export. - """ - try: - logger.debug("beginning print") - status = self._build_archive_and_export( - metadata=self._PRINT_METADATA, filename=self._PRINT_FN, filepaths=filepaths - ) - self.print_succeeded.emit(status) - logger.debug(f"Status {status}") - except ExportError as e: - logger.error("Export failed") - logger.debug(f"Export failed: {e}") - self.print_failed.emit(e) - - self.export_succeeded.emit(filepaths) diff --git a/client/securedrop_client/gui/conversation/export/dialog.py b/client/securedrop_client/gui/conversation/export/dialog.py index c71ebe2d84..6b184bf994 100644 --- a/client/securedrop_client/gui/conversation/export/dialog.py +++ b/client/securedrop_client/gui/conversation/export/dialog.py @@ -15,17 +15,17 @@ class Dialog(FileDialog): - Overrides the two slots that handles the export action to call said method. """ - def __init__(self, device: Device, summary: str, file_locations: List[str]) -> None: - super().__init__(device, "", summary) + def __init__(self, device: Device, summary: str, filepaths: List[str]) -> None: + super().__init__(device, summary, filepaths) - self.file_locations = file_locations + self.filepaths = filepaths @pyqtSlot(bool) def _export_files(self, checked: bool = False) -> None: self.start_animate_activestate() self.cancel_button.setEnabled(False) self.passphrase_field.setDisabled(True) - self._device.export_files(self.file_locations, self.passphrase_field.text()) + self._device.export(self.filepaths, self.passphrase_field.text()) @pyqtSlot() def _show_passphrase_request_message(self) -> None: diff --git a/client/securedrop_client/gui/conversation/export/file_dialog.py b/client/securedrop_client/gui/conversation/export/file_dialog.py index 931ef2fa77..bc9c2f43aa 100644 --- a/client/securedrop_client/gui/conversation/export/file_dialog.py +++ b/client/securedrop_client/gui/conversation/export/file_dialog.py @@ -2,7 +2,7 @@ A dialog that allows journalists to export sensitive files to a USB drive. """ from gettext import gettext as _ -from typing import Optional +from typing import List, Optional from pkg_resources import resource_string from PyQt5.QtCore import QSize, Qt, pyqtSlot @@ -23,12 +23,12 @@ class FileDialog(ModalDialog): NO_MARGIN = 0 FILENAME_WIDTH_PX = 260 - def __init__(self, device: Device, file_uuid: str, file_name: str) -> None: + def __init__(self, device: Device, file_name: str, filepaths: List[str]) -> None: super().__init__() self.setStyleSheet(self.DIALOG_CSS) self._device = device - self.file_uuid = file_uuid + self.filepaths = filepaths self.file_name = SecureQLabel( file_name, wordwrap=False, max_length=self.FILENAME_WIDTH_PX ).text() @@ -215,7 +215,7 @@ def _export_file(self, checked: bool = False) -> None: # TODO: If the drive is already unlocked, the passphrase field will be empty. # This is ok, but could violate expectations. The password should be passed # via qrexec in future, to avoid writing it to even a temporary file at all. - self._device.export_file_to_usb_drive(self.file_uuid, self.passphrase_field.text()) + self._device.export(self.filepaths, self.passphrase_field.text()) @pyqtSlot(object) def _on_export_preflight_check_succeeded(self, result: ExportStatus) -> None: diff --git a/client/securedrop_client/gui/conversation/export/print_dialog.py b/client/securedrop_client/gui/conversation/export/print_dialog.py index a4d74ed423..889a19c0d7 100644 --- a/client/securedrop_client/gui/conversation/export/print_dialog.py +++ b/client/securedrop_client/gui/conversation/export/print_dialog.py @@ -1,5 +1,5 @@ from gettext import gettext as _ -from typing import Optional +from typing import List, Optional from PyQt5.QtCore import QSize, pyqtSlot @@ -12,11 +12,11 @@ class PrintDialog(ModalDialog): FILENAME_WIDTH_PX = 260 - def __init__(self, device: Device, file_uuid: str, file_name: str) -> None: + def __init__(self, device: Device, file_name: str, filepaths: List[str]) -> None: super().__init__() self._device = device - self.file_uuid = file_uuid + self.filepaths = filepaths self.file_name = SecureQLabel( file_name, wordwrap=False, max_length=self.FILENAME_WIDTH_PX ).text() @@ -94,7 +94,7 @@ def _run_preflight(self) -> None: @pyqtSlot() def _print_file(self) -> None: - self._device.print_file(self.file_uuid) + self._device.print(self.filepaths) self.close() @pyqtSlot() diff --git a/client/securedrop_client/gui/conversation/export/print_transcript_dialog.py b/client/securedrop_client/gui/conversation/export/print_transcript_dialog.py index 9f47735ce3..4eeb4d52cd 100644 --- a/client/securedrop_client/gui/conversation/export/print_transcript_dialog.py +++ b/client/securedrop_client/gui/conversation/export/print_transcript_dialog.py @@ -1,3 +1,5 @@ +from typing import List + from PyQt5.QtCore import QSize, pyqtSlot from securedrop_client.gui.conversation.export import PrintDialog @@ -13,13 +15,15 @@ class PrintTranscriptDialog(PrintDialog): - Overrides the slot that handles the printing action to call said method. """ - def __init__(self, device: Device, file_name: str, transcript_location: str) -> None: - super().__init__(device, "", file_name) + def __init__(self, device: Device, file_name: str, filepath: List[str]) -> None: + super().__init__(device, file_name, filepath) - self.transcript_location = transcript_location + # List might seem like an odd choice for this, but this is on the + # way to standardizing one export/print dialog that can send multiple items + self.transcript_location = filepath def _print_transcript(self) -> None: - self._device.print_transcript(self.transcript_location) + self._device.print(self.transcript_location) self.close() @pyqtSlot() diff --git a/client/securedrop_client/gui/conversation/export/transcript_dialog.py b/client/securedrop_client/gui/conversation/export/transcript_dialog.py index 3318197076..e1f573dabe 100644 --- a/client/securedrop_client/gui/conversation/export/transcript_dialog.py +++ b/client/securedrop_client/gui/conversation/export/transcript_dialog.py @@ -2,6 +2,7 @@ A dialog that allows journalists to export sensitive files to a USB drive. """ from gettext import gettext as _ +from typing import List from PyQt5.QtCore import pyqtSlot @@ -17,16 +18,17 @@ class TranscriptDialog(FileDialog): - Overrides the two slots that handles the export action to call said method. """ - def __init__(self, device: Device, file_name: str, transcript_location: str) -> None: - super().__init__(device, "", file_name) + def __init__(self, device: Device, file_name: str, filepath: List[str]) -> None: + super().__init__(device, file_name, filepath) - self.transcript_location = transcript_location + # List[str] to foreshadow multifile export and combining all export dialogs + self.transcript_location = filepath def _export_transcript(self, checked: bool = False) -> None: self.start_animate_activestate() self.cancel_button.setEnabled(False) self.passphrase_field.setDisabled(True) - self._device.export_transcript(self.transcript_location, self.passphrase_field.text()) + self._device.export(self.transcript_location, self.passphrase_field.text()) @pyqtSlot() def _show_passphrase_request_message(self) -> None: diff --git a/client/securedrop_client/gui/widgets.py b/client/securedrop_client/gui/widgets.py index a4930ac7ff..7330b2ffe1 100644 --- a/client/securedrop_client/gui/widgets.py +++ b/client/securedrop_client/gui/widgets.py @@ -2459,10 +2459,10 @@ def _on_export_clicked(self) -> None: logger.debug("Clicked export but file not downloaded") return - export_device = conversation.ExportDevice([file_location]) + export_device = conversation.ExportDevice() self.export_dialog = conversation.ExportFileDialog( - export_device, self.uuid, self.file.filename + export_device, self.file.filename, [file_location] ) self.export_dialog.show() @@ -2476,9 +2476,10 @@ def _on_print_clicked(self) -> None: return filepath = self.file.location(self.controller.data_dir) - export_device = conversation.ExportDevice([filepath]) - dialog = conversation.PrintFileDialog(export_device, self.uuid, self.file.filename) + export_device = conversation.ExportDevice() + + dialog = conversation.PrintFileDialog(export_device, self.file.filename, [filepath]) dialog.exec() def _on_left_click(self) -> None: diff --git a/client/tests/conftest.py b/client/tests/conftest.py index d3c45c6e46..a5935b943f 100644 --- a/client/tests/conftest.py +++ b/client/tests/conftest.py @@ -78,7 +78,7 @@ def print_dialog(mocker, homedir): export_device = mocker.MagicMock(spec=conversation.ExportDevice) - dialog = conversation.PrintFileDialog(export_device, "file_UUID", "file123.jpg") + dialog = conversation.PrintFileDialog(export_device, "file123.jpg", ["/mock/path/to/file"]) yield dialog @@ -90,7 +90,7 @@ def print_transcript_dialog(mocker, homedir): export_device = mocker.MagicMock(spec=conversation.ExportDevice) dialog = conversation.PrintTranscriptDialog( - export_device, "transcript.txt", "some/path/transcript.txt" + export_device, "transcript.txt", ["some/path/transcript.txt"] ) yield dialog @@ -117,7 +117,7 @@ def export_file_dialog(mocker, homedir): export_device = mocker.MagicMock(spec=conversation.ExportDevice) - dialog = conversation.ExportFileDialog(export_device, "file_UUID", "file123.jpg") + dialog = conversation.ExportFileDialog(export_device, "file123.jpg", ["/mock/path/to/file"]) yield dialog @@ -129,7 +129,7 @@ def export_transcript_dialog(mocker, homedir): export_device = mocker.MagicMock(spec=conversation.ExportDevice) dialog = conversation.ExportTranscriptDialog( - export_device, "transcript.txt", "/some/path/transcript.txt" + export_device, "transcript.txt", ["/some/path/transcript.txt"] ) yield dialog @@ -169,16 +169,12 @@ def homedir(i18n): @pytest.fixture(scope="function") def mock_export(): - device = conversation.ExportDevice(["/mock/file/path"]) + device = conversation.ExportDevice() device.run_export_preflight_checks = lambda dir: None device.run_printer_preflight_checks = lambda dir: None - device.export_files = lambda dir, paths, passphrase: None - device.export_transcript = lambda dir, paths, passphrase: None - device.print_file = lambda uuid: None - device.print_transcript = lambda file: None - device.export_file_to_usb_drive = lambda uuid, passphrase: None - device.export_files = lambda filepaths, passphrase: None + device.print = lambda filepaths: None + device.export = lambda filepaths, passphrase: None return device diff --git a/client/tests/gui/conversation/export/test_device.py b/client/tests/gui/conversation/export/test_device.py index e3c01e9eab..a3f97cbadf 100644 --- a/client/tests/gui/conversation/export/test_device.py +++ b/client/tests/gui/conversation/export/test_device.py @@ -35,7 +35,7 @@ def setup_class(cls): def setup_method(cls): cls.mock_file = factory.File(source=factory.Source()) cls.mock_file_location = f"{_MOCK_FILEDIR}{cls.mock_file.filename}" - cls.device = Device([cls.mock_file_location]) + cls.device = Device() cls.device._create_archive = mock.MagicMock() cls.device._create_archive.return_value = _PATH_TO_PRETEND_ARCHIVE cls.mock_tmpdir = mock.MagicMock() @@ -47,7 +47,7 @@ def teardown_method(cls): cls.device._create_archive = None def test_Device_run_printer_preflight_checks(self, mock_subprocess): - device = Device(["/fake/file/name"]) + device = Device() device._create_archive = mock.MagicMock() device._create_archive.return_value = _PATH_TO_PRETEND_ARCHIVE @@ -70,30 +70,12 @@ def test_Device_run_print_preflight_checks_with_error(self, mock_sp): assert "Print preflight failed" in err.call_args[0] - def test_Device_run_print_file(self, mock_subprocess): + def test_Device_print(self, mock_subprocess): with mock.patch( "securedrop_client.gui.conversation.export.device.TemporaryDirectory", return_value=self.mock_tmpdir, ): - self.device.print_file(self.mock_file.uuid) - - self.device._create_archive.assert_called_once_with( - archive_dir=_MOCK_FILEDIR, - archive_fn=self.device._PRINT_FN, - metadata=self.device._PRINT_METADATA, - filepaths=[self.mock_file_location], - ) - mock_subprocess.assert_called_once() - assert _QREXEC_EXPORT_COMMAND in mock_subprocess.call_args[0] - - def test_Device_print_transcript(self, mock_subprocess): - with mock.patch( - "securedrop_client.gui.conversation.export.device.TemporaryDirectory", - return_value=self.mock_tmpdir, - ): - self.device.print_transcript(self.mock_file_location) - - mock_subprocess.assert_called_once() + self.device.print([self.mock_file_location]) self.device._create_archive.assert_called_once_with( archive_dir=_MOCK_FILEDIR, @@ -105,34 +87,18 @@ def test_Device_print_transcript(self, mock_subprocess): assert _QREXEC_EXPORT_COMMAND in mock_subprocess.call_args[0] def test_Device_print_file_file_missing(self, mock_subprocess, mocker): - device = Device(["/no/such/file"]) + device = Device() warning_logger = mocker.patch( "securedrop_client.gui.conversation.export.device.logger.warning" ) log_msg = "File not found at specified filepath, skipping" - device.print_file("some-missing-file-uuid") + device.print("some-missing-file-uuid") assert log_msg in warning_logger.call_args[0] mock_subprocess.assert_not_called() - def test_Device_print_file_when_orig_file_already_exists(self, mock_subprocess): - with mock.patch( - "securedrop_client.gui.conversation.export.device.TemporaryDirectory", - return_value=self.mock_tmpdir, - ): - self.device.print_file(self.mock_file.uuid) - - mock_subprocess.assert_called_once() - self.device._create_archive.assert_called_once_with( - archive_dir=_MOCK_FILEDIR, - archive_fn=self.device._PRINT_FN, - metadata=self.device._PRINT_METADATA, - filepaths=[self.mock_file_location], - ) - assert _QREXEC_EXPORT_COMMAND in mock_subprocess.call_args[0] - def test_Device_run_export_preflight_checks(self, mock_subprocess): with mock.patch( "securedrop_client.gui.conversation.export.device.TemporaryDirectory", @@ -145,7 +111,6 @@ def test_Device_run_export_preflight_checks(self, mock_subprocess): archive_dir=_MOCK_FILEDIR, archive_fn=self.device._USB_TEST_FN, metadata=self.device._USB_TEST_METADATA, - filepaths=[], ) mock_subprocess.assert_called_once() assert _QREXEC_EXPORT_COMMAND in mock_subprocess.call_args[0] @@ -158,30 +123,8 @@ def test_Device_run_export_preflight_checks_with_error(self, mock_sp): assert "Export preflight failed" in err.call_args[0] - def test_Device_export_file_to_usb_drive(self, mock_subprocess): - file = self.mock_file - passphrase = "mock passphrase" - - with mock.patch( - "securedrop_client.gui.conversation.export.device.TemporaryDirectory", - return_value=self.mock_tmpdir, - ): - self.device.export_file_to_usb_drive(file.uuid, passphrase) - mock_subprocess.assert_called_once() - - expected_md = self.device._DISK_METADATA.copy() - expected_md[self.device._DISK_ENCRYPTION_KEY_NAME] = passphrase - - self.device._create_archive.assert_called_once_with( - archive_dir=_MOCK_FILEDIR, - archive_fn=self.device._DISK_FN, - metadata=expected_md, - filepaths=[self.mock_file_location], - ) - - def test_Device_export_file_to_usb_drive_file_missing(self, mock_subprocess, mocker): - file = factory.File() # Not a real file, so not anywhere - device = Device(["/fake/file/location"]) + def test_Device_export_file_missing(self, mock_subprocess, mocker): + device = Device() warning_logger = mocker.patch( "securedrop_client.gui.conversation.export.device.logger.warning" @@ -193,37 +136,13 @@ def test_Device_export_file_to_usb_drive_file_missing(self, mock_subprocess, moc "securedrop_client.gui.conversation.export.device.TemporaryDirectory", return_value=self.mock_tmpdir, ): - device.export_file_to_usb_drive(file.uuid, "mock passphrase") + device.export(["/not/a/real/location"], "mock passphrase") warning_logger.assert_called_once() mock_subprocess.assert_not_called() # Todo: could get more specific about looking for the emitted failure signal - def test_Device_export_file_to_usb_drive_when_orig_file_already_exists(self, mock_subprocess): - file = self.mock_file - passphrase = "mock passphrase" - - with mock.patch( - "securedrop_client.gui.conversation.export.device.TemporaryDirectory", - return_value=self.mock_tmpdir, - ): - self.device.export_file_to_usb_drive(file.uuid, passphrase) - - expected_metadata = self.device._DISK_METADATA.copy() - expected_metadata[self.device._DISK_ENCRYPTION_KEY_NAME] = passphrase - expected_filepath = self.mock_file_location - - self.device._create_archive.assert_called_once_with( - archive_dir=_MOCK_FILEDIR, - archive_fn=self.device._DISK_FN, - metadata=expected_metadata, - filepaths=[expected_filepath], - ) - - mock_subprocess.assert_called_once() - assert _QREXEC_EXPORT_COMMAND in mock_subprocess.call_args[0] - - def test_Device_export_transcript(self, mock_subprocess): + def test_Device_export(self, mock_subprocess): filepath = "some/file/path" passphrase = "passphrase" @@ -231,7 +150,7 @@ def test_Device_export_transcript(self, mock_subprocess): "securedrop_client.gui.conversation.export.device.TemporaryDirectory", return_value=self.mock_tmpdir, ): - self.device.export_transcript(filepath, passphrase) + self.device.export([filepath], passphrase) expected_metadata = self.device._DISK_METADATA.copy() expected_metadata[self.device._DISK_ENCRYPTION_KEY_NAME] = passphrase @@ -245,28 +164,6 @@ def test_Device_export_transcript(self, mock_subprocess): mock_subprocess.assert_called_once() assert _QREXEC_EXPORT_COMMAND in mock_subprocess.call_args[0] - def test_Device_export_files(self, mock_subprocess): - filepaths = ["some/file/path", "some/other/file/path"] - passphrase = "Correct-horse-battery-staple!" - - expected_metadata = self.device._DISK_METADATA.copy() - expected_metadata[self.device._DISK_ENCRYPTION_KEY_NAME] = passphrase - - with mock.patch( - "securedrop_client.gui.conversation.export.device.TemporaryDirectory", - return_value=self.mock_tmpdir, - ): - self.device.export_files(filepaths, passphrase) - - self.device._create_archive.assert_called_once_with( - archive_dir=_MOCK_FILEDIR, - archive_fn=self.device._DISK_FN, - metadata=expected_metadata, - filepaths=filepaths, - ) - mock_subprocess.assert_called_once() - assert _QREXEC_EXPORT_COMMAND in mock_subprocess.call_args[0] - @pytest.mark.parametrize("status", [i.value for i in ExportStatus]) def test__run_qrexec_success(self, mocked_subprocess, status): mocked_subprocess.return_value = f"{status}\n".encode("utf-8") @@ -311,7 +208,7 @@ def test__create_archive(self, mocker): open(os.path.join(temp_dir, "temp_1"), "w+").close() open(os.path.join(temp_dir, "temp_2"), "w+").close() filepaths = [os.path.join(temp_dir, "temp_1"), os.path.join(temp_dir, "temp_2")] - device = Device(filepaths) + device = Device() archive_path = device._create_archive(temp_dir, "mock.sd-export", {}, filepaths) @@ -321,9 +218,7 @@ def test__create_archive(self, mocker): assert not os.path.exists(archive_path) def test__create_archive_with_an_export_file(self, mocker): - device = Device( - self.mock_file_location - ) # TODO might not work - might want the tmpdir below + device = Device() archive_path = None with TemporaryDirectory() as temp_dir, NamedTemporaryFile() as export_file: archive_path = device._create_archive( @@ -335,7 +230,7 @@ def test__create_archive_with_an_export_file(self, mocker): assert not os.path.exists(archive_path) def test__create_archive_with_multiple_export_files(self, mocker): - device = Device(self.mock_file_location) + device = Device() archive_path = None with TemporaryDirectory() as tmpdir, NamedTemporaryFile() as f1, NamedTemporaryFile() as f2: transcript_path = os.path.join(tmpdir, "transcript.txt") diff --git a/client/tests/gui/conversation/export/test_dialog.py b/client/tests/gui/conversation/export/test_dialog.py index adceb44c57..35a9d19a3a 100644 --- a/client/tests/gui/conversation/export/test_dialog.py +++ b/client/tests/gui/conversation/export/test_dialog.py @@ -161,7 +161,7 @@ def test_ExportDialog__export_files(mocker, export_dialog): export_dialog._export_files() - device.export_files.assert_called_once_with( + device.export.assert_called_once_with( ["/some/path/file123.jpg", "/some/path/memo.txt", "/some/path/transcript.txt"], "mock_passphrase", ) diff --git a/client/tests/gui/conversation/export/test_file_dialog.py b/client/tests/gui/conversation/export/test_file_dialog.py index 36c927fdd0..4608476f67 100644 --- a/client/tests/gui/conversation/export/test_file_dialog.py +++ b/client/tests/gui/conversation/export/test_file_dialog.py @@ -8,7 +8,7 @@ def test_ExportDialog_init(mocker): "securedrop_client.gui.conversation.ExportFileDialog._show_starting_instructions" ) - export_file_dialog = ExportFileDialog(mocker.MagicMock(), "mock_uuid", "mock.jpg") + export_file_dialog = ExportFileDialog(mocker.MagicMock(), "mock.jpg", ["/mock/path/to/file"]) _show_starting_instructions_fn.assert_called_once_with() assert export_file_dialog.passphrase_form.isHidden() @@ -21,7 +21,7 @@ def test_ExportDialog_init_sanitizes_filename(mocker): mocker.patch("securedrop_client.gui.widgets.QVBoxLayout.addWidget") filename = '' - ExportFileDialog(mocker.MagicMock(), "mock_uuid", filename) + ExportFileDialog(mocker.MagicMock(), filename, ["/mock/path/to/file"]) secure_qlabel.assert_any_call(filename, wordwrap=False, max_length=260) @@ -171,9 +171,7 @@ def test_ExportDialog__export_file(mocker, export_file_dialog): export_file_dialog._export_file() - device.export_file_to_usb_drive.assert_called_once_with( - export_file_dialog.file_uuid, "mock_passphrase" - ) + device.export.assert_called_once_with(export_file_dialog.filepaths, "mock_passphrase") def test_ExportDialog__on_export_preflight_check_succeeded(mocker, export_file_dialog): diff --git a/client/tests/gui/conversation/export/test_print_dialog.py b/client/tests/gui/conversation/export/test_print_dialog.py index fdb37ff8b4..0bd4836f8e 100644 --- a/client/tests/gui/conversation/export/test_print_dialog.py +++ b/client/tests/gui/conversation/export/test_print_dialog.py @@ -8,7 +8,7 @@ def test_PrintFileDialog_init(mocker): "securedrop_client.gui.conversation.PrintFileDialog._show_starting_instructions" ) - PrintFileDialog(mocker.MagicMock(), "mock_uuid", "mock.jpg") + PrintFileDialog(mocker.MagicMock(), "mock.jpg", ["/mock/path/to/file"]) _show_starting_instructions_fn.assert_called_once_with() @@ -19,7 +19,7 @@ def test_PrintFileDialog_init_sanitizes_filename(mocker): ) filename = '' - PrintFileDialog(mocker.MagicMock(), "mock_uuid", filename) + PrintFileDialog(mocker.MagicMock(), filename, ["/mock/path/to/file"]) secure_qlabel.assert_any_call(filename, wordwrap=False, max_length=260) diff --git a/client/tests/gui/conversation/export/test_transcript_dialog.py b/client/tests/gui/conversation/export/test_transcript_dialog.py index 380806ec56..27e77053b5 100644 --- a/client/tests/gui/conversation/export/test_transcript_dialog.py +++ b/client/tests/gui/conversation/export/test_transcript_dialog.py @@ -171,7 +171,7 @@ def test_TranscriptDialog__show_generic_error_message(mocker, export_transcript_ def test_TranscriptDialog__export_transcript(mocker, export_transcript_dialog): device = mocker.MagicMock() - device.export_transcript = mocker.MagicMock() + device.export = mocker.MagicMock() export_transcript_dialog._device = device export_transcript_dialog.passphrase_field.text = mocker.MagicMock( return_value="mock_passphrase" @@ -179,7 +179,7 @@ def test_TranscriptDialog__export_transcript(mocker, export_transcript_dialog): export_transcript_dialog._export_transcript() - device.export_transcript.assert_called_once_with("/some/path/transcript.txt", "mock_passphrase") + device.export.assert_called_once_with(["/some/path/transcript.txt"], "mock_passphrase") def test_TranscriptDialog__on_export_preflight_check_succeeded(mocker, export_transcript_dialog): diff --git a/client/tests/gui/test_actions.py b/client/tests/gui/test_actions.py index e4a97f4848..1354595607 100644 --- a/client/tests/gui/test_actions.py +++ b/client/tests/gui/test_actions.py @@ -279,7 +279,7 @@ def test_trigger(self, _): # action._export_device.run_printer_preflight_checks = ( # lambda: action._export_device.print_preflight_check_succeeded.emit() # ) - # action._export_device.print_transcript = ( + # action._export_device.print = ( # lambda transcript: action._export_device.print_succeeded.emit() # ) @@ -342,7 +342,7 @@ def test_trigger(self, _): # action._export_device.run_printer_preflight_checks = ( # lambda: action._export_device.print_preflight_check_succeeded.emit() # ) - # action._export_device.print_transcript = ( + # action._export_device.print = ( # lambda transcript: action._export_device.print_succeeded.emit() # ) diff --git a/client/tests/gui/test_widgets.py b/client/tests/gui/test_widgets.py index 11b4234bce..c0495d4b72 100644 --- a/client/tests/gui/test_widgets.py +++ b/client/tests/gui/test_widgets.py @@ -3587,6 +3587,7 @@ def test_FileWidget__on_export_clicked(mocker, session, source): get_file = mocker.MagicMock(return_value=file) controller = mocker.MagicMock(get_file=get_file) + file_location = file.location(controller.data_dir) export_device = mocker.patch("securedrop_client.gui.conversation.ExportDevice") fw = FileWidget( @@ -3600,7 +3601,7 @@ def test_FileWidget__on_export_clicked(mocker, session, source): dialog = mocker.patch("securedrop_client.gui.conversation.ExportFileDialog") fw._on_export_clicked() - dialog.assert_called_once_with(export_device(), file.uuid, file.filename) + dialog.assert_called_once_with(export_device(), file.filename, [file_location]) def test_FileWidget__on_export_clicked_missing_file(mocker, session, source): @@ -3637,7 +3638,7 @@ def test_FileWidget__on_export_clicked_missing_file(mocker, session, source): def test_FileWidget__on_print_clicked(mocker, session, source): """ - Ensure print_file is called when the PRINT button is clicked + Ensure print() is called when the PRINT button is clicked """ file = factory.File(source=source["source"], is_downloaded=True) session.add(file) @@ -3646,6 +3647,7 @@ def test_FileWidget__on_print_clicked(mocker, session, source): get_file = mocker.MagicMock(return_value=file) controller = mocker.MagicMock(get_file=get_file) export_device = mocker.patch("securedrop_client.gui.conversation.ExportDevice") + file_location = file.location(controller.data_dir) fw = FileWidget( file.uuid, @@ -3665,7 +3667,7 @@ def test_FileWidget__on_print_clicked(mocker, session, source): fw._on_print_clicked() - dialog.assert_called_once_with(export_device(), file.uuid, file.filename) + dialog.assert_called_once_with(export_device(), file.filename, [file_location]) def test_FileWidget__on_print_clicked_missing_file(mocker, session, source): diff --git a/client/tests/integration/conftest.py b/client/tests/integration/conftest.py index 8fc665843e..76796c2a6b 100644 --- a/client/tests/integration/conftest.py +++ b/client/tests/integration/conftest.py @@ -148,7 +148,7 @@ def modal_dialog(mocker, homedir): @pytest.fixture(scope="function") def mock_export(mocker): - device = Device(["/tmp/imaginary-export/imaginary-file-1.tar.gz.gpg"]) + device = Device() """A export that assumes the Qubes RPC calls are successful and skips them.""" device.run_preflight_checks = lambda: ExportStatus.DEVICE_LOCKED @@ -184,8 +184,8 @@ def print_dialog(mocker, homedir): controller.qubes = False gui.setup(controller) gui.login_dialog.close() - export_device = conversation.ExportDevice(["/mock/export/file"]) - dialog = conversation.PrintFileDialog(export_device, "file_uuid", "file_name") + export_device = conversation.ExportDevice() + dialog = conversation.PrintFileDialog(export_device, "file_name", ["/mock/export/file"]) yield dialog @@ -217,8 +217,10 @@ def export_file_dialog(mocker, homedir): controller.qubes = False gui.setup(controller) gui.login_dialog.close() - export_device = conversation.ExportDevice(["/mock/export/filepath"]) - dialog = conversation.ExportFileDialog(export_device, "file_uuid", "file_name") + export_device = conversation.ExportDevice() + dialog = conversation.ExportFileDialog( + export_device, "file_name", ["/mock/export/filepath"] + ) dialog.show() yield dialog