Rewrite ExportWizard integration, unit, and functional tests. Include…

… small bugfixes in cli (export).

Extract localization strings.
Simplify wizard control flow.

client/securedrop_client/export.py

@@ -193,8 +193,9 @@ def _cleanup_tmpdir(self) -> None:
 
     def _on_export_process_complete(self) -> None:
         """
-        Callback, handle and emit QProcess result. As with all such callbacks,
-        the method signature cannot change.
+        Callback, handle and emit results from QProcess. Information
+        can be read from stdout/err. This callback will be triggered
+        if the QProcess exits with return code 0.
         """
         self._cleanup_tmpdir()
         # securedrop-export writes status to stderr
@@ -270,8 +271,6 @@ def _on_print_success(self) -> None:
         self._cleanup_tmpdir()
         logger.debug("Print success")
         self.print_succeeded.emit(ExportStatus.PRINT_SUCCESS)
-        # TODO: Previously emitted [filepaths]
-        self.export_completed.emit([])
 
     def end_process(self) -> None:
         """
@@ -305,6 +304,7 @@ def print(self, filepaths: List[str]) -> None:
             logger.debug("Beginning print")
 
             self.tmpdir = mkdtemp()
+            os.chmod(self.tmpdir, 0o700)
             archive_path = self._create_archive(
                 archive_dir=self.tmpdir,
                 archive_fn=self._PRINT_FN,
@@ -327,8 +327,6 @@ def print(self, filepaths: List[str]) -> None:
                 logger.error("Print failed while creating archive (no status supplied)")
                 self.print_failed.emit(ExportError(ExportStatus.ERROR_PRINT))
 
-        self.export_completed.emit(filepaths)
-
     def _create_archive(
         self, archive_dir: str, archive_fn: str, metadata: dict, filepaths: List[str] = []
     ) -> str:

client/securedrop_client/export_status.py

@@ -31,8 +31,9 @@ class ExportStatus(Enum):
     SUCCESS_EXPORT = "SUCCESS_EXPORT"
     ERROR_EXPORT = "ERROR_EXPORT"  # Could not write to disk
 
-    # Export succeeds but drives were not properly unmounted
+    # Export succeeds but drives were not properly closed
     ERROR_EXPORT_CLEANUP = "ERROR_EXPORT_CLEANUP"
+    ERROR_UNMOUNT_VOLUME_BUSY = "ERROR_UNMOUNT_VOLUME_BUSY"
 
     DEVICE_ERROR = "DEVICE_ERROR"  # Something went wrong while trying to check the device
 

client/securedrop_client/gui/actions.py

@@ -10,13 +10,13 @@
 from typing import Callable, Optional
 
 from PyQt5.QtCore import Qt, pyqtSlot
-from PyQt5.QtWidgets import QAction, QDialog, QMenu
+from PyQt5.QtWidgets import QAction, QApplication, QDialog, QMenu
 
 from securedrop_client import state
 from securedrop_client.conversation import Transcript as ConversationTranscript
 from securedrop_client.db import Source
+from securedrop_client.export import Export
 from securedrop_client.gui.base import ModalDialog
-from securedrop_client.gui.conversation import ExportDevice
 from securedrop_client.gui.conversation import (
     PrintTranscriptDialog as PrintConversationTranscriptDialog,
 )
@@ -184,7 +184,7 @@ 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()
+            export = Export()
             dialog = PrintConversationTranscriptDialog(
                 export, TRANSCRIPT_FILENAME, [str(file_path)]
             )
@@ -234,7 +234,7 @@ 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()
+            export_device = Export()
             wizard = ExportWizard(export_device, TRANSCRIPT_FILENAME, [str(file_path)])
             wizard.exec()
 
@@ -320,7 +320,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()
+            export_device = Export()
             files = [
                 stack.enter_context(open(file_location, "r")) for file_location in file_locations
             ]
@@ -335,6 +335,7 @@ def _prepare_to_export(self) -> None:
                 export_device,
                 summary,
                 [str(file_location) for file_location in file_locations],
+                QApplication.activeWindow(),
             )
             wizard.exec()
 

client/securedrop_client/gui/conversation/__init__.py

@@ -3,7 +3,6 @@
 """
 # Import classes here to make possible to import them from securedrop_client.gui.conversation
 from .delete import DeleteConversationDialog  # noqa: F401
