From a6e523f9e4310e8be67fafcac4a7955233192575 Mon Sep 17 00:00:00 2001 From: Harshit Bansal Date: Sun, 10 Mar 2019 07:57:19 +0000 Subject: [PATCH] ldap: Ensure email is valid for realm before registering. Previously, the LDAP authentication model ignored the realm-level settings for who can join a realm. This was sort of reasonable at the time, because the original LDAP auth was an SSO solution that didn't allow multiple realms, and so one could fully configure authentication settings on the LDAP side. But now that we allow multiple realms with the LDAP backend, one could easily imagine wanting different restrictions on them, and so it makes sense to add this enforcement. --- zerver/tests/test_auth_backends.py | 48 ++++++++++++++++++++++++------ zproject/backends.py | 21 ++++++++++--- 2 files changed, 56 insertions(+), 13 deletions(-) diff --git a/zerver/tests/test_auth_backends.py b/zerver/tests/test_auth_backends.py index 2593807cc9..1d4af87127 100644 --- a/zerver/tests/test_auth_backends.py +++ b/zerver/tests/test_auth_backends.py @@ -43,7 +43,7 @@ from zerver.lib.test_classes import ( ) from zerver.models import \ get_realm, email_to_username, CustomProfileField, CustomProfileFieldValue, \ - UserProfile, PreregistrationUser, Realm, get_user, MultiuseInvite + UserProfile, PreregistrationUser, Realm, RealmDomain, get_user, MultiuseInvite from zerver.signals import JUST_CREATED_THRESHOLD from confirmation.models import Confirmation, create_confirmation_link @@ -2538,6 +2538,34 @@ class TestLDAP(ZulipLDAPTestCase): with self.assertRaisesRegex(Exception, 'LDAP user doesn\'t have the needed email attribute'): backend.get_or_build_user(email, _LDAPUser()) + @override_settings(AUTHENTICATION_BACKENDS=('zproject.backends.ZulipLDAPAuthBackend',)) + def test_get_or_build_user_email(self) -> None: + class _LDAPUser: + attrs = {'fn': ['Test User']} + + ldap_user_attr_map = {'full_name': 'fn'} + + with self.settings(AUTH_LDAP_USER_ATTR_MAP=ldap_user_attr_map): + realm = self.backend._realm + realm.emails_restricted_to_domains = False + realm.disallow_disposable_email_addresses = True + realm.save() + + email = 'spam@mailnator.com' + with self.assertRaisesRegex(ZulipLDAPException, 'Email validation failed.'): + self.backend.get_or_build_user(email, _LDAPUser()) + + realm.emails_restricted_to_domains = True + realm.save(update_fields=['emails_restricted_to_domains']) + + email = 'spam+spam@mailnator.com' + with self.assertRaisesRegex(ZulipLDAPException, 'Email validation failed.'): + self.backend.get_or_build_user(email, _LDAPUser()) + + email = 'spam@acme.com' + with self.assertRaisesRegex(ZulipLDAPException, "This email domain isn't allowed in this organization."): + self.backend.get_or_build_user(email, _LDAPUser()) + @override_settings(AUTHENTICATION_BACKENDS=('zproject.backends.ZulipLDAPAuthBackend',)) def test_get_or_build_user_when_ldap_has_no_full_name_mapping(self) -> None: class _LDAPUser: @@ -2573,13 +2601,15 @@ class TestLDAP(ZulipLDAPTestCase): } } ldap_user_attr_map = {'full_name': 'fn', 'short_name': 'sn'} + + Realm.objects.create(string_id='acme') with self.settings( LDAP_APPEND_DOMAIN='zulip.com', AUTH_LDAP_BIND_PASSWORD='', AUTH_LDAP_USER_DN_TEMPLATE='uid=%(user)s,ou=users,dc=zulip,dc=com', AUTH_LDAP_USER_ATTR_MAP=ldap_user_attr_map): user_profile = self.backend.authenticate(self.example_email('hamlet'), 'testing', - realm=get_realm('zephyr')) + realm=get_realm('acme')) self.assertEqual(user_profile.email, self.example_email('hamlet')) @override_settings(AUTHENTICATION_BACKENDS=('zproject.backends.ZulipLDAPAuthBackend',)) @@ -2635,8 +2665,8 @@ class TestLDAP(ZulipLDAPTestCase): "full_name": "cn", "avatar": "thumbnailPhoto", }) - def test_login_success_when_user_does_not_exist_with_valid_subdomain( - self) -> None: + def test_login_success_when_user_does_not_exist_with_valid_subdomain(self) -> None: + RealmDomain.objects.create(realm=self.backend._realm, domain='acme.com') self.mock_ldap.directory = { 'uid=nonexisting,ou=users,dc=acme,dc=com': { 'cn': ['NonExisting', ], @@ -2663,21 +2693,21 @@ class TestLDAP(ZulipLDAPTestCase): @override_settings(AUTHENTICATION_BACKENDS=('zproject.backends.ZulipLDAPAuthBackend',)) def test_login_success_when_user_does_not_exist_with_split_full_name_mapping(self) -> None: self.mock_ldap.directory = { - 'uid=nonexisting,ou=users,dc=acme,dc=com': { + 'uid=nonexisting,ou=users,dc=zulip,dc=com': { 'fn': ['Non', ], 'ln': ['Existing', ], 'userPassword': ['testing', ], } } with self.settings( - LDAP_APPEND_DOMAIN='acme.com', + LDAP_APPEND_DOMAIN='zulip.com', AUTH_LDAP_BIND_PASSWORD='', - AUTH_LDAP_USER_DN_TEMPLATE='uid=%(user)s,ou=users,dc=acme,dc=com', + AUTH_LDAP_USER_DN_TEMPLATE='uid=%(user)s,ou=users,dc=zulip,dc=com', AUTH_LDAP_USER_ATTR_MAP={'first_name': 'fn', 'last_name': 'ln'}): - user_profile = self.backend.authenticate('nonexisting@acme.com', 'testing', + user_profile = self.backend.authenticate('nonexisting@zulip.com', 'testing', realm=get_realm('zulip')) assert(user_profile is not None) - self.assertEqual(user_profile.email, 'nonexisting@acme.com') + self.assertEqual(user_profile.email, 'nonexisting@zulip.com') self.assertEqual(user_profile.full_name, 'Non Existing') self.assertEqual(user_profile.realm.string_id, 'zulip') diff --git a/zproject/backends.py b/zproject/backends.py index 70d36cd4be..629a81e1e4 100644 --- a/zproject/backends.py +++ b/zproject/backends.py @@ -32,13 +32,14 @@ from social_core.backends.oauth import BaseOAuth2 from social_core.exceptions import AuthFailed, SocialAuthBaseException from zerver.lib.actions import do_create_user, do_reactivate_user, do_deactivate_user, \ - do_update_user_custom_profile_data + do_update_user_custom_profile_data, validate_email_for_realm from zerver.lib.dev_ldap_directory import init_fakeldap from zerver.lib.request import JsonableError from zerver.lib.users import check_full_name, validate_user_custom_profile_field -from zerver.models import CustomProfileField, PreregistrationUser, UserProfile, Realm, \ - custom_profile_fields_for_realm, get_default_stream_groups, get_user_profile_by_id, \ - remote_user_to_email, email_to_username, get_realm, get_user_by_delivery_email +from zerver.models import CustomProfileField, DisposableEmailError, DomainNotAllowedForRealmError, \ + EmailContainsPlusError, PreregistrationUser, UserProfile, Realm, custom_profile_fields_for_realm, \ + email_allowed_for_realm, get_default_stream_groups, get_user_profile_by_id, remote_user_to_email, \ + email_to_username, get_realm, get_user_by_delivery_email # This first batch of methods is used by other code in Zulip to check # whether a given authentication backend is enabled for a given realm. @@ -550,6 +551,18 @@ class ZulipLDAPAuthBackend(ZulipLDAPAuthBackendBase): # deactivated, so we shouldn't create a new user account raise ZulipLDAPException("Realm has been deactivated") + # Makes sure that email domain hasn't be restricted for this + # realm. The main thing here is email_allowed_for_realm; but + # we also call validate_email_for_realm just for consistency, + # even though its checks were already done above. + try: + email_allowed_for_realm(username, self._realm) + validate_email_for_realm(self._realm, username) + except DomainNotAllowedForRealmError: + raise ZulipLDAPException("This email domain isn't allowed in this organization.") + except (DisposableEmailError, EmailContainsPlusError): + raise ZulipLDAPException("Email validation failed.") + # We have valid LDAP credentials; time to create an account. full_name, short_name = self.get_mapped_name(ldap_user) try: