Mirror
/
zulip
mirror of https://github.com/zulip/zulip.git
1
0
Fork 0
Code Issues Projects Releases Wiki Activity

ldap: Do a proper search for email in email_belongs_to_ldap. 

This fixes a collection of bugs surrounding LDAP configurations A and
C (i.e. LDAP_APPEND_DOMAIN=None) with EmailAuthBackend also enabled.

The core problem was that our desired security model in that setting
of requiring LDAP authentication for accounts managed by LDAP was not
implementable without a way to

Now admins can configure an LDAPSearch query that will find if there
are users in LDAP that have the email address and
email_belongs_to_ldap() will take advantage of that - no longer
returning True in response to all requests and thus blocking email
backend authentication.

In the documentation, we describe this as mandatory configuration for
users (and likely will make it so soon in the code) because the
failure modes for this not being configured are confusing.

But making that change is pending work to improve the relevant error
messages.

Fixes #11715.
This commit is contained in:
Mateusz Mandera 2019-10-05 01:02:46 +02:00 committed by Tim Abbott
parent 8d172d8bf6
commit fea4d0b2be
5 changed files with 91 additions and 15 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

10
docs/production/authentication-methods.md
View File

@ -182,6 +182,16 @@ In either configuration, you will need to do the following:
To do this, set `AUTH_LDAP_USER_SEARCH` to query by LDAP
username, and `LDAP_EMAIL_ATTR = "email"`.
5. In configurations (A) and (C), you need to tell Zulip how to look
up a user's LDAP data given their user's email address:
* Set `AUTH_LDAP_REVERSE_EMAIL_SEARCH` to a query that will find an
LDAP user given their email address. Generally, this will be
`AUTH_LDAP_USER_SEARCH` in configuration (A) or a search by
`LDAP_EMAIL_ATTR` in configuration (C).
* Set `AUTH_LDAP_USERNAME_ATTR` to the name of the LDAP attribute
for the user's LDAP username in that search result.
You can quickly test whether your configuration works by running:
```

43
zerver/tests/test_auth_backends.py
View File

@ -3,7 +3,7 @@ from django.conf import settings
from django.core import mail
from django.http import HttpResponse
from django.test import override_settings
from django_auth_ldap.backend import LDAPBackend, _LDAPUser
from django_auth_ldap.backend import LDAPBackend, LDAPSearch, _LDAPUser
from django.test.client import RequestFactory
from django.utils.timezone import now as timezone_now
from typing import Any, Callable, Dict, List, Optional, Set, Tuple
@ -12,6 +12,7 @@ from django.urls import reverse
import httpretty
import ldap
import jwt
import mock
import re
@ -56,7 +57,7 @@ from zproject.backends import ZulipDummyBackend, EmailAuthBackend, \
require_email_format_usernames, AUTH_BACKEND_NAME_MAP, \
ZulipLDAPConfigurationError, ZulipLDAPExceptionOutsideDomain, \
ZulipLDAPException, query_ldap, sync_user_from_ldap, SocialAuthMixin, \
PopulateUserLDAPError, SAMLAuthBackend, saml_auth_enabled
PopulateUserLDAPError, SAMLAuthBackend, saml_auth_enabled, email_belongs_to_ldap
from zerver.views.auth import (maybe_send_to_registration,
_subdomain_token_salt)
@ -2468,6 +2469,44 @@ class TestLDAP(ZulipLDAPTestCase):
assert (user_profile is not None)
self.assertEqual(user_profile, self.example_user("aaron"))
@override_settings(AUTHENTICATION_BACKENDS=('zproject.backends.EmailAuthBackend',
'zproject.backends.ZulipLDAPAuthBackend',))
def test_email_and_ldap_backends_together(self) -> None:
with self.settings(
LDAP_EMAIL_ATTR='mail',
AUTH_LDAP_REVERSE_EMAIL_SEARCH = LDAPSearch("ou=users,dc=zulip,dc=com",
ldap.SCOPE_ONELEVEL,
"(mail=%(email)s)"),
AUTH_LDAP_USERNAME_ATTR = "uid"
):
realm = get_realm('zulip')
self.assertEqual(email_belongs_to_ldap(realm, self.example_email("aaron")), True)
# aaron's uid in our test directory is "letham".
user_profile = ZulipLDAPAuthBackend().authenticate(username="letham", password='testing',
realm=realm)
self.assertEqual(user_profile, self.example_user("aaron"))
othello = self.example_user('othello')
password = "testpassword"
othello.set_password(password)
othello.save()
self.assertEqual(email_belongs_to_ldap(realm, othello.email), False)
user_profile = EmailAuthBackend().authenticate(username=othello.email, password=password,
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'):

30
zproject/backends.py
View File

@ -18,7 +18,8 @@ import magic
import ujson
from typing import Any, Dict, List, Optional, Set, Tuple, Union
from django_auth_ldap.backend import LDAPBackend, _LDAPUser, ldap_error
from django_auth_ldap.backend import LDAPBackend, LDAPReverseEmailSearch, \
_LDAPUser, ldap_error
from django.contrib.auth import get_backends
from django.contrib.auth.backends import RemoteUserBackend
from django.conf import settings
@ -241,6 +242,14 @@ def is_valid_email(email: str) -> bool:
return False
return True
def find_ldap_users_by_email(email: str) -> Optional[List[_LDAPUser]]:
"""
Returns list of _LDAPUsers matching the email search,
or None if no matches are found.
"""
email_search = LDAPReverseEmailSearch(LDAPBackend(), email)
return email_search.search_for_users(should_populate=False)
def email_belongs_to_ldap(realm: Realm, email: str) -> bool:
"""Used to make determinations on whether a user's email address is
managed by LDAP. For environments using both LDAP and
@ -252,14 +261,21 @@ 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:
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
# Otherwise, check if the email ends with LDAP_APPEND_DOMAIN
return email.strip().lower().endswith("@" + settings.LDAP_APPEND_DOMAIN)
if find_ldap_users_by_email(email):
return True
else:
return False
class ZulipLDAPException(_LDAPUser.AuthenticationFailed):
"""Since this inherits from _LDAPUser.AuthenticationFailed, these will

21
zproject/prod_settings_template.py
View File

@ -485,12 +485,11 @@ AUTH_LDAP_BIND_DN = ""
AUTH_LDAP_USER_SEARCH = LDAPSearch("ou=users,dc=example,dc=com",
ldap.SCOPE_SUBTREE, "(uid=%(user)s)")
# Domain to combine with a user's username to figure out their email address.
#
# If users log in as e.g. "sam" when their email address is "sam@example.com",
# set this to "example.com". If users log in with their full email addresses,
# leave as None; if the username -> email address mapping isn't so simple,
# leave as None and see LDAP_EMAIL_ATTR.
# Configuration to lookup a user's LDAP data given their email address
# (For Zulip reverse mapping). If users log in as e.g. "sam" when
# their email address is "sam@example.com", set LDAP_APPEND_DOMAIN to
# "example.com". Otherwise, leave LDAP_APPEND_DOMAIN=None and set
# AUTH_LDAP_REVERSE_EMAIL_SEARCH and AUTH_LDAP_USERNAME_ATTR below.
LDAP_APPEND_DOMAIN = None # type: Optional[str]
# LDAP attribute to find a user's email address.
@ -499,6 +498,16 @@ LDAP_APPEND_DOMAIN = None # type: Optional[str]
# or if using LDAP_APPEND_DOMAIN.
LDAP_EMAIL_ATTR = None # type: Optional[str]
# AUTH_LDAP_REVERSE_EMAIL_SEARCH works like AUTH_LDAP_USER_SEARCH and
# should query an LDAP user given their email address. It and
# AUTH_LDAP_USERNAME_ATTR are required when LDAP_APPEND_DOMAIN is None.
#AUTH_LDAP_REVERSE_EMAIL_SEARCH = LDAPSearch("ou=users,dc=example,dc=com",
# ldap.SCOPE_SUBTREE, "(email=%(email)s)")
# AUTH_LDAP_USERNAME_ATTR should be the Zulip username attribute
# (defined in AUTH_LDAP_USER_SEARCH).
#AUTH_LDAP_USERNAME_ATTR = "uid"
# This map defines how to populate attributes of a Zulip user from LDAP.
#
# The format is `zulip_name: ldap_name`; each entry maps a Zulip

2
zproject/settings.py
View File

@ -147,6 +147,8 @@ DEFAULT_SETTINGS = {
# LDAP auth
'AUTH_LDAP_SERVER_URI': "",
'LDAP_EMAIL_ATTR': None,
'AUTH_LDAP_USERNAME_ATTR': None,
'AUTH_LDAP_REVERSE_EMAIL_SEARCH': None,
# AUTH_LDAP_CONNECTION_OPTIONS: we set ldap.OPT_REFERRALS below if unset.
'AUTH_LDAP_CONNECTION_OPTIONS': {},
# Disable django-auth-ldap caching, to prevent problems with OU changes.