-from .export import Export as ExportDevice  # noqa: F401
 from .export import ExportWizard as ExportWizard  # noqa: F401
-from .export import PrintDialog as PrintFileDialog  # noqa: F401
+from .export import PrintDialog  # noqa: F401
 from .export import PrintTranscriptDialog  # noqa: F401

client/securedrop_client/gui/conversation/export/__init__.py

@@ -1,4 +1,3 @@
-from ....export import Export  # noqa: F401
 from .export_wizard import ExportWizard  # noqa: F401
 from .print_dialog import PrintDialog  # noqa: F401
 from .print_transcript_dialog import PrintTranscriptDialog  # noqa: F401

client/securedrop_client/gui/conversation/export/export_wizard.py

@@ -1,12 +1,12 @@
 import logging
 from gettext import gettext as _
-from typing import List
+from typing import List, Optional
 
 from pkg_resources import resource_string
 from PyQt5.QtCore import QSize, Qt, pyqtSlot
 from PyQt5.QtGui import QIcon, QKeyEvent
 from PyQt5.QtWidgets import QAbstractButton  # noqa: F401
-from PyQt5.QtWidgets import QApplication, QWizard, QWizardPage
+from PyQt5.QtWidgets import QApplication, QWidget, QWizard, QWizardPage
 
 from securedrop_client.export import Export
 from securedrop_client.export_status import ExportStatus
@@ -41,15 +41,26 @@ class ExportWizard(QWizard):
     # it's populated later.
     PASS_PLACEHOLDER_FIELD = ""
 
-    def __init__(self, export: Export, summary_text: str, filepaths: List[str]) -> None:
-        parent = QApplication.activeWindow()
+    def __init__(
+        self,
+        export: Export,
+        summary_text: str,
+        filepaths: List[str],
+        parent: Optional[QWidget] = None,
+    ) -> None:
+        # Normally, the active window is the right parent, but if the wizard is launched
+        # via another element (a modal dialog, such as the "Some files may not be exported"
+        # modal), the parent will be the modal dialog and the wizard layout will be affected.
+        # In those cases we want to be able to specify a different parent.
+        if not parent:
+            parent = QApplication.activeWindow()
         super().__init__(parent)
         self.export = export
         self.summary_text = SecureQLabel(
             summary_text, wordwrap=False, max_length=self.FILENAME_WIDTH_PX
         ).text()
         self.filepaths = filepaths
-        self.current_status = None  # Optional[ExportStatus]
+        self.current_status: Optional[ExportStatus] = None
 
         # Signal from qrexec command runner
         self.export.export_state_changed.connect(self.on_status_received)
@@ -58,17 +69,6 @@ def __init__(self, export: Export, summary_text: str, filepaths: List[str]) -> N
         # (Avoid orphaned QProcess)
         self.finished.connect(self.export.end_process)
 
-        # Activestate animation
-        self.button_animation = load_movie("activestate-wide.gif")
-        self.button_animation.setScaledSize(QSize(32, 32))
-        self.button_animation.frameChanged.connect(self._animate_activestate)
-
-        # Buttons
-        self.next_button = self.button(QWizard.WizardButton.NextButton)  # type: QAbstractButton
-        self.cancel_button = self.button(QWizard.WizardButton.CancelButton)  # type: QAbstractButton
-        self.back_button = self.button(QWizard.WizardButton.BackButton)  # type: QAbstractButton
-        self.finish_button = self.button(QWizard.WizardButton.FinishButton)  # type: QAbstractButton
-
         self._style_buttons()
         self._set_layout()
         self._set_pages()
