From 9b33e3bb1496075d8970f609b0b148f09b2ce105 Mon Sep 17 00:00:00 2001 From: Mateusz Mandera Date: Wed, 19 Mar 2025 03:46:15 +0800 Subject: [PATCH] export: Also add guardrail to the management command. --- zerver/lib/export.py | 27 +++++++++++ zerver/management/commands/export.py | 21 +++++++- zerver/tests/test_management_commands.py | 2 +- zerver/views/realm_export.py | 61 +++++++++--------------- 4 files changed, 71 insertions(+), 40 deletions(-) diff --git a/zerver/lib/export.py b/zerver/lib/export.py index 493aa8bad9..2abd884e2a 100644 --- a/zerver/lib/export.py +++ b/zerver/lib/export.py @@ -3034,3 +3034,30 @@ def do_common_export_processes(output_dir: str) -> None: logging.info("Exporting migration status") export_migration_status(output_dir) + + +def check_export_with_consent_is_usable(realm: Realm) -> bool: + # Users without consent enabled will end up deactivated in the exported + # data. An organization without a consenting Owner would therefore not be + # functional after export->import. That's most likely not desired by the user + # so check for such a case. + consented_user_ids = get_consented_user_ids(realm) + return UserProfile.objects.filter( + id__in=consented_user_ids, role=UserProfile.ROLE_REALM_OWNER, realm=realm + ).exists() + + +def check_public_export_is_usable(realm: Realm) -> bool: + # Since users with email visibility set to NOBODY won't have their real emails + # exported, this could result in a lack of functional Owner accounts. + # We make sure that at least one Owner can have their real email address exported. + return UserProfile.objects.filter( + role=UserProfile.ROLE_REALM_OWNER, + email_address_visibility__in=[ + UserProfile.EMAIL_ADDRESS_VISIBILITY_EVERYONE, + UserProfile.EMAIL_ADDRESS_VISIBILITY_MEMBERS, + UserProfile.EMAIL_ADDRESS_VISIBILITY_MODERATORS, + UserProfile.EMAIL_ADDRESS_VISIBILITY_ADMINS, + ], + realm=realm, + ).exists() diff --git a/zerver/management/commands/export.py b/zerver/management/commands/export.py index c8be4cd774..f48d1a4299 100644 --- a/zerver/management/commands/export.py +++ b/zerver/management/commands/export.py @@ -9,7 +9,11 @@ from django.utils.timezone import now as timezone_now from typing_extensions import override from zerver.actions.realm_settings import do_deactivate_realm -from zerver.lib.export import export_realm_wrapper +from zerver.lib.export import ( + check_export_with_consent_is_usable, + check_public_export_is_usable, + export_realm_wrapper, +) from zerver.lib.management import ZulipBaseCommand from zerver.models import RealmExport @@ -102,6 +106,11 @@ class Command(ZulipBaseCommand): action="store_true", help="Whether to export private data of users who consented", ) + parser.add_argument( + "--force", + action="store_true", + help="Skip checks for whether the generated export will be a usable realm.", + ) parser.add_argument( "--upload", action="store_true", @@ -117,6 +126,7 @@ class Command(ZulipBaseCommand): output_dir = options["output_dir"] public_only = options["public_only"] export_full_with_consent = options["export_full_with_consent"] + assert not (public_only and export_full_with_consent) print(f"\033[94mExporting realm\033[0m: {realm.string_id}") @@ -151,6 +161,15 @@ class Command(ZulipBaseCommand): f"Refusing to overwrite existing tarball: {tarball_path}. Aborting..." ) + if (not options["force"]) and ( + (export_full_with_consent and not check_export_with_consent_is_usable(realm)) + or (public_only and not check_public_export_is_usable(realm)) + ): + raise CommandError( + "The generated export will not be a usable organization! " + "You can pass --force to skip this check." + ) + if options["deactivate_realm"]: print(f"\033[94mDeactivating realm\033[0m: {realm.string_id}") do_deactivate_realm( diff --git a/zerver/tests/test_management_commands.py b/zerver/tests/test_management_commands.py index 8e37df70df..afc4bcf0d3 100644 --- a/zerver/tests/test_management_commands.py +++ b/zerver/tests/test_management_commands.py @@ -519,7 +519,7 @@ class TestExport(ZulipTestCase): self.example_user("iago"), "allow_private_data_export", True, acting_user=None ) do_change_user_setting( - self.example_user("iago"), "allow_private_data_export", True, acting_user=None + self.example_user("desdemona"), "allow_private_data_export", True, acting_user=None ) with ( diff --git a/zerver/views/realm_export.py b/zerver/views/realm_export.py index c9c4659a09..81b71f6eba 100644 --- a/zerver/views/realm_export.py +++ b/zerver/views/realm_export.py @@ -11,7 +11,11 @@ from analytics.models import RealmCount from zerver.actions.realm_export import do_delete_realm_export, notify_realm_export from zerver.decorator import require_realm_admin from zerver.lib.exceptions import JsonableError -from zerver.lib.export import get_consented_user_ids, get_realm_exports_serialized +from zerver.lib.export import ( + check_export_with_consent_is_usable, + check_public_export_is_usable, + get_realm_exports_serialized, +) from zerver.lib.queue import queue_event_on_commit from zerver.lib.response import json_success from zerver.lib.send_email import FromAddress @@ -82,43 +86,24 @@ def export_realm( ) ) - if export_type == RealmExport.EXPORT_FULL_WITH_CONSENT: - # Users without consent enabled will end up deactivated in the exported - # data. An organization without a consenting Owner would therefore not be - # functional after export->import. That's most likely not desired by the user - # so check for such a case and return an error. - consented_user_ids = get_consented_user_ids(realm) - if not UserProfile.objects.filter( - id__in=consented_user_ids, role=UserProfile.ROLE_REALM_OWNER, realm=realm - ).exists(): - raise JsonableError( - _( - "Make sure at least one Organization Owner is consenting to the export " - "or contact {email} for help." - ).format(email=FromAddress.SUPPORT) - ) - elif export_type == RealmExport.EXPORT_PUBLIC: - # Since users with email visibility set to NOBODY won't have their real emails - # exported, this could result in a lack of functional Owner accounts. - # We make sure that at least one Owner can have their real email address exported - # or return an error. - if not UserProfile.objects.filter( - role=UserProfile.ROLE_REALM_OWNER, - email_address_visibility__in=[ - UserProfile.EMAIL_ADDRESS_VISIBILITY_EVERYONE, - UserProfile.EMAIL_ADDRESS_VISIBILITY_MEMBERS, - UserProfile.EMAIL_ADDRESS_VISIBILITY_MODERATORS, - UserProfile.EMAIL_ADDRESS_VISIBILITY_ADMINS, - ], - realm=realm, - ).exists(): - raise JsonableError( - _( - "Make sure at least one Organization Owner allows other " - "Administrators to see their email address " - "or contact {email} for help" - ).format(email=FromAddress.SUPPORT) - ) + if ( + export_type == RealmExport.EXPORT_FULL_WITH_CONSENT + and not check_export_with_consent_is_usable(realm) + ): + raise JsonableError( + _( + "Make sure at least one Organization Owner is consenting to the export " + "or contact {email} for help." + ).format(email=FromAddress.SUPPORT) + ) + elif export_type == RealmExport.EXPORT_PUBLIC and not check_public_export_is_usable(realm): + raise JsonableError( + _( + "Make sure at least one Organization Owner allows other " + "Administrators to see their email address " + "or contact {email} for help" + ).format(email=FromAddress.SUPPORT) + ) row = RealmExport.objects.create( realm=realm,