From 84a5e7e82de4bdf21ca7fd42376585163e7e9aa9 Mon Sep 17 00:00:00 2001 From: Anders Pearson Date: Thu, 2 Jun 2016 14:42:00 +0100 Subject: [PATCH 1/5] add ldap3 version of ldap_lookup --- djangowind/auth.py | 72 +++++++++++++++++++++++++++++++++++ djangowind/tests/test_auth.py | 59 +++++++++++++++++++++++++--- 2 files changed, 126 insertions(+), 5 deletions(-) diff --git a/djangowind/auth.py b/djangowind/auth.py index 254db90..2903e5c 100644 --- a/djangowind/auth.py +++ b/djangowind/auth.py @@ -374,6 +374,78 @@ def _handle_ldap_entry(result_data): return (found, r) +# some of the things we get back are lists, some are strings +def force_list(s): + if type(s) == list: + return s + return [s] + + +def _handle_ldap3_entry(entry): + """ldap returns stuff in a slightly weird format where each entry in + the dict has a list of values with one entry instead of just a + value. convert that to something a little more useful. switch some + field names around while we're at it + """ + + field_maps = [ + ('sn', 'lastname'), + ('givenname', 'firstname'), + ('givenName', 'firstname'), + ('telephoneNumber', 'telephonenumber'), + ] + r = dict() + attributes = entry['attributes'] + for k, v in attributes.items(): + r[k] = ", ".join(force_list(v)) + for a, b in field_maps: + if k == a: + r[b] = r[k] + return r + + +# ldap3 requires that we specifically list the fields that +# we want back. These are all the ones that I can find that +# CU's LDAP server might give us. + +LDAP_ATTRS = [ + 'sn', 'cn', 'givenName', 'telephoneNumber', 'cuMiddlename', + 'departmentNumber', 'objectClass', 'title', 'mail', 'campusphone', + 'uni', 'postalAddress', 'ou', +] + + +def ldap3_lookup(uni=""): + statsd.incr("djangowind.ldap3_lookup") + try: + import ldap3 + except ImportError: + statsd.incr('djangowind.ldap3_lookup.import_failed') + warn("""this requires the python ldap3 library.""") + raise + LDAP_SERVER = "ldap.columbia.edu" + BASE_DN = "o=Columbia University, c=us" + if hasattr(settings, 'LDAP_SERVER'): + LDAP_SERVER = settings.LDAP_SERVER + if hasattr(settings, 'BASE_DN'): + BASE_DN = settings.BASE_DN + baseDN = BASE_DN + searchFilter = "(uni=%s)" % uni + server = ldap3.Server(LDAP_SERVER, get_info=ldap3.ALL) + conn = ldap3.Connection(server, auto_bind=True) + conn.search(baseDN, searchFilter, attributes=LDAP_ATTRS) + results_dict = {'found': False, 'lastname': '', 'firstname': ''} + + if len(conn.response) > 0: + response = conn.response[0] + results_dict.update(_handle_ldap3_entry(response)) + results_dict['found'] = True + + if results_dict['lastname'] == "": + results_dict['lastname'] = uni + return results_dict + + def ldap_lookup(uni=""): statsd.incr('djangowind.ldap_lookup') try: diff --git a/djangowind/tests/test_auth.py b/djangowind/tests/test_auth.py index 2fc0ff4..9c808d8 100644 --- a/djangowind/tests/test_auth.py +++ b/djangowind/tests/test_auth.py @@ -11,11 +11,13 @@ from mock import Mock, patch from django.test import TestCase -from djangowind.auth import validate_wind_ticket, WindAuthBackend -from djangowind.auth import validate_cas2_ticket, CAS2AuthBackend -from djangowind.auth import validate_saml_ticket, SAMLAuthBackend -from djangowind.auth import AffilGroupMapper, StaffMapper, SuperuserMapper -from djangowind.auth import _handle_ldap_entry +from djangowind.auth import ( + validate_wind_ticket, WindAuthBackend, validate_cas2_ticket, + CAS2AuthBackend, validate_saml_ticket, SAMLAuthBackend, + AffilGroupMapper, StaffMapper, SuperuserMapper, + _handle_ldap_entry, _handle_ldap3_entry, +) + from django.contrib.auth.models import User, Group import os.path @@ -810,3 +812,50 @@ def test_handle_ldap_entry(self): self.assertEqual(r[0], True) self.assertEqual( r[1], {'lastname': 'd', 'one': 'a, b, c', 'sn': 'd, e, f'}) + + +class HandleLdap3EntryTest(TestCase): + def test_handle_ldap3_entry_empty(self): + e = dict(attributes=dict()) + r = _handle_ldap3_entry(e) + self.assertEqual(r, dict()) + + def test_handle_ldap3_entry(self): + e = {'dn': u'uni=anp8,ou=People,o=Columbia University,c=US', + 'attributes': { + u'telephoneNumber': [u'+1 212 854 1813'], + u'departmentNumber': [u'1612303'], + u'cuMiddlename': [u'N.'], + u'cn': [u'Anders N. Pearson'], + u'title': [u'Manager, Web Infrastructure'], + u'objectClass': [u'person', u'organizationalPerson', + u'inetOrgPerson', u'cuPerson', + u'cuRestricted', u'eduPerson'], + u'campusphone': [u'MS 4-1813'], + u'sn': [u'Pearson'], + u'uni': u'anp8', + u'mail': [u'anders@columbia.edu'], + u'postalAddress': [u'somewhere'], + u'givenName': [u'Anders'], + u'ou': [u'CU Information Technology']}, + 'raw_attributes': { + u'telephoneNumber': ['+1 212 854 1813'], + u'departmentNumber': ['1612303'], + u'cuMiddlename': ['N.'], + u'cn': ['Anders N. Pearson'], + u'title': ['Manager, Web Infrastructure'], + u'objectClass': ['person', 'organizationalPerson', + 'inetOrgPerson', 'cuPerson', + 'cuRestricted', 'eduPerson'], + u'campusphone': ['MS 4-1813'], + u'sn': ['Pearson'], u'uni': ['anp8'], + u'mail': ['anders@columbia.edu'], + u'postalAddress': ['somewhere'], + u'givenName': ['Anders'], + u'ou': ['CU Information Technology']}, + 'type': 'searchResEntry'} + r = _handle_ldap3_entry(e) + self.assertEqual(r['uni'], 'anp8') + self.assertEqual(r['sn'], 'Pearson') + self.assertEqual(r['firstname'], 'Anders') + self.assertEqual(r['lastname'], 'Pearson') From 105b7e6587b487f43f8bee397ec8468bbb4abbe9 Mon Sep 17 00:00:00 2001 From: Anders Pearson Date: Thu, 2 Jun 2016 14:54:32 +0100 Subject: [PATCH 2/5] CDAPProfileHandler automatically uses the appropriate function --- djangowind/auth.py | 37 ++++++++++++++++++++++++++++++++++--- 1 file changed, 34 insertions(+), 3 deletions(-) diff --git a/djangowind/auth.py b/djangowind/auth.py index 2903e5c..5db0bd0 100644 --- a/djangowind/auth.py +++ b/djangowind/auth.py @@ -446,7 +446,7 @@ def ldap3_lookup(uni=""): return results_dict -def ldap_lookup(uni=""): +def python_ldap_lookup(uni=""): statsd.incr('djangowind.ldap_lookup') try: import ldap @@ -487,14 +487,45 @@ def ldap_lookup(uni=""): class CDAPProfileHandler(object): - """ fills in email, last_name, first_name from CDAP """ + def __init__(self): + self._set_ldap_lookup() + + def _ldap3_lookup(self, uni): + return ldap3_lookup(uni) + + def _python_ldap_lookup(self, uni): + return python_ldap_lookup(uni) + + def _set_ldap_lookup(self): + """ set the ldap lookup method based on what library is available """ + try: + # prefer ldap3 + import ldap3 + self.ldap_lookup = self._ldap3_lookup + return + except ImportError: + pass + + try: + # fallback to python-ldap + import ldap + self.ldap_lookup = self._python_ldap_lookup + except ImportError: + # neither are available + statsd.incr('djangowind.ldap_lookup.import_failed') + warn("""this requires a python ldap library. + you probably need to install 'ldap3', 'python-ldap' or + an equivalent""") + raise + def process(self, user): + """ fills in email, last_name, first_name from LDAP """ statsd.incr('djangowind.cdap.called') if not user.email: user.email = user.username + "@columbia.edu" if not user.last_name or not user.first_name: try: - r = ldap_lookup(user.username) + r = self.ldap_lookup(user.username) if r.get('found', False): statsd.incr('djangowind.cdap.found') user.last_name = r.get('lastname', r.get('sn', '')) From c91ee9be8f5144db0a2b4cfdc11a4c54d5913212 Mon Sep 17 00:00:00 2001 From: Anders Pearson Date: Thu, 2 Jun 2016 15:01:18 +0100 Subject: [PATCH 3/5] do all the importing in one place --- djangowind/auth.py | 51 +++++++++++++++++++--------------------------- 1 file changed, 21 insertions(+), 30 deletions(-) diff --git a/djangowind/auth.py b/djangowind/auth.py index 5db0bd0..38161ed 100644 --- a/djangowind/auth.py +++ b/djangowind/auth.py @@ -23,6 +23,16 @@ except ImportError: from urllib2 import quote +ldap3 = None +ldap = None +try: + import ldap3 +except ImportError: + try: + import ldap + except ImportError: + pass + from django.core.exceptions import ImproperlyConfigured from warnings import warn from django_statsd.clients import statsd @@ -417,12 +427,6 @@ def _handle_ldap3_entry(entry): def ldap3_lookup(uni=""): statsd.incr("djangowind.ldap3_lookup") - try: - import ldap3 - except ImportError: - statsd.incr('djangowind.ldap3_lookup.import_failed') - warn("""this requires the python ldap3 library.""") - raise LDAP_SERVER = "ldap.columbia.edu" BASE_DN = "o=Columbia University, c=us" if hasattr(settings, 'LDAP_SERVER'): @@ -448,15 +452,6 @@ def ldap3_lookup(uni=""): def python_ldap_lookup(uni=""): statsd.incr('djangowind.ldap_lookup') - try: - import ldap - except ImportError: - statsd.incr('djangowind.ldap_lookup.import_failed') - warn("""this requires the python ldap library. -you probably need to install 'python-ldap' (on linux) or -an equivalent""") - raise - LDAP_SERVER = "ldap.columbia.edu" BASE_DN = "o=Columbia University, c=us" if hasattr(settings, 'LDAP_SERVER'): @@ -498,25 +493,21 @@ def _python_ldap_lookup(self, uni): def _set_ldap_lookup(self): """ set the ldap lookup method based on what library is available """ - try: - # prefer ldap3 - import ldap3 + # prefer ldap3 + if ldap3 is not None: self.ldap_lookup = self._ldap3_lookup return - except ImportError: - pass - try: - # fallback to python-ldap - import ldap + # fallback to python-ldap + if ldap is not None: self.ldap_lookup = self._python_ldap_lookup - except ImportError: - # neither are available - statsd.incr('djangowind.ldap_lookup.import_failed') - warn("""this requires a python ldap library. - you probably need to install 'ldap3', 'python-ldap' or - an equivalent""") - raise + return + + # neither are available + statsd.incr('djangowind.ldap_lookup.import_failed') + warn("""this requires a python ldap library. + you probably need to install 'ldap3', 'python-ldap' or + an equivalent""") def process(self, user): """ fills in email, last_name, first_name from LDAP """ From b13d75c0c06827d7c28d142506c7d82325ee4fce Mon Sep 17 00:00:00 2001 From: Anders Pearson Date: Fri, 3 Jun 2016 08:32:31 +0100 Subject: [PATCH 4/5] forgot the default dummy implementation --- djangowind/auth.py | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/djangowind/auth.py b/djangowind/auth.py index 38161ed..01315b0 100644 --- a/djangowind/auth.py +++ b/djangowind/auth.py @@ -485,6 +485,10 @@ class CDAPProfileHandler(object): def __init__(self): self._set_ldap_lookup() + def ldap_lookup(self, uni): + warn("""no ldap library available""") + return dict(found=False, lastname=uni, firstname="") + def _ldap3_lookup(self, uni): return ldap3_lookup(uni) From 465a93e97ff825623917d62da86767182a683320 Mon Sep 17 00:00:00 2001 From: Anders Pearson Date: Fri, 3 Jun 2016 14:03:22 +0100 Subject: [PATCH 5/5] try/catch not necessary any more --- djangowind/auth.py | 21 +++++++++------------ 1 file changed, 9 insertions(+), 12 deletions(-) diff --git a/djangowind/auth.py b/djangowind/auth.py index 01315b0..8ca05e9 100644 --- a/djangowind/auth.py +++ b/djangowind/auth.py @@ -519,18 +519,15 @@ def process(self, user): if not user.email: user.email = user.username + "@columbia.edu" if not user.last_name or not user.first_name: - try: - r = self.ldap_lookup(user.username) - if r.get('found', False): - statsd.incr('djangowind.cdap.found') - user.last_name = r.get('lastname', r.get('sn', '')) - user.first_name = r.get( - 'firstname', - r.get('givenName', '')) - else: - statsd.incr('djangowind.cdap.not_found') - except ImportError: - pass + r = self.ldap_lookup(user.username) + if r.get('found', False): + statsd.incr('djangowind.cdap.found') + user.last_name = r.get('lastname', r.get('sn', '')) + user.first_name = r.get( + 'firstname', + r.get('givenName', '')) + else: + statsd.incr('djangowind.cdap.not_found') user.save()