Merge c3efa62 into 709ece0

CHANGES.rst

@@ -4,6 +4,23 @@ History
 1.0b7 (unreleased)
 ------------------
 
+- Do not catch ``ValueError`` in
+  ``node.ext.ldap._node.LDAPStorage.batched_search``.
+  [rnix]
+
+- Use property decorators for ``node.ext.ldap._node.LDAPStorage.changed``
+  and ``node.ext.ldap.session.LDAPSession.baseDN``.
+  [rnix]
+
+- Fix signature of ``node.ext.ldap.interfaces.ILDAPStorage.search`` to match
+  the actual implementation in ``node.ext.ldap._node.LDAPStorage.search``.
+  [rnix]
+
+- Fix signature of ``node.ext.ldap.ugm.LDAPPrincipals.search`` according to
+  ``node.ext.ugm.interfaces.IPrincipals.search``. The implementation exposed
+  LDAP related arguments and has been renamed to ``raw_search``.
+  [rnix]
+
 - Add ``exists`` property to ``LDAPStorage``.
   [rnix]
 

src/node/ext/ldap/__init__.py

@@ -1,4 +1,5 @@
 # -*- coding: utf-8 -*-
+# do not change import order
 from node.ext.ldap.scope import BASE
 from node.ext.ldap.scope import ONELEVEL
 from node.ext.ldap.scope import SUBTREE

src/node/ext/ldap/_node.py

@@ -56,7 +56,7 @@ def __init__(_next, self, name=None, parent=None):
     @default
     def load(self):
         ldap_node = self.parent
-        # nothong to load
+        # nothing to load
         if not ldap_node.name \
                 or not ldap_node.ldap_session \
                 or ldap_node._action == ACTION_ADD:
@@ -83,10 +83,10 @@ def load(self):
         )
         # result length must be 1
         if len(entry) != 1:
-            raise RuntimeError(                            # pragma NO COVERAGE
-                u"Fatal. Expected entry does not exist "   # pragma NO COVERAGE
-                u"or more than one entry found"            # pragma NO COVERAGE
-            )                                              # pragma NO COVERAGE
+            raise RuntimeError(                            #pragma NO COVERAGE
+                u"Fatal. Expected entry does not exist "   #pragma NO COVERAGE
+                u"or more than one entry found"            #pragma NO COVERAGE
+            )                                              #pragma NO COVERAGE
         # read attributes from result and set to self
         attrs = entry[0][1]
         for key, item in attrs.items():
@@ -140,7 +140,8 @@ def is_multivalued(self, name):
         return name in self.parent.root._multivalued_attributes
 
 
