diff --git a/zerver/tests/test_auth_backends.py b/zerver/tests/test_auth_backends.py index 66ab3b4201..3e03a2194c 100644 --- a/zerver/tests/test_auth_backends.py +++ b/zerver/tests/test_auth_backends.py @@ -2678,6 +2678,20 @@ class TestZulipLDAPUserPopulator(ZulipLDAPTestCase): hamlet = self.example_user('hamlet') self.assertFalse(hamlet.is_active) + @mock.patch("zproject.backends.ZulipLDAPAuthBackendBase.sync_full_name_from_ldap") + def test_dont_sync_disabled_ldap_user(self, fake_sync: mock.MagicMock) -> None: + self.mock_ldap.directory = { + 'uid=hamlet,ou=users,dc=zulip,dc=com': { + 'cn': ['King Hamlet', ], + 'userAccountControl': ['2', ], + } + } + + with self.settings(AUTH_LDAP_USER_ATTR_MAP={'full_name': 'cn', + 'userAccountControl': 'userAccountControl'}): + self.perform_ldap_sync(self.example_user('hamlet')) + fake_sync.assert_not_called() + def test_reactivate_user(self) -> None: self.mock_ldap.directory = { 'uid=hamlet,ou=users,dc=zulip,dc=com': { diff --git a/zproject/backends.py b/zproject/backends.py index ac5440f858..2284940c6c 100644 --- a/zproject/backends.py +++ b/zproject/backends.py @@ -426,20 +426,23 @@ class ZulipLDAPAuthBackendBase(ZulipAuthMixin, LDAPBackend): In authentication contexts, this is overriden in ZulipLDAPAuthBackend. """ (user, built) = super().get_or_build_user(username, ldap_user) - self.sync_avatar_from_ldap(user, ldap_user) - self.sync_full_name_from_ldap(user, ldap_user) - self.sync_custom_profile_fields_from_ldap(user, ldap_user) if 'userAccountControl' in settings.AUTH_LDAP_USER_ATTR_MAP: user_disabled_in_ldap = self.is_account_control_disabled_user(ldap_user) - if user_disabled_in_ldap and user.is_active: - logging.info("Deactivating user %s because they are disabled in LDAP." % - (user.email,)) - do_deactivate_user(user) + if user_disabled_in_ldap: + if user.is_active: + logging.info("Deactivating user %s because they are disabled in LDAP." % + (user.email,)) + do_deactivate_user(user) + # Do an early return to avoid trying to sync additional data. return (user, built) - if not user_disabled_in_ldap and not user.is_active: + elif not user.is_active: logging.info("Reactivating user %s because they are not disabled in LDAP." % (user.email,)) do_reactivate_user(user) + + self.sync_avatar_from_ldap(user, ldap_user) + self.sync_full_name_from_ldap(user, ldap_user) + self.sync_custom_profile_fields_from_ldap(user, ldap_user) return (user, built) class ZulipLDAPAuthBackend(ZulipLDAPAuthBackendBase):