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.
This commit is contained in:
Mateusz Mandera 2020-01-03 05:45:05 +01:00 committed by Tim Abbott
parent bfb963b9aa
commit e81aa740bc
1 changed files with 40 additions and 11 deletions

View File

@ -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'])