From e81aa740bc57c2bb5ec124ed0e37d79ef7da47c2 Mon Sep 17 00:00:00 2001 From: Mateusz Mandera Date: Fri, 3 Jan 2020 05:45:05 +0100 Subject: [PATCH] ldap: Protect against troublesome deactivations in ldap sync. If ldap sync is run while ldap is misconfigured, it can end up causing troublesome deactivations due to not finding users in ldap - deactivating all users, or deactivating all administrators of a realm, which then will require manual intervention to reactivate at least one admin in django shell. This change prevents such potential troublesome situations which are overwhelmingly likely to be unintentional. If intentional, --force option can be used to remove the protection. --- .../commands/sync_ldap_user_data.py | 51 +++++++++++++++---- 1 file changed, 40 insertions(+), 11 deletions(-) diff --git a/zerver/management/commands/sync_ldap_user_data.py b/zerver/management/commands/sync_ldap_user_data.py index 0a063090f5..87c6e54270 100644 --- a/zerver/management/commands/sync_ldap_user_data.py +++ b/zerver/management/commands/sync_ldap_user_data.py @@ -3,7 +3,7 @@ import logging from argparse import ArgumentParser from typing import Any, List - +from django.db import transaction from django.conf import settings from zerver.lib.logging_util import log_to_file @@ -16,20 +16,49 @@ logger = logging.getLogger('zulip.sync_ldap_user_data') log_to_file(logger, settings.LDAP_SYNC_LOG_PATH) # Run this on a cronjob to pick up on name changes. -def sync_ldap_user_data(user_profiles: List[UserProfile]) -> None: +def sync_ldap_user_data(user_profiles: List[UserProfile], deactivation_protection: bool=True) -> None: logger.info("Starting update.") - for u in user_profiles: - # This will save the user if relevant, and will do nothing if the user - # does not exist. - try: - sync_user_from_ldap(u, logger) - except ZulipLDAPException as e: - logger.error("Error attempting to update user %s:" % (u.delivery_email,)) - logger.error(e) + with transaction.atomic(): + realms = set([u.realm.string_id for u in user_profiles]) + + for u in user_profiles: + # This will save the user if relevant, and will do nothing if the user + # does not exist. + try: + sync_user_from_ldap(u, logger) + except ZulipLDAPException as e: + logger.error("Error attempting to update user %s:" % (u.delivery_email,)) + logger.error(e) + + if deactivation_protection: + if not UserProfile.objects.filter(is_bot=False, is_active=True).exists(): + error_msg = ("Ldap sync would have deactivated all users. This is most likely due " + + "to a misconfiguration of ldap settings. Rolling back...\n" + + "Use the --force option if the mass deactivation is intended.") + logger.error(error_msg) + # Raising an exception in this atomic block will rollback the transaction. + raise Exception(error_msg) + for string_id in realms: + if not UserProfile.objects.filter(is_bot=False, is_active=True, realm__string_id=string_id, + role__gte=UserProfile.ROLE_REALM_ADMINISTRATOR).exists(): + error_msg = ("Ldap sync would have deactivated all administrators of realm %s. " + + "This is most likely due " + + "to a misconfiguration of ldap settings. Rolling back...\n" + + "Use the --force option if the mass deactivation is intended.") + error_msg = error_msg % (string_id,) + logger.error(error_msg) + raise Exception(error_msg) + logger.info("Finished update.") class Command(ZulipBaseCommand): def add_arguments(self, parser: ArgumentParser) -> None: + parser.add_argument('-f', '--force', + dest='force', + action="store_true", + default=False, + help='Disable the protection against deactivating all users.') + self.add_realm_args(parser) self.add_user_list_args(parser) @@ -40,4 +69,4 @@ class Command(ZulipBaseCommand): include_deactivated=True) else: user_profiles = UserProfile.objects.select_related().filter(is_bot=False) - sync_ldap_user_data(user_profiles) + sync_ldap_user_data(user_profiles, not options['force'])