@@ -93,26 +93,42 @@ def _style_buttons(self) -> None:
         Style QWizard buttons and connect "Next" button click event to
         request_export slot.
         """
+        # Activestate animation
+        self.button_animation = load_movie("activestate-wide.gif")
+        self.button_animation.setScaledSize(QSize(32, 32))
+        self.button_animation.frameChanged.connect(self._animate_activestate)
+
+        # Buttons
+        self.next_button = self.button(QWizard.WizardButton.NextButton)  # type: QAbstractButton
+        self.cancel_button = self.button(QWizard.WizardButton.CancelButton)  # type: QAbstractButton
+        self.back_button = self.button(QWizard.WizardButton.BackButton)  # type: QAbstractButton
+        self.finish_button = self.button(QWizard.WizardButton.FinishButton)  # type: QAbstractButton
+
         self.next_button.setObjectName("QWizardButton_PrimaryButton")
         self.next_button.setStyleSheet(self.BUTTON_CSS)
-        self.next_button.setMinimumSize(QSize(130, 40))
+        self.next_button.setMinimumSize(QSize(142, 40))
         self.next_button.setMaximumHeight(40)
+        self.next_button.setIconSize(QSize(21, 21))
         self.next_button.clicked.connect(self.request_export)
+        self.next_button.setFixedSize(QSize(142, 40))
 
         self.cancel_button.setObjectName("QWizardButton_GenericButton")
         self.cancel_button.setStyleSheet(self.BUTTON_CSS)
-        self.cancel_button.setMinimumSize(QSize(130, 40))
+        self.cancel_button.setMinimumSize(QSize(142, 40))
         self.cancel_button.setMaximumHeight(40)
+        self.cancel_button.setFixedSize(QSize(142, 40))
 
         self.back_button.setObjectName("QWizardButton_GenericButton")
         self.back_button.setStyleSheet(self.BUTTON_CSS)
-        self.back_button.setMinimumSize(QSize(130, 40))
+        self.back_button.setMinimumSize(QSize(142, 40))
         self.back_button.setMaximumHeight(40)
+        self.back_button.setFixedSize(QSize(142, 40))
 
         self.finish_button.setObjectName("QWizardButton_GenericButton")
         self.finish_button.setStyleSheet(self.BUTTON_CSS)
-        self.finish_button.setMinimumSize(QSize(130, 40))
+        self.finish_button.setMinimumSize(QSize(142, 40))
         self.finish_button.setMaximumHeight(40)
+        self.finish_button.setFixedSize(QSize(142, 40))
 
         self.setButtonText(QWizard.WizardButton.NextButton, _("CONTINUE"))
         self.setButtonText(QWizard.WizardButton.CancelButton, _("CANCEL"))
@@ -130,7 +146,8 @@ def _stop_animate_activestate(self) -> None:
         self.button_animation.stop()
 
     def _set_layout(self) -> None:
-        self.setWindowTitle(f"Export {self.summary_text}")  # TODO (il8n)
+        title = ("Export %(summary)s") % {"summary": self.summary_text}
+        self.setWindowTitle(title)
         self.setObjectName("QWizard_export")
         self.setStyleSheet(self.WIZARD_CSS)
         self.setModal(False)
@@ -183,79 +200,19 @@ def request_export(self) -> None:
     @pyqtSlot(object)
     def on_status_received(self, status: ExportStatus) -> None:
         """
-        Receive status updates from export process. The QWizard is responsible for
-        listening for a status that requires it to adjust its own position outside of the
-        normal wizard control flow, such as jumping to an error page if an unrecoverable error
-        status is encountered, or "rewinding" to a previous page if an unexpected status is
-        encountered (USB device removed after proceeding past that part of the workflow).
-
+        Receive status update from export process in order to update the animation.
         Child QWizardPages also implement this listener in order to update their own UI and store
         a reference to the current status.
 
