diff --git a/zerver/tests/test_auth_backends.py b/zerver/tests/test_auth_backends.py index 9b44281c26..b152345c0a 100644 --- a/zerver/tests/test_auth_backends.py +++ b/zerver/tests/test_auth_backends.py @@ -2416,14 +2416,6 @@ class DjangoToLDAPUsernameTests(ZulipTestCase): username = self.backend.django_to_ldap_username('"hamlet@test"@zulip') self.assertEqual(username, '"hamlet@test"@zulip') - def test_django_to_ldap_username_without_email_search(self) -> None: - with self.settings(AUTH_LDAP_REVERSE_EMAIL_SEARCH=None): - self.assertEqual(self.backend.django_to_ldap_username("hamlet"), "hamlet") - self.assertEqual(self.backend.django_to_ldap_username("newuser_email_as_uid@zulip.com"), - "newuser_email_as_uid@zulip.com") - with self.assertRaises(ZulipLDAPExceptionNoMatchingLDAPUser): - self.backend.django_to_ldap_username("hamlet@example.com") - def test_django_to_ldap_username_with_email_search(self) -> None: self.assertEqual(self.backend.django_to_ldap_username("hamlet"), self.ldap_username("hamlet")) @@ -2481,25 +2473,7 @@ class DjangoToLDAPUsernameTests(ZulipTestCase): user_profile.set_password(password) user_profile.save() - # Without email search, can't login via ldap: - with self.settings(AUTH_LDAP_REVERSE_EMAIL_SEARCH=None, LDAP_EMAIL_ATTR='mail'): - # Using hamlet's ldap password fails without email search: - self.assertEqual( - authenticate(username=user_profile.email, password=self.ldap_password(), realm=realm), - None) - # Need hamlet's zulip password to login (via email backend) - self.assertEqual( - authenticate(username=user_profile.email, password="testpassword", realm=realm), - user_profile) - # To login via ldap, username needs to be the ldap username, not email: - self.assertEqual( - authenticate(username=self.ldap_username("hamlet"), password=self.ldap_password(), - realm=realm), - user_profile) - - # With email search: with self.settings(LDAP_EMAIL_ATTR='mail'): - # Ldap password works now: self.assertEqual( authenticate(username=user_profile.email, password=self.ldap_password(), realm=realm), user_profile) @@ -2523,11 +2497,6 @@ class DjangoToLDAPUsernameTests(ZulipTestCase): # Syncing didn't deactivate the user: self.assertTrue(user_profile.is_active) - with self.settings(AUTH_LDAP_REVERSE_EMAIL_SEARCH=None): - # Without email search, the user will get deactivated. - sync_user_from_ldap(user_profile, mock.Mock()) - self.assertFalse(user_profile.is_active) - class ZulipLDAPTestCase(ZulipTestCase): def setUp(self) -> None: super().setUp() @@ -2642,17 +2611,6 @@ class TestLDAP(ZulipLDAPTestCase): realm=realm) self.assertEqual(user_profile, othello) - @override_settings(AUTH_LDAP_REVERSE_EMAIL_SEARCH=None, LDAP_APPEND_DOMAIN=None, - AUTHENTICATION_BACKENDS=('zproject.backends.ZulipLDAPAuthBackend',)) - def test_no_append_domain_and_email_search_not_configured(self) -> None: - with mock.patch("zproject.backends.logging.warning") as mock_warning: - result = email_belongs_to_ldap(get_realm("zulip"), "nonexistant@email.com") - self.assertEqual(result, True) - mock_warning.assert_called_with( - "LDAP_APPEND_DOMAIN isn't used, but searching by email " - "is not configured. email_belongs_to_ldap will always return True." - ) - @override_settings(AUTHENTICATION_BACKENDS=('zproject.backends.ZulipLDAPAuthBackend',)) def test_login_failure_due_to_wrong_password(self) -> None: with self.settings(LDAP_APPEND_DOMAIN='zulip.com'): diff --git a/zproject/backends.py b/zproject/backends.py index 56875c9208..2c4feb9e42 100644 --- a/zproject/backends.py +++ b/zproject/backends.py @@ -246,6 +246,11 @@ def is_valid_email(email: str) -> bool: return False return True +def check_ldap_config() -> None: + if not settings.LDAP_APPEND_DOMAIN: + # Email search needs to be configured in this case. + assert settings.AUTH_LDAP_USERNAME_ATTR and settings.AUTH_LDAP_REVERSE_EMAIL_SEARCH + def find_ldap_users_by_email(email: str) -> Optional[List[_LDAPUser]]: """ Returns list of _LDAPUsers matching the email search, @@ -265,16 +270,12 @@ def email_belongs_to_ldap(realm: Realm, email: str) -> bool: if not ldap_auth_enabled(realm): return False + check_ldap_config() if settings.LDAP_APPEND_DOMAIN: # Check if the email ends with LDAP_APPEND_DOMAIN return email.strip().lower().endswith("@" + settings.LDAP_APPEND_DOMAIN) # If we don't have an LDAP domain, we have to do a lookup for the email. - if not(settings.AUTH_LDAP_USERNAME_ATTR and settings.AUTH_LDAP_REVERSE_EMAIL_SEARCH): - logging.warning("LDAP_APPEND_DOMAIN isn't used, but searching by email " - "is not configured. email_belongs_to_ldap will always return True.") - return True - if find_ldap_users_by_email(email): return True else: @@ -313,6 +314,8 @@ class ZulipLDAPAuthBackendBase(ZulipAuthMixin, LDAPBackend): if settings.DEVELOPMENT and settings.FAKE_LDAP_MODE: # nocoverage init_fakeldap() + check_ldap_config() + # Disable django-auth-ldap's permissions functions -- we don't use # the standard Django user/group permissions system because they # are prone to performance issues. @@ -342,8 +345,7 @@ class ZulipLDAPAuthBackendBase(ZulipAuthMixin, LDAPBackend): raise ZulipLDAPExceptionOutsideDomain("Email %s does not match LDAP domain %s." % ( username, settings.LDAP_APPEND_DOMAIN)) result = email_to_username(username) - - elif settings.AUTH_LDAP_USERNAME_ATTR and settings.AUTH_LDAP_REVERSE_EMAIL_SEARCH: + else: # We can use find_ldap_users_by_email if is_valid_email(username): email_search_result = find_ldap_users_by_email(username)