Mirror
/
zulip
mirror of https://github.com/zulip/zulip.git
1
0
Fork 0
Code Issues Projects Releases Wiki Activity

ldap: Fix bad interaction between EMAIL_ADDRESS_VISIBILITY and LDAP sync. 

A block of LDAP integration code related to data synchronization did
not correctly handle EMAIL_ADDRESS_VISIBILITY_ADMINS, as it was
accessing .email, not .delivery_email, both for logging and doing the
mapping between email addresses and LDAP users.

Fixes #13539.
This commit is contained in:
Tim Abbott 2019-12-15 11:10:09 -08:00
parent 998b52dd8c
commit 02169c48cf
2 changed files with 19 additions and 6 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

13
zerver/tests/test_auth_backends.py
View File

@ -28,6 +28,7 @@ from zerver.lib.actions import (
do_invite_users, do_invite_users,
do_reactivate_realm, do_reactivate_realm,
do_reactivate_user, do_reactivate_user,
do_set_realm_property,
ensure_stream, ensure_stream,
validate_email, validate_email,
) )
@ -2986,6 +2987,18 @@ class TestZulipLDAPUserPopulator(ZulipLDAPTestCase):
hamlet = self.example_user('hamlet') hamlet = self.example_user('hamlet')
self.assertEqual(hamlet.full_name, 'New Name') self.assertEqual(hamlet.full_name, 'New Name')
def test_update_with_hidden_emails(self) -> None:
hamlet = self.example_user('hamlet')
realm = get_realm("zulip")
do_set_realm_property(realm, 'email_address_visibility', Realm.EMAIL_ADDRESS_VISIBILITY_ADMINS)
hamlet.refresh_from_db()
self.change_ldap_user_attr('hamlet', 'cn', 'New Name')
self.perform_ldap_sync(hamlet)
hamlet.refresh_from_db()
self.assertEqual(hamlet.full_name, 'New Name')
def test_update_split_full_name(self) -> None: def test_update_split_full_name(self) -> None:
self.change_ldap_user_attr('hamlet', 'cn', 'Name') self.change_ldap_user_attr('hamlet', 'cn', 'Name')
self.change_ldap_user_attr('hamlet', 'sn', 'Full') self.change_ldap_user_attr('hamlet', 'sn', 'Full')

12
zproject/backends.py
View File

@ -682,13 +682,13 @@ class ZulipLDAPUserPopulator(ZulipLDAPAuthBackendBase):
if user_disabled_in_ldap: if user_disabled_in_ldap:
if user.is_active: if user.is_active:
logging.info("Deactivating user %s because they are disabled in LDAP." % logging.info("Deactivating user %s because they are disabled in LDAP." %
(user.email,)) (user.delivery_email,))
do_deactivate_user(user) do_deactivate_user(user)
# Do an early return to avoid trying to sync additional data. # Do an early return to avoid trying to sync additional data.
return (user, built) return (user, built)
elif not user.is_active: elif not user.is_active:
logging.info("Reactivating user %s because they are not disabled in LDAP." % logging.info("Reactivating user %s because they are not disabled in LDAP." %
(user.email,)) (user.delivery_email,))
do_reactivate_user(user) do_reactivate_user(user)
self.sync_avatar_from_ldap(user, ldap_user) self.sync_avatar_from_ldap(user, ldap_user)
@ -717,14 +717,14 @@ def catch_ldap_error(signal: Signal, **kwargs: Any) -> None:
def sync_user_from_ldap(user_profile: UserProfile, logger: logging.Logger) -> bool: def sync_user_from_ldap(user_profile: UserProfile, logger: logging.Logger) -> bool:
backend = ZulipLDAPUserPopulator() backend = ZulipLDAPUserPopulator()
try: try:
ldap_username = backend.django_to_ldap_username(user_profile.email) ldap_username = backend.django_to_ldap_username(user_profile.delivery_email)
except ZulipLDAPExceptionNoMatchingLDAPUser: except ZulipLDAPExceptionNoMatchingLDAPUser:
if settings.LDAP_DEACTIVATE_NON_MATCHING_USERS: if settings.LDAP_DEACTIVATE_NON_MATCHING_USERS:
do_deactivate_user(user_profile) do_deactivate_user(user_profile)
logger.info("Deactivated non-matching user: %s" % (user_profile.email,)) logger.info("Deactivated non-matching user: %s" % (user_profile.delivery_email,))
return True return True
elif user_profile.is_active: elif user_profile.is_active:
logger.warning("Did not find %s in LDAP." % (user_profile.email,)) logger.warning("Did not find %s in LDAP." % (user_profile.delivery_email,))
return False return False
# What one would expect to see like to do here is just a call to # What one would expect to see like to do here is just a call to
@ -744,7 +744,7 @@ def sync_user_from_ldap(user_profile: UserProfile, logger: logging.Logger) -> bo
# making this flow possible in a more directly supported fashion. # making this flow possible in a more directly supported fashion.
updated_user = ZulipLDAPUser(backend, ldap_username, realm=user_profile.realm).populate_user() updated_user = ZulipLDAPUser(backend, ldap_username, realm=user_profile.realm).populate_user()
if updated_user: if updated_user:
logger.info("Updated %s." % (user_profile.email,)) logger.info("Updated %s." % (user_profile.delivery_email,))
return True return True
raise PopulateUserLDAPError("populate_user unexpectedly returned {}".format(updated_user)) raise PopulateUserLDAPError("populate_user unexpectedly returned {}".format(updated_user))