From 78297efefd6f820a4b21ff1077e364f9cef12900 Mon Sep 17 00:00:00 2001 From: Mateusz Mandera Date: Wed, 25 Aug 2021 23:15:27 +0200 Subject: [PATCH] ldap: Use a workaround instead of forking django-auth-ldap. Till now, we've been forking django-auth-ldap at https://github.com/zulip/django-auth-ldap to put the LDAPReverseEmailSearch feature in it, hoping to get it merged upstream in https://github.com/django-auth-ldap/django-auth-ldap/pull/150 The efforts to get it merged have stalled for now however and we don't want to be on the fork forever, so this commit puts the email search feature as a clumsy workaround inside our codebase and switches to using the latest upstream release instead of the fork. --- requirements/common.in | 5 +--- requirements/dev.txt | 5 ++-- requirements/prod.txt | 5 ++-- version.py | 2 +- zproject/backends.py | 52 ++++++++++++++++++++++++++++++++++++------ 5 files changed, 53 insertions(+), 16 deletions(-) diff --git a/requirements/common.in b/requirements/common.in index fe030d95cd..60c2205918 100644 --- a/requirements/common.in +++ b/requirements/common.in @@ -42,10 +42,7 @@ mypy-boto3-s3 defusedxml # Needed for LDAP support -# Using our fork for the feature of searching users by email. -# https://github.com/django-auth-ldap/django-auth-ldap/pull/150 for monitoring -# progress on merging this upstream. -https://github.com/zulip/django-auth-ldap/archive/e26d0ef2a7ff77ab3fdd7b6578a76081f780778c.zip#egg=django-auth-ldap==2.0.0zulip1 +django-auth-ldap # Django extension providing bitfield support django-bitfield diff --git a/requirements/dev.txt b/requirements/dev.txt index d35fbad28f..e4bc455b40 100644 --- a/requirements/dev.txt +++ b/requirements/dev.txt @@ -391,8 +391,9 @@ django[argon2]==3.2.6 \ # django-phonenumber-field # django-sendfile2 # django-two-factor-auth -https://github.com/zulip/django-auth-ldap/archive/e26d0ef2a7ff77ab3fdd7b6578a76081f780778c.zip#egg=django-auth-ldap==2.0.0zulip1 \ - --hash=sha256:1a104fdb5085ef9340996ae82d4b302f99c39c5d9d60d4ae55bcc7c1f58cb65e +django-auth-ldap==3.0.0 \ + --hash=sha256:19ee19034f344d9efd07ed88d3187e256ec33ae39d6a47222083b89f7d35c5f6 \ + --hash=sha256:1f2d5c562d9ba9a5e9a64099ae9798e1a63840a11afe4d1c4a9c74121f066eaa # via -r requirements/common.in django-bitfield==2.1.0 \ --hash=sha256:158f1056e22cce450d0a49633ea77bfd84b85a2294b1ef03faa775a485f4065d \ diff --git a/requirements/prod.txt b/requirements/prod.txt index 65f82eab39..9c6469d8cf 100644 --- a/requirements/prod.txt +++ b/requirements/prod.txt @@ -242,8 +242,9 @@ django[argon2]==3.2.6 \ # django-phonenumber-field # django-sendfile2 # django-two-factor-auth -https://github.com/zulip/django-auth-ldap/archive/e26d0ef2a7ff77ab3fdd7b6578a76081f780778c.zip#egg=django-auth-ldap==2.0.0zulip1 \ - --hash=sha256:1a104fdb5085ef9340996ae82d4b302f99c39c5d9d60d4ae55bcc7c1f58cb65e +django-auth-ldap==3.0.0 \ + --hash=sha256:19ee19034f344d9efd07ed88d3187e256ec33ae39d6a47222083b89f7d35c5f6 \ + --hash=sha256:1f2d5c562d9ba9a5e9a64099ae9798e1a63840a11afe4d1c4a9c74121f066eaa # via -r requirements/common.in django-bitfield==2.1.0 \ --hash=sha256:158f1056e22cce450d0a49633ea77bfd84b85a2294b1ef03faa775a485f4065d \ diff --git a/version.py b/version.py index b510f55412..556c71c86c 100644 --- a/version.py +++ b/version.py @@ -48,4 +48,4 @@ API_FEATURE_LEVEL = 93 # historical commits sharing the same major version, in which case a # minor version bump suffices. -PROVISION_VERSION = "155.1" +PROVISION_VERSION = "156.0" diff --git a/zproject/backends.py b/zproject/backends.py index 7b42ec7012..5de407d6ee 100644 --- a/zproject/backends.py +++ b/zproject/backends.py @@ -31,7 +31,7 @@ from django.http import HttpRequest, HttpResponse, HttpResponseRedirect from django.shortcuts import render from django.urls import reverse from django.utils.translation import gettext as _ -from django_auth_ldap.backend import LDAPBackend, LDAPReverseEmailSearch, _LDAPUser, ldap_error +from django_auth_ldap.backend import LDAPBackend, _LDAPUser, ldap_error from lxml.etree import XMLSyntaxError from onelogin.saml2.errors import OneLogin_Saml2_Error from onelogin.saml2.response import OneLogin_Saml2_Response @@ -434,13 +434,11 @@ def check_ldap_config() -> None: assert settings.AUTH_LDAP_USERNAME_ATTR and settings.AUTH_LDAP_REVERSE_EMAIL_SEARCH -def find_ldap_users_by_email(email: str) -> Optional[List[_LDAPUser]]: +def find_ldap_users_by_email(email: str) -> List[_LDAPUser]: """ - Returns list of _LDAPUsers matching the email search, - or None if no matches are found. + Returns list of _LDAPUsers matching the email search """ - email_search = LDAPReverseEmailSearch(LDAPBackend(), email) - return email_search.search_for_users(should_populate=False) + return LDAPReverseEmailSearch().search_for_users(email) def email_belongs_to_ldap(realm: Realm, email: str) -> bool: @@ -469,6 +467,46 @@ def email_belongs_to_ldap(realm: Realm, email: str) -> bool: ldap_logger = logging.getLogger("zulip.ldap") +class LDAPReverseEmailSearch(_LDAPUser): + """ + This class is a workaround - we want to use + django-auth-ldap to query the ldap directory for + users with the specified email address, but it doesn't + provide an API for that or an isolated class for handling + the connection. Because connection-handling is tightly integrated + into the _LDAPUser class, we have to make this strange inheritance here, + in order to be able to comfortably have an ldap connection and make search + queries. + + We may be able to get rid of this in the future if we can get + https://github.com/django-auth-ldap/django-auth-ldap/pull/150 merged upstream. + """ + + def __init__(self) -> None: + # Superclass __init__ requires a username argument - it doesn't actually + # impact anything for us in this class, given its very limited use + # for only making a search query, so we pass an empty string. + super().__init__(LDAPBackend(), username="") + + def search_for_users(self, email: str) -> List[_LDAPUser]: + search = settings.AUTH_LDAP_REVERSE_EMAIL_SEARCH + USERNAME_ATTR = settings.AUTH_LDAP_USERNAME_ATTR + + results = search.execute(self.connection, {"email": email}) + + ldap_users = [] + for result in results: + user_dn, user_attrs = result + username = user_attrs[USERNAME_ATTR][0] + ldap_user = _LDAPUser(self.backend, username=username) + ldap_user._user_dn = user_dn + ldap_user._user_attrs = user_attrs + + ldap_users.append(ldap_user) + + return ldap_users + + class ZulipLDAPException(_LDAPUser.AuthenticationFailed): """Since this inherits from _LDAPUser.AuthenticationFailed, these will be caught and logged at debug level inside django-auth-ldap's authenticate()""" @@ -544,7 +582,7 @@ class ZulipLDAPAuthBackendBase(ZulipAuthMixin, LDAPBackend): # We can use find_ldap_users_by_email if is_valid_email(username): email_search_result = find_ldap_users_by_email(username) - if email_search_result is None: + if not email_search_result: result = username elif len(email_search_result) == 1: return email_search_result[0]._username