-AttributesBehavior = LDAPAttributesBehavior  # B/C
+# B/C
+AttributesBehavior = LDAPAttributesBehavior
 deprecated('AttributesBehavior', """
 ``node.ext.ldap._node.AttributesBehavior`` is deprecated as of node.ext.ldap
 1.0 and will be removed in node.ext.ldap 1.1. Use
@@ -164,11 +165,8 @@ def __init__(self, name=None, props=None):
         """LDAP Node expects ``name`` and ``props`` arguments for the root LDAP
         Node or nothing for children.
 
-        name
-            Initial base DN for the root LDAP Node.
-
-        props
-            ``node.ext.ldap.LDAPProps`` object.
+        :param name: Initial base DN for the root LDAP Node.
+        :param props: ``node.ext.ldap.LDAPProps`` instance.
         """
         if (name and not props) or (props and not name):
             raise ValueError(u"Wrong initialization.")
@@ -380,11 +378,14 @@ def DN(self):
     def rdn_attr(self):
         return self.name and self.name.split('=')[0] or None
 
-    def _get_changed(self):
+    @property
+    def changed(self):
         return self._changed
 
-    def _set_changed(self, value):
-        """Set the changed flag
+    @default
+    @changed.setter
+    def changed(self, value):
+        """Set the changed flag.
 
         Set:
             - if self.attrs are changed (attrs set us)
@@ -424,8 +425,6 @@ def _set_changed(self, value):
         if self._changed is not oldval and self.parent is not None:
             self.parent.changed = self._changed
 
-    changed = default(property(_get_changed, _set_changed))
-
     @default
     def child_dn(self, key):
         # return child DN for key
@@ -444,7 +443,7 @@ def exists(self):
             )
             # this probably never happens
             if len(res) != 1:
-                raise RuntimeError()
+                raise RuntimeError()                        #pragma NO COVERAGE
             return True
         except NO_SUCH_OBJECT:
             return False
@@ -560,13 +559,10 @@ def batched_search(self, page_size=None, search_func=None, **kw):
         cookie = None
         kw['page_size'] = page_size
         while True:
-            try:
-                kw['cookie'] = cookie
-                matches, cookie = search_func(**kw)
-                for item in matches:
-                    yield item
-            except ValueError:
-                break
+            kw['cookie'] = cookie
+            matches, cookie = search_func(**kw)
+            for item in matches:
+                yield item
             if not cookie:
                 break
 
@@ -631,7 +627,6 @@ def _ldap_modify(self):
         # modifies attributs of self on the ldap directory.
         modlist = list()
         orgin = self.attributes_factory(name='__attrs__', parent=self)
-
         for key in orgin:
             # MOD_DELETE
             if key not in self.attrs:

src/node/ext/ldap/base.py

@@ -5,7 +5,6 @@
 from node.ext.ldap.interfaces import ICacheProviderFactory
 from node.ext.ldap.properties import LDAPProps
 from zope.component import queryUtility
-
 import hashlib
 import ldap
 import logging
@@ -17,14 +16,11 @@
 def testLDAPConnectivity(server=None, port=None, props=None):
     """Function to test the availability of the LDAP Server.
 
-    server
-        Server IP or name
-
-    port
-        LDAP port
-
-    props
-        LDAPProps object. If given, server and port are ignored.
+    :param server: Server IP or name
+    :param port: LDAP port
+    :param props: LDAPProps object. If given, server and port are ignored.
+    :return object: Either string 'success' if connectivity, otherwise ldap
+        error instance.
     """
     if props is None:
         props = LDAPProps(server=server, port=port)
@@ -39,7 +35,10 @@ def testLDAPConnectivity(server=None, port=None, props=None):
 
 
 def md5digest(key):
-    """abbrev to create a md5 hex digest
+    """Abbrev to create a md5 hex digest.
+
+    :param key: Key to create a md5 hex digest for.
+    :return digest: hex digest.
     """
     m = hashlib.md5()
     m.update(key)
@@ -69,7 +68,9 @@ class LDAPConnector(object):
     """
 
     def __init__(self, props=None):
-        """Initialize LDAPConnector.
+        """Initialize LDAP connector.
+
+        :param props: ``LDAPServerProperties`` instance.
         """
         self.protocol = ldap.VERSION3
         self._uri = props.uri
@@ -96,7 +97,7 @@ def bind(self):
         if self._start_tls:
             # ignore in tests for now. nevertheless provide a test environment
             # for TLS and SSL later
-            self._con.start_tls_s()                        # pragma NO COVERAGE
+            self._con.start_tls_s()                        #pragma NO COVERAGE
         self._con.simple_bind_s(self._bindDN, self._bindPW)
         return self._con
 
@@ -116,9 +117,9 @@ class LDAPCommunicator(object):
     """
 
     def __init__(self, connector):
-        """
-        connector
-            LDAPConnector instance.
+        """Initialize LDAP communicator.
+
+        :param connector: ``LDAPConnector`` instance.
         """
         self.baseDN = ''
         self._connector = connector
@@ -162,36 +163,21 @@ def search(self, queryFilter, scope, baseDN=None,
                page_size=None, cookie=None):
         """Search the directory.
 
-        queryFilter
-            LDAP query filter
-
-        scope
-            LDAP search scope
-
-        baseDN
-            Search base. Defaults to ``self.baseDN``
-
-        force_reload
-            Force reload of result if cache enabled.
-
-        attrlist
-            LDAP attrlist to query.
-
-        attrsonly
-            Flag whether to return only attribute names, without corresponding
-            values.
-
-        page_size
-            Number of items per page, when doing pagination.
-
-        cookie
-            Cookie string returned by previous search with pagination.
+        :param queryFilter: LDAP query filter
+        :param scope: LDAP search scope
+        :param baseDN: Search base. Defaults to ``self.baseDN``
+        :param force_reload: Force reload of result if cache enabled.
+        :param attrlist: LDAP attrlist to query.
+        :param attrsonly: Flag whether to return only attribute names, without
+            corresponding values.
+        :param page_size: Number of items per page, when doing pagination.
+        :param cookie: Cookie string returned by previous search with
+            pagination.
         """
         if baseDN is None:
             baseDN = self.baseDN
             if not baseDN:
                 raise ValueError(u"baseDN unset.")
-
         if page_size:
             if cookie is None:
                 cookie = ''
@@ -209,7 +195,6 @@ def _search(baseDN, scope, queryFilter,
             # in case we do pagination of results
             if type(attrlist) in (list, tuple):
                 attrlist = [str(_) for _ in attrlist]
-
             msgid = self._con.search_ext(
                 baseDN,
                 scope,
@@ -225,7 +210,6 @@ def _search(baseDN, scope, queryFilter,
                 return results, pctrls[0].cookie
             else:
                 return results
-
         args = [baseDN, scope, queryFilter, attrlist, attrsonly, serverctrls]
         if self._cache:
             key_items = [
@@ -251,11 +235,8 @@ def _search(baseDN, scope, queryFilter,
     def add(self, dn, data):
         """Insert an entry into directory.
 
-        dn
-            adding DN
-
-        data
-            dict containing key/value pairs of entry attributes
+        :param dn: Adding DN
+        :param data: Dict containing key/value pairs of entry attributes
         """
         attributes = [(k, v) for k, v in data.items()]
         self._con.add_s(dn, attributes)

src/node/ext/ldap/base.rst

@@ -28,7 +28,7 @@ LDAP credentials::
     ...     server=host,
     ...     port=port,
     ...     user=binddn,
-    ...     password=bindpw,
+    ...     password=bindpw
     ... )
 
 Test main script, could be used by command line with
@@ -94,6 +94,31 @@ Set base dn and check if previously imported entries are present.::
     >>> len(res)
     7
 
+Test search pagination::
+
+    >>> res, cookie = communicator.search(
+    ...     '(objectClass=*)', SUBTREE, page_size=4, cookie='')
+    >>> len(res)
+    4
+
+    >>> res, cookie = communicator.search(
+    ...     '(objectClass=*)', SUBTREE, page_size=4, cookie=cookie)
+
+    >>> len(res)
+    3
+
+    >>> cookie
+    ''
+
+Pagination search fails if cookie but no page size given::
+
+    >>> res, cookie = communicator.search(
+    ...     '(objectClass=*)', SUBTREE, page_size=4, cookie='')
+    >>> communicator.search('(objectClass=*)', SUBTREE, cookie=cookie)
+    Traceback (most recent call last):
+      ...
+    ValueError: cookie passed without page_size
+
 Test inserting entries.::
 
     >>> entry = {

src/node/ext/ldap/filter.py

@@ -58,8 +58,7 @@ def __or__(self, other):
         return LDAPFilter(res)
 
     def __contains__(self, attr):
-        attr = '(%s=' % (attr,)
-        return attr in self._filter
+        return self._filter.find('({}='.format(attr)) > -1
 
     def __str__(self):
         return self._filter and self._filter or ''
@@ -100,34 +99,21 @@ def __str__(self):
         """turn relation string into ldap filter string
         """
         dictionary = dict()
-
         parsedRelation = dict()
         for pair in self.relation.split('|'):
             k, _, v = pair.partition(':')
             if k not in parsedRelation:
                 parsedRelation[k] = list()
             parsedRelation[k].append(v)
-
         existing = [x for x in self.gattrs]
         for k, vals in parsedRelation.items():
             for v in vals:
-                if (
-                    str(v) == '' or
+                if (str(v) == '' or
                     str(k) == '' or
-                    str(k) not in existing
-                ):
+                    str(k) not in existing):
                     continue
                 dictionary[str(v)] = self.gattrs[str(k)]
-
         self.dictionary = dictionary
-
-        # this "if/else creates an unused result and has no effect
-        # _filter = LDAPFilter()
-        # if len(dictionary) is 1:
-        #    _filter = LDAPFilter(self.relation)
-        # else:
-        #     _filter = dict_to_filter(parsedRelation, self.or_search)
-
         if self.dictionary:
             return str(dict_to_filter(self.dictionary, self.or_search))
         return ''