ldap: Fix unintended user deactivation in case of connection failure.

Fixes #13130.

django_auth_ldap doesn't give any other way of detecting that LDAPError
happened other than catching the signal it emits - so we have to
register a receiver. In the receiver we just raise our own Exception
which will properly propagate without being silenced by
django_auth_ldap. This will stop execution before the user gets
deactivated.

zerver/tests/test_auth_backends.py

@@ -55,7 +55,8 @@
     dev_auth_enabled, password_auth_enabled, github_auth_enabled, google_auth_enabled, \
     require_email_format_usernames, AUTH_BACKEND_NAME_MAP, \
     ZulipLDAPConfigurationError, ZulipLDAPExceptionOutsideDomain, \
-    ZulipLDAPException, query_ldap, sync_user_from_ldap, SocialAuthMixin
+    ZulipLDAPException, query_ldap, sync_user_from_ldap, SocialAuthMixin, \
+    PopulateUserLDAPError
 
 from zerver.views.auth import (maybe_send_to_registration,
                                _subdomain_token_salt)
@@ -2601,6 +2602,22 @@ def perform_ldap_sync(self, user_profile: UserProfile) -> None:
             result = sync_user_from_ldap(user_profile)
             self.assertTrue(result)
 
+    @mock.patch("zproject.backends.do_deactivate_user")
+    def test_ldap_auth_error_doesnt_deactivate_user(self, mock_deactivate: mock.MagicMock) -> None:
+        """
+        This is a test for a bug where failure to connect to LDAP in sync_user_from_ldap
+        (e.g. due to invalid credentials) would cause the user to be deactivated if
+        LDAP_DEACTIVATE_NON_MATCHING_USERS was True.
+        Details: https://github.com/zulip/zulip/issues/13130
+        """
+        with self.settings(
+                LDAP_DEACTIVATE_NON_MATCHING_USERS=True,
+                LDAP_APPEND_DOMAIN='zulip.com',
+                AUTH_LDAP_BIND_PASSWORD='wrongpass'):
+            with self.assertRaises(PopulateUserLDAPError):
+                sync_user_from_ldap(self.example_user('hamlet'))
+            mock_deactivate.assert_not_called()
+
     def test_update_full_name(self) -> None:
         self.mock_ldap.directory = {
             'uid=hamlet,ou=users,dc=zulip,dc=com': {

zproject/backends.py

@@ -17,12 +17,13 @@
 import magic
 from typing import Any, Dict, List, Optional, Set, Tuple, Union
 
-from django_auth_ldap.backend import LDAPBackend, _LDAPUser
+from django_auth_ldap.backend import LDAPBackend, _LDAPUser, ldap_error
 from django.contrib.auth import get_backends
 from django.contrib.auth.backends import RemoteUserBackend
 from django.conf import settings
 from django.core.exceptions import ValidationError
 from django.core.validators import validate_email
+from django.dispatch import receiver, Signal
 from django.http import HttpResponse, HttpResponseRedirect
 from django.shortcuts import render
 from django.urls import reverse
@@ -566,6 +567,24 @@ def authenticate(self, *, username: str, password: str, realm: Realm,
                      return_data: Optional[Dict[str, Any]]=None) -> Optional[UserProfile]:
         return None
 
+class PopulateUserLDAPError(Exception):
+    pass
+
+@receiver(ldap_error, sender=ZulipLDAPUserPopulator)
+def catch_ldap_error(signal: Signal, **kwargs: Any) -> None:
+    """
+    Inside django_auth_ldap populate_user(), if LDAPError is raised,
+    e.g. due to invalid connection credentials, the function catches it
+    and emits a signal (ldap_error) to communicate this error to others.
+    We normally don't use signals, but here there's no choice, so in this function
+    we essentially convert the signal to a normal exception that will properly
+    propagate out of django_auth_ldap internals.
+    """
+    if kwargs['context'] == 'populate_user':
+        # The exception message can contain the password (if it was invalid),
+        # so it seems better not to log that, and only use the original exception's name here.
+        raise PopulateUserLDAPError(kwargs['exception'].__class__.__name__)
+
 def sync_user_from_ldap(user_profile: UserProfile) -> bool:
     backend = ZulipLDAPUserPopulator()
     updated_user = backend.populate_user(backend.django_to_ldap_username(user_profile.email))