slack_import: Pipe file processing error message to the user.

When the slack import fails due to invalid zip file being uploaded,
we take user back to the file upload page with an appropriate
error message.

templates/zerver/slack_import.html

@@ -70,7 +70,7 @@
                         {% endtrans %}
                     </div>
                     <div id="slack-import-drag-and-drop" data-max-file-size="{{max_file_size}}"></div>
-                    <p id="slack-import-file-upload-error" class="help-inline text-error"></p>
+                    <p id="slack-import-file-upload-error" class="help-inline text-error">{{ invalid_file_error_message }}</p>
                 </div>
                 <form id="slack-import-start-upload-wrapper" method="post" class="form-inline {% if uploaded_import_file_name %}{% else %}hidden{% endif %}" action="{{ url('import_realm_from_slack') }}">
                     {{ csrf_input }}

zerver/actions/data_import.py

@@ -10,6 +10,7 @@ from zerver.actions.realm_settings import do_delete_all_realm_attachments
 from zerver.actions.users import do_change_user_role
 from zerver.context_processors import is_realm_import_enabled
 from zerver.data_import.slack import do_convert_zipfile
+from zerver.lib.exceptions import SlackImportInvalidFileError
 from zerver.lib.import_realm import do_import_realm
 from zerver.lib.upload import save_attachment_contents
 from zerver.models.prereg_users import PreregistrationRealm
@@ -78,6 +79,9 @@ def import_slack_data(event: dict[str, Any]) -> None:
             # Clean up the realm if the import failed
             preregistration_realm.created_realm = None
             preregistration_realm.data_import_metadata["is_import_work_queued"] = False
+            if type(e) is SlackImportInvalidFileError:
+                # Store the error to be displayed to the user.
+                preregistration_realm.data_import_metadata["invalid_file_error_message"] = str(e)
             preregistration_realm.save()
 
             realm = Realm.objects.get(string_id=string_id)

zerver/data_import/slack.py

@@ -49,6 +49,7 @@ from zerver.data_import.slack_message_conversion import (
     process_slack_block_and_attachment,
 )
 from zerver.lib.emoji import codepoint_to_name, get_emoji_file_name
+from zerver.lib.exceptions import SlackImportInvalidFileError
 from zerver.lib.export import MESSAGE_BATCH_CHUNK_SIZE, do_common_export_processes
 from zerver.lib.mime_types import guess_type
 from zerver.lib.storage import static_path
@@ -1523,7 +1524,9 @@ def do_convert_zipfile(
                 # top-level directories, or as `canvas_in_the_conversation.json`
                 # files in channel directories.  We do not parse these currently.
                 if not re.match(r"[^/]+(\.json|/([^/]+\.json)?)$", fileinfo.filename):
-                    raise Exception("This zip file does not look like a Slack archive")
+                    raise SlackImportInvalidFileError(
+                        "Uploaded zip file is not a valid Slack export."
+                    )
 
                 # file_size is the uncompressed size of the file
                 total_size += fileinfo.file_size
@@ -1532,7 +1535,7 @@ def do_convert_zipfile(
             # than a 10x size magnification is suspect, particularly
             # if it results in over 1GB.
             if total_size > 1024 * 1024 * 1024 and total_size > 10 * os.path.getsize(original_path):
-                raise Exception("This zip file is possibly malicious")
+                raise SlackImportInvalidFileError("Uploaded zip file is not a valid Slack export.")
 
             zipObj.extractall(slack_data_dir)
 

zerver/lib/exceptions.py

@@ -776,3 +776,14 @@ class DeliveryTimeNotInFutureError(JsonableError):
     @override
     def msg_format() -> str:
         return _("Scheduled delivery time must be in the future.")
+
+
+class SlackImportInvalidFileError(Exception):
+    """
+    An error that is raised during the Slack import process
+    and is intended to be shown to the user.
+    """
+
+    def __init__(self, message: str) -> None:
+        super().__init__(message)
+        self.message = message

zerver/tests/test_slack_importer.py

@@ -52,6 +52,7 @@ from zerver.data_import.slack import (
     slack_workspace_to_realm,
     users_to_zerver_userprofile,
 )
+from zerver.lib.exceptions import SlackImportInvalidFileError
 from zerver.lib.import_realm import do_import_realm
 from zerver.lib.test_classes import ZulipTestCase
 from zerver.lib.test_helpers import find_key_by_email, read_test_image_file
@@ -2255,7 +2256,7 @@ by Pieter
             email="test@example.com",
             data_import_metadata={"import_from": "slack"},
         )
-        mock_do_import_realm.side_effect = AssertionError("Import failed")
+        mock_do_import_realm.side_effect = SlackImportInvalidFileError("Invalid file")
 
         event = {
             "preregistration_realm_id": prereg_realm.id,
@@ -2264,7 +2265,7 @@ by Pieter
         }
 
         with (
-            self.assertRaises(AssertionError),
+            self.assertRaises(SlackImportInvalidFileError),
             self.assertLogs("zerver.actions.data_import", "ERROR"),
         ):
             import_slack_data(event)
@@ -2272,6 +2273,9 @@ by Pieter
         prereg_realm.refresh_from_db()
         self.assertIsNone(prereg_realm.created_realm)
         self.assertFalse(prereg_realm.data_import_metadata["is_import_work_queued"])
+        self.assertEqual(
+            prereg_realm.data_import_metadata["invalid_file_error_message"], "Invalid file"
+        )
         self.assertFalse(Realm.objects.filter(string_id="test-realm").exists())
 
     @mock.patch("zerver.actions.data_import.do_import_realm")

zerver/views/registration.py

@@ -390,6 +390,9 @@ def registration_helper(
                 context["uploaded_import_file_name"] = prereg_realm.data_import_metadata.get(
                     "uploaded_import_file_name"
                 )
+                context["invalid_file_error_message"] = prereg_realm.data_import_metadata.get(
+                    "invalid_file_error_message", ""
+                )
 
             return TemplateResponse(
                 request,
@@ -1055,7 +1058,22 @@ def realm_import_status(
         # TODO: Either store the path to the temporary conversion directory on
         # preregistration_realm.data_import_metadata, or have the conversion
         # process support writing updates to this for a better progress indicator.
-        return json_success(request, {"status": _("Converting Slack data… This may take a while.")})
+        if preregistration_realm.data_import_metadata.get("is_import_work_queued"):
+            return json_success(
+                request, {"status": _("Converting Slack data… This may take a while.")}
+            )
+        elif preregistration_realm.data_import_metadata.get("invalid_file_error_message"):
+            # Redirect user the file upload page if we have an error message to display.
+            result = {
+                "status": preregistration_realm.data_import_metadata.get(
+                    "invalid_file_error_message"
+                ),
+                "redirect": reverse(
+                    "get_prereg_key_and_redirect", kwargs={"confirmation_key": confirmation_key}
+                ),
+            }
+            return json_success(request, result)
+        # TODO: If there is something we need to fix for the import, we should notify the user.
 
     if realm.deactivated:
         # These "if" cases are in the inverse order than they're done