From 99672adec869fd7cb967cccb17c52db4c28cf5bf Mon Sep 17 00:00:00 2001 From: Benjamin Dauvergne Date: Tue, 25 Jun 2019 23:23:11 +0200 Subject: [PATCH] ldap: do not block check_password and get_attributes if LDAP is down (#34316) --- src/authentic2/attributes_ng/sources/ldap.py | 2 +- src/authentic2/backends/ldap_backend.py | 45 ++++++++++++++------ tests/test_ldap.py | 42 ++++++++++++++++++ 3 files changed, 76 insertions(+), 13 deletions(-) diff --git a/src/authentic2/attributes_ng/sources/ldap.py b/src/authentic2/attributes_ng/sources/ldap.py index 23de4b6e..bc095b1d 100644 --- a/src/authentic2/attributes_ng/sources/ldap.py +++ b/src/authentic2/attributes_ng/sources/ldap.py @@ -38,5 +38,5 @@ def get_dependencies(instance, ctx): def get_attributes(instance, ctx): user = ctx.get('user') if user and isinstance(user, LDAPUser): - ctx.update(user.get_attributes()) + ctx.update(user.get_attributes(instance, ctx)) return ctx diff --git a/src/authentic2/backends/ldap_backend.py b/src/authentic2/backends/ldap_backend.py index 47485c79..839d5079 100644 --- a/src/authentic2/backends/ldap_backend.py +++ b/src/authentic2/backends/ldap_backend.py @@ -14,6 +14,8 @@ # You should have received a copy of the GNU Affero General Public License # along with this program. If not, see . +import hashlib + try: import ldap import ldap.modlist @@ -35,6 +37,7 @@ import os # code originaly copied from by now merely inspired by # http://www.amherst.k12.oh.us/django-ldap.html +from django.core.cache import cache from django.core.exceptions import ImproperlyConfigured from django.conf import settings from django.contrib.auth import get_user_model @@ -70,6 +73,7 @@ CA_BUNDLE_PATHS = [ '/var/lib/ca-certificates/ca-bundle.pem', # OpenSuse ] + # Select a system certificate store for bundle_path in CA_BUNDLE_PATHS: if os.path.exists(bundle_path): @@ -327,15 +331,17 @@ class LDAPUser(User): def check_password(self, raw_password): connection = self.ldap_backend.get_connection(self.block) - try: - connection.simple_bind_s(self.dn, raw_password) - except ldap.INVALID_CREDENTIALS: - return False - except ldap.LDAPError as e: - log.error('LDAPUser.check_password() failed: %r', e) - return False - self._current_password = raw_password - return True + if connection: + try: + connection.simple_bind_s(self.dn, raw_password) + self._current_password = raw_password + return True + except ldap.INVALID_CREDENTIALS: + return False + except ldap.LDAPError as e: + log.warning('ldap: check_password failed, %r', e) + log.warning('ldap: check_password failed, could not get a connection') + return False def set_password(self, new_password): # Allow change password to work in all cases, as the form does a check_password() first @@ -343,6 +349,9 @@ class LDAPUser(User): _current_password = getattr(self, '_current_password', None) or self.get_password_in_session() if _current_password != new_password: conn = self.get_connection() + if not conn: + log.warning('ldap: set_password failed, could not get a connection') + return self.ldap_backend.modify_password(conn, self.block, self.dn, _current_password, new_password) self._current_password = new_password self.keep_password_in_session(new_password) @@ -365,9 +374,18 @@ class LDAPUser(User): self.ldap_backend.update_default(self.block, validate=False) return self.ldap_backend.get_connection(self.block, credentials=credentials) - def get_attributes(self): + def get_attributes(self, attribute_source, ctx): + cache_key = hashlib.md5((force_text(str(self.pk)) + ';' + force_text(self.dn)).encode('utf-8')).hexdigest() conn = self.get_connection() - return self.ldap_backend.get_ldap_attributes(self.block, conn, self.dn) or {} + # prevents blocking on temporary LDAP failures + if conn is not None: + attributes = self.ldap_backend.get_ldap_attributes(self.block, conn, self.dn) or {} + # keep attributes in cache for 8 hours + cache.set(cache_key, attributes, 3600 * 8) + return attributes + else: + log.warning('ldap: get_attributes failed, could not get a connection') + return cache.get(cache_key, {}) def save(self, *args, **kwargs): if hasattr(self, 'keep_pk'): @@ -1284,7 +1302,7 @@ class LDAPBackend(object): '''Try to get at least one connection''' for conn in cls.get_connections(block, credentials=credentials): return conn - log.error('could not get a connection') + return None @classmethod def update_default(cls, block, validate=True): @@ -1387,6 +1405,9 @@ class LDAPBackendPasswordLost(LDAPBackend): continue for external_id_tuple in map_text(block['external_id_tuples']): conn = self.ldap_backend.get_connection(block) + if not conn: + log.warning('ldap: password-lost authenticate failed, could not get a connection') + continue try: if external_id_tuple == ('dn:noquote',): dn = external_id diff --git a/tests/test_ldap.py b/tests/test_ldap.py index c7a54a36..05b0d167 100644 --- a/tests/test_ldap.py +++ b/tests/test_ldap.py @@ -48,6 +48,7 @@ UID = 'etienne.michu' CN = 'Étienne Michu' DN = 'cn=%s,o=ôrga' % escape_dn_chars(CN) PASS = 'passé' +UPASS = u'passé' EMAIL = 'etienne.michu@example.net' base_dir = os.path.dirname(__file__) @@ -836,3 +837,44 @@ def test_alert_on_wrong_user_filter(slapd, settings, client, db, caplog): response = client.post('/login/', {'login-password-submit': '1', 'username': USERNAME, 'password': PASS}, follow=True) + + +def test_get_attributes(slapd, settings, db, rf): + settings.LDAP_AUTH_SETTINGS = [{ + 'url': [slapd.ldap_url], + 'basedn': u'o=ôrga', + 'use_tls': False, + }] + user = authenticate(username=USERNAME, password=UPASS) + assert user + assert user.get_attributes(object(), {}) == { + 'dn': u'cn=\xc9tienne Michu,o=\xf4rga', + 'givenname': [u'\xc9tienne'], + 'mail': [u'etienne.michu@example.net'], + 'sn': [u'Michu'], + 'uid': [u'etienne.michu'], + } + # simulate LDAP down + slapd.stop() + assert user.get_attributes(object(), {}) == { + 'dn': u'cn=\xc9tienne Michu,o=\xf4rga', + 'givenname': [u'\xc9tienne'], + 'mail': [u'etienne.michu@example.net'], + 'sn': [u'Michu'], + 'uid': [u'etienne.michu'], + } + assert not user.check_password(UPASS) + # simulate LDAP come back up + slapd.start() + assert user.check_password(UPASS) + # modify LDAP record and check attributes are updated + conn = slapd.get_connection_admin() + ldif = [(ldap.MOD_REPLACE, 'sn', ['Micho'])] + conn.modify_s(DN, ldif) + assert user.get_attributes(object(), {}) == { + 'dn': u'cn=\xc9tienne Michu,o=\xf4rga', + 'givenname': [u'\xc9tienne'], + 'mail': [u'etienne.michu@example.net'], + 'sn': [u'Micho'], + 'uid': [u'etienne.michu'], + } -- 2.20.1