From be68f9bf30783303ad35741ba8a6f17039eadedc Mon Sep 17 00:00:00 2001 From: Charles Duffy Date: Tue, 23 Apr 2013 14:57:49 -0500 Subject: [PATCH 1/6] Allow attribute filter to be passed on queries, use during group lookups Fixes: #7 --- pyramid_ldap/__init__.py | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/pyramid_ldap/__init__.py b/pyramid_ldap/__init__.py index ba56ea6..380abd7 100644 --- a/pyramid_ldap/__init__.py +++ b/pyramid_ldap/__init__.py @@ -58,10 +58,12 @@ def query_cache(self, cache_key): return result def execute(self, conn, **kw): + attrlist = kw.get('attrlist') cache_key = ( bytes_(self.base_dn % kw, 'utf-8'), self.scope, - bytes_(self.filter_tmpl % kw, 'utf-8') + bytes_(self.filter_tmpl % kw, 'utf-8'), + tuple(attrlist) if isinstance(attrlist, list) else attrlist, ) logger.debug('searching for %r' % (cache_key,)) @@ -128,7 +130,7 @@ def authenticate(self, login, password): exc_info=True) return None - def user_groups(self, userdn): + def user_groups(self, userdn, attrlist=None): """ Given a user DN, return a sequence of LDAP attribute dictionaries matching the groups of which the DN is a member. If the DN does not exist, return ``None``. @@ -150,7 +152,7 @@ def user_groups(self, userdn): raise ConfigurationError( 'set_ldap_groups_query was not called during setup') try: - result = search.execute(conn, userdn=userdn) + result = search.execute(conn, userdn=userdn, attrlist=attrlist) return _ldap_decode(result) except ldap.LDAPError: logger.debug( @@ -279,7 +281,7 @@ def groupfinder(userdn, request): principal in the list of results; if the user does not exist, it returns None.""" connector = get_ldap_connector(request) - group_list = connector.user_groups(userdn) + group_list = connector.user_groups(userdn, attrlist=['cn']) if group_list is None: return None group_dns = [] From 81d5cdb143dea9423f7db7d15a6ceb2a940590aa Mon Sep 17 00:00:00 2001 From: Charles Duffy Date: Wed, 24 Apr 2013 08:17:56 -0500 Subject: [PATCH 2/6] Fix up unit tests for new cache format --- pyramid_ldap/tests.py | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/pyramid_ldap/tests.py b/pyramid_ldap/tests.py index 72d8b4c..751deff 100644 --- a/pyramid_ldap/tests.py +++ b/pyramid_ldap/tests.py @@ -212,19 +212,19 @@ def test_execute_no_cache_period(self): conn = DummyConnection('abc') result = inst.execute(conn, login='foo') self.assertEqual(result, 'abc') - self.assertEqual(conn.arg, ('foo', None, 'foo')) + self.assertEqual(conn.arg, ('foo', None, 'foo', None)) def test_execute_with_cache_period_miss(self): inst = self._makeOne('%(login)s', '%(login)s', None, 1) conn = DummyConnection('abc') result = inst.execute(conn, login='foo') self.assertEqual(result, 'abc') - self.assertEqual(conn.arg, ('foo', None, 'foo')) + self.assertEqual(conn.arg, ('foo', None, 'foo', None)) def test_execute_with_cache_period_hit(self): inst = self._makeOne('%(login)s', '%(login)s', None, 1) inst.last_timeslice = sys.maxint - inst.cache[('foo', None, 'foo')] = 'def' + inst.cache[('foo', None, 'foo', None)] = 'def' conn = DummyConnection('abc') result = inst.execute(conn, login='foo') self.assertEqual(result, 'def') @@ -234,7 +234,7 @@ def __init__(self, dn, group_list): self.dn = dn self.group_list = group_list - def user_groups(self, dn): + def user_groups(self, dn, attrlist=None): return self.group_list class Dummy(object): From 9f7c47a48c0232c11598bc00f344aab67f8d8fe8 Mon Sep 17 00:00:00 2001 From: Charles Duffy Date: Wed, 24 Apr 2013 08:33:42 -0500 Subject: [PATCH 3/6] Normalize attrlist ordering, add test case for same --- pyramid_ldap/__init__.py | 2 +- pyramid_ldap/tests.py | 9 ++++++++- 2 files changed, 9 insertions(+), 2 deletions(-) diff --git a/pyramid_ldap/__init__.py b/pyramid_ldap/__init__.py index 380abd7..a864cec 100644 --- a/pyramid_ldap/__init__.py +++ b/pyramid_ldap/__init__.py @@ -63,7 +63,7 @@ def execute(self, conn, **kw): bytes_(self.base_dn % kw, 'utf-8'), self.scope, bytes_(self.filter_tmpl % kw, 'utf-8'), - tuple(attrlist) if isinstance(attrlist, list) else attrlist, + tuple(sorted(attrlist)) if isinstance(attrlist, list) else attrlist, ) logger.debug('searching for %r' % (cache_key,)) diff --git a/pyramid_ldap/tests.py b/pyramid_ldap/tests.py index 751deff..0083ed0 100644 --- a/pyramid_ldap/tests.py +++ b/pyramid_ldap/tests.py @@ -207,6 +207,13 @@ def test_query_cache_with_rollover(self): self.assertEqual(inst.cache, {}) self.assertNotEqual(inst.last_timeslice, 0) + def test_execute_cache_with_filter(self): + inst = self._makeOne('%(login)s', '%(login)s', None, 0) + conn = DummyConnection('abc') + result = inst.execute(conn, login='foo', attrlist=['foo', 'bar']) + self.assertEqual(result, 'abc') + self.assertEqual(conn.arg, ('foo', None, 'foo', ('bar', 'foo'))) + def test_execute_no_cache_period(self): inst = self._makeOne('%(login)s', '%(login)s', None, 0) conn = DummyConnection('abc') @@ -228,7 +235,7 @@ def test_execute_with_cache_period_hit(self): conn = DummyConnection('abc') result = inst.execute(conn, login='foo') self.assertEqual(result, 'def') - + class DummyLDAPConnector(object): def __init__(self, dn, group_list): self.dn = dn From 2b70a23765302548ae61647422ab09ec119c69b9 Mon Sep 17 00:00:00 2001 From: Charles Duffy Date: Wed, 24 Apr 2013 12:49:05 -0500 Subject: [PATCH 4/6] issues/7: Update per review feedback --- pyramid_ldap/__init__.py | 28 +++++++++++++++++----------- pyramid_ldap/tests.py | 11 +++-------- 2 files changed, 20 insertions(+), 19 deletions(-) diff --git a/pyramid_ldap/__init__.py b/pyramid_ldap/__init__.py index a864cec..e87f016 100644 --- a/pyramid_ldap/__init__.py +++ b/pyramid_ldap/__init__.py @@ -27,17 +27,22 @@ def __init__(self, *arg, **kw): class _LDAPQuery(object): """ Represents an LDAP query. Provides rudimentary in-RAM caching of query results.""" - def __init__(self, base_dn, filter_tmpl, scope, cache_period): + def __init__(self, base_dn, filter_tmpl, scope, attrlist, cache_period): self.base_dn = base_dn self.filter_tmpl = filter_tmpl self.scope = scope + if attrlist is not None: + self.attrlist = tuple(sorted(attrlist)) + else: + self.attrlist = None self.cache_period = cache_period self.last_timeslice = 0 self.cache = {} def __str__(self): return ('base_dn=%(base_dn)s, filter_tmpl=%(filter_tmpl)s, ' - 'scope=%(scope)s, cache_period=%(cache_period)s' % + 'scope=%(scope)s, attrlist=%(attrlist)r, ' + 'cache_period=%(cache_period)s' % self.__dict__) def query_cache(self, cache_key): @@ -58,12 +63,11 @@ def query_cache(self, cache_key): return result def execute(self, conn, **kw): - attrlist = kw.get('attrlist') cache_key = ( bytes_(self.base_dn % kw, 'utf-8'), self.scope, bytes_(self.filter_tmpl % kw, 'utf-8'), - tuple(sorted(attrlist)) if isinstance(attrlist, list) else attrlist, + self.attrlist, ) logger.debug('searching for %r' % (cache_key,)) @@ -130,7 +134,7 @@ def authenticate(self, login, password): exc_info=True) return None - def user_groups(self, userdn, attrlist=None): + def user_groups(self, userdn): """ Given a user DN, return a sequence of LDAP attribute dictionaries matching the groups of which the DN is a member. If the DN does not exist, return ``None``. @@ -152,7 +156,7 @@ def user_groups(self, userdn, attrlist=None): raise ConfigurationError( 'set_ldap_groups_query was not called during setup') try: - result = search.execute(conn, userdn=userdn, attrlist=attrlist) + result = search.execute(conn, userdn=userdn) return _ldap_decode(result) except ldap.LDAPError: logger.debug( @@ -161,7 +165,8 @@ def user_groups(self, userdn, attrlist=None): return None def ldap_set_login_query(config, base_dn, filter_tmpl, - scope=ldap.SCOPE_ONELEVEL, cache_period=0): + scope=ldap.SCOPE_ONELEVEL, cache_period=0, + attrlist=None): """ Configurator method to set the LDAP login search. ``base_dn`` is the DN at which to begin the search. ``filter_tmpl`` is a string which can be used as an LDAP filter: it should contain the replacement value @@ -181,7 +186,7 @@ def ldap_set_login_query(config, base_dn, filter_tmpl, The registered search must return one and only one value to be considered a valid login. """ - query = _LDAPQuery(base_dn, filter_tmpl, scope, cache_period) + query = _LDAPQuery(base_dn, filter_tmpl, scope, attrlist, cache_period) def register(): config.registry.ldap_login_query = query @@ -195,7 +200,8 @@ def register(): config.action('ldap-set-login-query', register, introspectables=(intr,)) def ldap_set_groups_query(config, base_dn, filter_tmpl, - scope=ldap.SCOPE_SUBTREE, cache_period=0): + scope=ldap.SCOPE_SUBTREE, cache_period=0, + attrlist=('',)): """ Configurator method to set the LDAP groups search. ``base_dn`` is the DN at which to begin the search. ``filter_tmpl`` is a string which can be used as an LDAP filter: it should contain the replacement value @@ -213,7 +219,7 @@ def ldap_set_groups_query(config, base_dn, filter_tmpl, ) """ - query = _LDAPQuery(base_dn, filter_tmpl, scope, cache_period) + query = _LDAPQuery(base_dn, filter_tmpl, scope, attrlist, cache_period) def register(): config.registry.ldap_groups_query = query intr = config.introspectable( @@ -281,7 +287,7 @@ def groupfinder(userdn, request): principal in the list of results; if the user does not exist, it returns None.""" connector = get_ldap_connector(request) - group_list = connector.user_groups(userdn, attrlist=['cn']) + group_list = connector.user_groups(userdn) if group_list is None: return None group_dns = [] diff --git a/pyramid_ldap/tests.py b/pyramid_ldap/tests.py index 0083ed0..84c9cfd 100644 --- a/pyramid_ldap/tests.py +++ b/pyramid_ldap/tests.py @@ -118,6 +118,7 @@ def test_it_defaults(self): self._callFUT(config, 'dn', 'tmpl') self.assertEqual(config.registry.ldap_groups_query.base_dn, 'dn') self.assertEqual(config.registry.ldap_groups_query.filter_tmpl, 'tmpl') + self.assertEqual(config.registry.ldap_groups_query.attrlist, ('',)) self.assertEqual(config.registry.ldap_groups_query.scope, ldap.SCOPE_SUBTREE) self.assertEqual(config.registry.ldap_groups_query.cache_period, 0) @@ -133,6 +134,7 @@ def test_it_defaults(self): self._callFUT(config, 'dn', 'tmpl') self.assertEqual(config.registry.ldap_login_query.base_dn, 'dn') self.assertEqual(config.registry.ldap_login_query.filter_tmpl, 'tmpl') + self.assertEqual(config.registry.ldap_login_query.attrlist, None) self.assertEqual(config.registry.ldap_login_query.scope, ldap.SCOPE_ONELEVEL) self.assertEqual(config.registry.ldap_login_query.cache_period, 0) @@ -192,7 +194,7 @@ def test_user_groups_execute_raises(self): class Test_LDAPQuery(unittest.TestCase): def _makeOne(self, base_dn, filter_tmpl, scope, cache_period): from pyramid_ldap import _LDAPQuery - return _LDAPQuery(base_dn, filter_tmpl, scope, cache_period) + return _LDAPQuery(base_dn, filter_tmpl, scope, None, cache_period) def test_query_cache_no_rollover(self): inst = self._makeOne(None, None, None, 1) @@ -207,13 +209,6 @@ def test_query_cache_with_rollover(self): self.assertEqual(inst.cache, {}) self.assertNotEqual(inst.last_timeslice, 0) - def test_execute_cache_with_filter(self): - inst = self._makeOne('%(login)s', '%(login)s', None, 0) - conn = DummyConnection('abc') - result = inst.execute(conn, login='foo', attrlist=['foo', 'bar']) - self.assertEqual(result, 'abc') - self.assertEqual(conn.arg, ('foo', None, 'foo', ('bar', 'foo'))) - def test_execute_no_cache_period(self): inst = self._makeOne('%(login)s', '%(login)s', None, 0) conn = DummyConnection('abc') From 6c8ceb1fba2504276533a44d00f05012af10184c Mon Sep 17 00:00:00 2001 From: Charles Duffy Date: Fri, 3 May 2013 09:28:11 -0500 Subject: [PATCH 5/6] Update CONTRIBUTORS; Indeed.com agrees to the CA --- CONTRIBUTORS.txt | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/CONTRIBUTORS.txt b/CONTRIBUTORS.txt index 80fbb23..84bb893 100644 --- a/CONTRIBUTORS.txt +++ b/CONTRIBUTORS.txt @@ -103,4 +103,4 @@ Contributors ------------ - Chris McDonough, 2013/04/24 - +- Charles Duffy (Indeed.com), 2013/05/03 From b9a7200312b575e98ee9fb9b81965ad31634a0ab Mon Sep 17 00:00:00 2001 From: Charles Duffy Date: Mon, 6 May 2013 09:13:56 -0500 Subject: [PATCH 6/6] Update docs, remove change to pyramid_ldap/tests from prior rev --- CHANGES.txt | 7 +++++++ docs/index.rst | 8 +++++--- pyramid_ldap/tests.py | 2 +- 3 files changed, 13 insertions(+), 4 deletions(-) diff --git a/CHANGES.txt b/CHANGES.txt index fa53044..0a40619 100644 --- a/CHANGES.txt +++ b/CHANGES.txt @@ -1,3 +1,10 @@ +Development +----------- + +- Add attrlist to LDAPQuery, set_login_query, set_groups_query to allow + retrieved attributes to be filtered. (Default: All attributes on users, no + attributes on groups). + 0.1 --- diff --git a/docs/index.rst b/docs/index.rst index fc4a475..61ab9dc 100644 --- a/docs/index.rst +++ b/docs/index.rst @@ -95,7 +95,8 @@ the startup phase of your Pyramid application. ``Configurator.ldap_set_login_query`` This configurator method accepts parameters which tell ``pyramid_ldap`` - how to find a user based on a login. Invoking this method allows the LDAP + how to find a user based on a login, and which LDAP attributes to retrieve + during user lookups. Invoking this method allows the LDAP connector's ``authenticate`` method to work. See :func:`pyramid_ldap.ldap_set_login_query` for argument details. @@ -105,8 +106,9 @@ the startup phase of your Pyramid application. ``Configurator.ldap_set_groups_query`` This configurator method accepts parameters which tell ``pyramid_ldap`` - how to find groups based on a user DN. Invoking this method allows the - connector's ``user_groups`` method to work. See + how to find groups based on a user DN, and which group attributes to + retrieve during lookups. Invoking this method allows the connector's + ``user_groups`` method to work. See :func:`pyramid_ldap.ldap_set_groups_query` for argument details. If ``ldap_set_groups_query`` is not called, the diff --git a/pyramid_ldap/tests.py b/pyramid_ldap/tests.py index 84c9cfd..6612048 100644 --- a/pyramid_ldap/tests.py +++ b/pyramid_ldap/tests.py @@ -236,7 +236,7 @@ def __init__(self, dn, group_list): self.dn = dn self.group_list = group_list - def user_groups(self, dn, attrlist=None): + def user_groups(self, dn): return self.group_list class Dummy(object):