user_settings: Prevent LDAP users from setting a Zulip password.

Previously, if both EmailAuthBackend and LDAPAuthBackend were enabled,
LDAP users could set a password using EmailAuthBackend and continue to
use that password, even if their LDAP account was later deactivated.

That configuration wasn't supported at all before, so this doesn't fix
a pre-existing security issue, but now that we're making that a valid
configuration, we need to cover this case.

zerver/tests/test_settings.py

@@ -2,12 +2,14 @@
 import ujson
 
 from django.http import HttpResponse
+from django.test import override_settings
 from mock import patch
 from typing import Any, Dict
 
 from zerver.lib.initial_password import initial_password
 from zerver.lib.sessions import get_session_dict_user
 from zerver.lib.test_classes import ZulipTestCase
+from zerver.lib.test_helpers import MockLDAP
 from zerver.models import get_realm, get_user, UserProfile
 
 class ChangeSettingsTest(ZulipTestCase):
@@ -150,6 +152,64 @@ class ChangeSettingsTest(ZulipTestCase):
             ))
         self.assert_json_error(result, "Wrong password!")
 
+    @override_settings(AUTHENTICATION_BACKENDS=('zproject.backends.ZulipLDAPAuthBackend',
+                                                'zproject.backends.EmailAuthBackend',
+                                                'zproject.backends.ZulipDummyBackend'),
+                       AUTH_LDAP_BIND_PASSWORD='',
+                       AUTH_LDAP_USER_DN_TEMPLATE='uid=%(user)s,ou=users,dc=zulip,dc=com')
+    def test_change_password_ldap_backend(self) -> None:
+        ldap_user_attr_map = {'full_name': 'fn', 'short_name': 'sn'}
+        ldap_patcher = patch('django_auth_ldap.config.ldap.initialize')
+        mock_initialize = ldap_patcher.start()
+        mock_ldap = MockLDAP()
+        mock_initialize.return_value = mock_ldap
+
+        mock_ldap.directory = {
+            'uid=hamlet,ou=users,dc=zulip,dc=com': {
+                'userPassword': 'ldappassword',
+                'fn': ['New LDAP fullname']
+            }
+        }
+
+        self.login(self.example_email("hamlet"))
+        with self.settings(LDAP_APPEND_DOMAIN="zulip.com",
+                           AUTH_LDAP_USER_ATTR_MAP=ldap_user_attr_map):
+            result = self.client_patch(
+                "/json/settings",
+                dict(
+                    old_password=initial_password(self.example_email("hamlet")),
+                    new_password="ignored",
+                ))
+            self.assert_json_error(result, "Your Zulip password is managed in LDAP")
+
+            result = self.client_patch(
+                "/json/settings",
+                dict(
+                    old_password='ldappassword',
+                    new_password="ignored",
+                ))
+            self.assert_json_error(result, "Your Zulip password is managed in LDAP")
+
+        with self.settings(LDAP_APPEND_DOMAIN="example.com",
+                           AUTH_LDAP_USER_ATTR_MAP=ldap_user_attr_map):
+            result = self.client_patch(
+                "/json/settings",
+                dict(
+                    old_password=initial_password(self.example_email("hamlet")),
+                    new_password="ignored",
+                ))
+            self.assert_json_success(result)
+
+        with self.settings(LDAP_APPEND_DOMAIN=None,
+                           AUTH_LDAP_USER_ATTR_MAP=ldap_user_attr_map):
+            result = self.client_patch(
+                "/json/settings",
+                dict(
+                    old_password=initial_password(self.example_email("hamlet")),
+                    new_password="ignored",
+                ))
+            self.assert_json_error(result, "Your Zulip password is managed in LDAP")
+
     def test_changing_nothing_returns_error(self) -> None:
         """
         We need to supply at least one non-empty parameter

zerver/views/user_settings.py

@@ -25,6 +25,7 @@ from zerver.models import UserProfile, Realm, name_changes_disabled, \
     EmailChangeStatus
 from confirmation.models import get_object_from_key, render_confirmation_key_error, \
     ConfirmationKeyException, Confirmation
+from zproject.backends import email_belongs_to_ldap
 
 def confirm_email_change(request: HttpRequest, confirmation_key: str) -> HttpResponse:
     try:
@@ -62,8 +63,11 @@ def json_change_settings(request: HttpRequest, user_profile: UserProfile,
         return json_error(_("Please fill out all fields."))
 
     if new_password != "":
+        return_data = {}  # type: Dict[str, Any]
+        if email_belongs_to_ldap(user_profile.realm, user_profile.email):
+            return json_error(_("Your Zulip password is managed in LDAP"))
         if not authenticate(username=user_profile.email, password=old_password,
-                            realm=user_profile.realm):
+                            realm=user_profile.realm, return_data=return_data):
             return json_error(_("Wrong password!"))
         do_change_password(user_profile, new_password)
         # In Django 1.10, password changes invalidates sessions, see

zproject/backends.py

@@ -366,6 +366,19 @@ class ZulipRemoteUserBackend(RemoteUserBackend):
         email = remote_user_to_email(remote_user)
         return common_get_active_user(email, realm, return_data=return_data)
 
+def email_belongs_to_ldap(realm: Realm, email: str) -> bool:
+    if not ldap_auth_enabled(realm):
+        return False
+
+    # If we don't have an LDAP domain, it's impossible to tell which
+    # accounts are LDAP accounts, so treat all of them as LDAP
+    # accounts
+    if not settings.LDAP_APPEND_DOMAIN:
+        return True
+
+    # Otherwise, check if the email ends with LDAP_APPEND_DOMAIN
+    return email.strip().lower().endswith("@" + settings.LDAP_APPEND_DOMAIN)
+
 class ZulipLDAPException(_LDAPUser.AuthenticationFailed):
     pass