diff --git a/templates/zerver/slack_import.html b/templates/zerver/slack_import.html index e3bf5b57e9..f9e7dd2e0e 100644 --- a/templates/zerver/slack_import.html +++ b/templates/zerver/slack_import.html @@ -70,7 +70,7 @@ {% endtrans %}
-

+

{{ invalid_file_error_message }}

{{ csrf_input }} diff --git a/zerver/actions/data_import.py b/zerver/actions/data_import.py index e25577046f..61518db802 100644 --- a/zerver/actions/data_import.py +++ b/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) diff --git a/zerver/data_import/slack.py b/zerver/data_import/slack.py index 6f187bb8c5..b4774e0329 100644 --- a/zerver/data_import/slack.py +++ b/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) diff --git a/zerver/lib/exceptions.py b/zerver/lib/exceptions.py index a0a0b654f6..ff019bc3d4 100644 --- a/zerver/lib/exceptions.py +++ b/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 diff --git a/zerver/tests/test_slack_importer.py b/zerver/tests/test_slack_importer.py index 35e7e3444b..834387887a 100644 --- a/zerver/tests/test_slack_importer.py +++ b/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") diff --git a/zerver/views/registration.py b/zerver/views/registration.py index 66e71027e7..01f64ec292 100644 --- a/zerver/views/registration.py +++ b/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