-        Advancing through the normal QWizard control flow is handled by child pages.
+        Adjusting the QWizard control flow based on ExportStatus is handled by each child page.
         """
-        logger.debug(f"Wizard received {status.value}. Current page is {type(self.currentPage())}")
-
         # Release the page (page was held during "next" button click event)
         page = self.currentPage()
         if isinstance(page, ExportWizardPage):
             page.set_complete(True)
         self._stop_animate_activestate()
-
-        # Unrecoverable - end the wizard
-        if status in [
-            ExportStatus.ERROR_MOUNT,
-            ExportStatus.ERROR_EXPORT,
-            ExportStatus.ERROR_MISSING_FILES,
-            ExportStatus.DEVICE_ERROR,
-            ExportStatus.CALLED_PROCESS_ERROR,
-            ExportStatus.UNEXPECTED_RETURN_STATUS,
-        ]:
-            logger.error(f"Encountered {status.value}, cannot export")
-            self.end_wizard_with_error(status)
-            return
-
-        target = None  # Optional[PageEnum]
-        if status in [
-            ExportStatus.NO_DEVICE_DETECTED,
-            ExportStatus.MULTI_DEVICE_DETECTED,
-            ExportStatus.INVALID_DEVICE_DETECTED,
-        ]:
-            target = Pages.INSERT_USB
-        elif status in [ExportStatus.DEVICE_LOCKED, ExportStatus.ERROR_UNLOCK_LUKS]:
-            target = Pages.UNLOCK_USB
-
-        # Someone may have yanked out or unmounted a USB
-        if target and self.currentId() > target:
-            self.rewind(target)
-
-        # Update status
         self.current_status = status
 
-    def rewind(self, target: Pages) -> None:
-        """
-        Navigate back to target page.
-        """
-        logger.debug(f"Wizard: rewind from {self.currentId()} to {target}")
-        while self.currentId() > target:
-            self.back()
-
-    def end_wizard_with_error(self, error: ExportStatus) -> None:
-        """
-        If an end state is reached, display message and let user
-        end the wizard.
-        """
-        logger.debug("End wizard with error")
-        if isinstance(self.currentPage(), PreflightPage):
-            logger.debug("On preflight page, no reordering needed")
-        else:
-            while self.currentId() > Pages.ERROR:
-                self.back()
-        logger.debug(f"Target: {Pages.ERROR}. Actual: {self.currentId()}")
-        page = self.currentPage()
-        if isinstance(page, ExportWizardPage):
-            page.update_content(error)
-
     def _create_preflight(self) -> QWizardPage:
         return PreflightPage(self.export, self.summary_text)
 

client/securedrop_client/gui/conversation/export/export_wizard_constants.py

@@ -25,17 +25,24 @@ class Pages(IntEnum):
 # Human-readable status info
 STATUS_MESSAGES = {
     ExportStatus.NO_DEVICE_DETECTED: _("No device detected"),
-    ExportStatus.MULTI_DEVICE_DETECTED: _("Too many USBs; please insert one supported device."),
+    ExportStatus.MULTI_DEVICE_DETECTED: _(
+        "Too many USB devices detected; please " "insert one supported device."
+    ),
     ExportStatus.INVALID_DEVICE_DETECTED: _(
         "Either the drive is not encrypted or there is something else wrong with it."
+        "If this is a VeraCrypt drive, please unlock it from within `sd-devices`, then try again."
     ),
     ExportStatus.DEVICE_WRITABLE: _("The device is ready for export."),
     ExportStatus.DEVICE_LOCKED: _("The device is locked."),
     ExportStatus.ERROR_UNLOCK_LUKS: _("The passphrase provided did not work. Please try again."),
     ExportStatus.ERROR_MOUNT: _("Error mounting drive"),
     ExportStatus.ERROR_EXPORT: _("Error during export"),
+    ExportStatus.ERROR_UNMOUNT_VOLUME_BUSY: _(
+        "Files were exported succesfully, but the USB device could not be unmounted."
+    ),
     ExportStatus.ERROR_EXPORT_CLEANUP: _(
-        "Files were exported succesfully, but the drive could not be unmounted"
+        "Files were exported succesfully, but some temporary files remain on disk."
+        "Reboot to remove them."
     ),
     ExportStatus.SUCCESS_EXPORT: _("Export successful"),
     ExportStatus.DEVICE_ERROR: _(