On 8.11.2013 17:56, Petr Viktorin wrote:
Patch 198:

Also update ipaldap's find_entries docstring, it no longer uses IPA
defaults.

Done.



While you're touching this part of code, I had some other improvements
in mind -- you can consider them:

In find_entries,
     attrs_list = [a.lower() for a in attrs_list]
to make sure 'memberindirect' is case insensitive

Done.


In get_memberof, construct `indirect` as a set, for Ο(1) remove().
^ ignore that, it's nuked in 201 \o/

Changing MEMBERS_ALL et.al. from numbers to descriptive strings, for
easier debugging.
^ these can be removed entirely in 201

Removed.




Patch 199: Looks great


Patch 200:

objtype, res_list, red_id, res_ctrls = result
Minor typo ----------^

Fixed.



This construction won't work as you'd expect in Python 2:

try:
     (possibly raise interesting exception) (*)
except:
     try:
         (possibly raise exception to ignore) (**)
     except:
         pass
     raise  # (***)

The problem is that the exception in (**) overwrites the "current active
exception" raised in (*). In (***) the exception from the cleanup will be
re-raised.
The solution is to store the wanted exception info, including the
traceback:
     exc_type, exc_value, exc_traceback = sys.exc_info()
and then re-raise explicitly:
     raise exc_type, exc_value, exc_traceback

Turns out LDAPError does not fly well with sys.exc_info() (all exception data is lost, probably a python-ldap bug), so I used "except LDAPError, e: ... raise e" instead.


Also, please log the ignored exception from cancelling the paged search.

Done.





Patch 201:
Great patch!
A nitpick, I'd rename _process_member{,of} to _process_member{,of}indirect

Renamed.



Patch 202: Looks good
While we're on the subject: Each Plugin has an "api" attribute. It would
be nice if we started preferring `self.api` instead of the global
singleton wherever possible, even though they're currently always the same.

+1, fixed.

Updated patches attached.

Honza

--
Jan Cholasta
>From 6b0826c37445aab09b6821c121d56e435ae39a0a Mon Sep 17 00:00:00 2001
From: Jan Cholasta <jchol...@redhat.com>
Date: Thu, 31 Oct 2013 11:47:53 +0000
Subject: [PATCH 1/5] Move IPA specific code from LDAPClient to the ldap2
 plugin.

https://fedorahosted.org/freeipa/ticket/3971
---
 ipapython/ipaldap.py       | 219 ++-------------------------------------------
 ipaserver/plugins/ldap2.py | 204 +++++++++++++++++++++++++++++++++++++++++
 2 files changed, 211 insertions(+), 212 deletions(-)

diff --git a/ipapython/ipaldap.py b/ipapython/ipaldap.py
index 027bfa9..7bc4cd9 100644
--- a/ipapython/ipaldap.py
+++ b/ipapython/ipaldap.py
@@ -47,11 +47,6 @@ DEFAULT_TIMEOUT = 10
 DN_SYNTAX_OID = '1.3.6.1.4.1.1466.115.121.1.12'
 _debug_log_ldap = False
 
-# Group Member types
-MEMBERS_ALL = 0
-MEMBERS_DIRECT = 1
-MEMBERS_INDIRECT = 2
-
 _missing = object()
 
 
@@ -1084,13 +1079,6 @@ class LDAPClient(object):
     def _init_connection(self):
         self.conn = None
 
-    def get_api(self):
-        """Return the API if available, otherwise None
-
-        May be overridden in a subclass.
-        """
-        return None
-
     @contextlib.contextmanager
     def error_handler(self, arg_desc=None):
         """Context manager that handles LDAPErrors
@@ -1396,9 +1384,9 @@ class LDAPClient(object):
         attrs_list -- list of attributes to return, all if None (default None)
         base_dn -- dn of the entry at which to start the search (default '')
         scope -- search scope, see LDAP docs (default ldap2.SCOPE_SUBTREE)
-        time_limit -- time limit in seconds (default use IPA config values)
+        time_limit -- time limit in seconds (default unlimited)
         size_limit -- size (number of entries returned) limit
-            (default use IPA config values)
+            (default unlimited)
         search_refs -- allow search references to be returned
             (default skips these entries)
         """
@@ -1410,21 +1398,17 @@ class LDAPClient(object):
         res = []
         truncated = False
 
-        if time_limit is None or size_limit is None:
-            config = self.get_ipa_config()
-            if time_limit is None:
-                time_limit = config.get('ipasearchtimelimit', [-1])[0]
-            if size_limit is None:
-                size_limit = config.get('ipasearchrecordslimit', [0])[0]
-        if time_limit == 0:
-            time_limit = -1
+        if time_limit is None or time_limit == 0:
+            time_limit = -1.0
+        if size_limit is None:
+            size_limit = 0
         if not isinstance(size_limit, int):
             size_limit = int(size_limit)
         if not isinstance(time_limit, float):
             time_limit = float(time_limit)
 
         if attrs_list:
-            attrs_list = list(set(attrs_list))
+            attrs_list = [a.lower() for a in set(attrs_list)]
 
         # pass arguments to python-ldap
         with self.error_handler():
@@ -1448,37 +1432,6 @@ class LDAPClient(object):
         if not res and not truncated:
             raise errors.NotFound(reason='no such entry')
 
-        if attrs_list and (
-                'memberindirect' in attrs_list or '*' in attrs_list):
-            for r in res:
-                if not 'member' in r[1]:
-                    continue
-                else:
-                    members = r[1]['member']
-                    indirect = self.get_members(
-                        r[0], members, membertype=MEMBERS_INDIRECT,
-                        time_limit=time_limit, size_limit=size_limit)
-                    if len(indirect) > 0:
-                        r[1]['memberindirect'] = indirect
-        if attrs_list and (
-                'memberofindirect' in attrs_list or '*' in attrs_list):
-            for r in res:
-                if 'memberof' in r[1]:
-                    memberof = r[1]['memberof']
-                    del r[1]['memberof']
-                elif 'memberOf' in r[1]:
-                    memberof = r[1]['memberOf']
-                    del r[1]['memberOf']
-                else:
-                    continue
-                direct, indirect = self.get_memberof(
-                    r[0], memberof, time_limit=time_limit,
-                    size_limit=size_limit)
-                if len(direct) > 0:
-                    r[1]['memberof'] = direct
-                if len(indirect) > 0:
-                    r[1]['memberofindirect'] = indirect
-
         return (res, truncated)
 
     def find_entry_by_attr(self, attr, value, object_class, attrs_list=None,
@@ -1527,164 +1480,6 @@ class LDAPClient(object):
             raise errors.LimitsExceeded()
         return entry[0]
 
-    def get_ipa_config(self, attrs_list=None):
-        """Returns the IPA configuration entry.
-
-        Overriden in the subclasses that have access to IPA configuration.
-        """
-        return {}
-
-    def get_memberof(self, entry_dn, memberof, time_limit=None,
-                     size_limit=None):
-        """
-        Examine the objects that an entry is a member of and determine if they
-        are a direct or indirect member of that group.
-
-        entry_dn: dn of the entry we want the direct/indirect members of
-        memberof: the memberOf attribute for entry_dn
-
-        Returns two memberof lists: (direct, indirect)
-        """
-
-        assert isinstance(entry_dn, DN)
-
-        self.log.debug(
-            "get_memberof: entry_dn=%s memberof=%s", entry_dn, memberof)
-        if not type(memberof) in (list, tuple):
-            return ([], [])
-        if len(memberof) == 0:
-            return ([], [])
-
-        search_entry_dn = ldap.filter.escape_filter_chars(str(entry_dn))
-        attr_list = ["memberof"]
-        searchfilter = "(|(member=%s)(memberhost=%s)(memberuser=%s))" % (
-            search_entry_dn, search_entry_dn, search_entry_dn)
-
-        # Search only the groups for which the object is a member to
-        # determine if it is directly or indirectly associated.
-
-        results = []
-        for group in memberof:
-            assert isinstance(group, DN)
-            try:
-                result, truncated = self.find_entries(
-                    searchfilter, attr_list,
-                    group, time_limit=time_limit, size_limit=size_limit,
-                    scope=ldap.SCOPE_BASE)
-                results.extend(list(result))
-            except errors.NotFound:
-                pass
-
-        direct = []
-        # If there is an exception here, it is likely due to a failure in
-        # referential integrity. All members should have corresponding
-        # memberOf entries.
-        indirect = list(memberof)
-        for r in results:
-            direct.append(r[0])
-            try:
-                indirect.remove(r[0])
-            except ValueError, e:
-                self.log.info(
-                    'Failed to remove indirect entry %s from %s',
-                    r[0], entry_dn)
-                raise e
-
-        self.log.debug(
-            "get_memberof: result direct=%s indirect=%s", direct, indirect)
-        return (direct, indirect)
-
-    def get_members(self, group_dn, members, attr_list=[],
-                    membertype=MEMBERS_ALL, time_limit=None, size_limit=None):
-        """Do a memberOf search of groupdn and return the attributes in
-           attr_list (an empty list returns all attributes).
-
-           membertype = MEMBERS_ALL all members returned
-           membertype = MEMBERS_DIRECT only direct members are returned
-           membertype = MEMBERS_INDIRECT only inherited members are returned
-
-           Members may be included in a group as a result of being a member
-           of a group that is a member of the group being queried.
-
-           Returns a list of DNs.
-        """
-
-        assert isinstance(group_dn, DN)
-
-        if membertype not in [MEMBERS_ALL, MEMBERS_DIRECT, MEMBERS_INDIRECT]:
-            return None
-
-        self.log.debug(
-            "get_members: group_dn=%s members=%s membertype=%s",
-            group_dn, members, membertype)
-        search_group_dn = ldap.filter.escape_filter_chars(str(group_dn))
-        searchfilter = "(memberof=%s)" % search_group_dn
-
-        attr_list.append("member")
-
-        # Verify group membership
-
-        results = []
-        if membertype == MEMBERS_ALL or membertype == MEMBERS_INDIRECT:
-            api = self.get_api()
-            if api:
-                user_container_dn = DN(api.env.container_user, api.env.basedn)
-                host_container_dn = DN(api.env.container_host, api.env.basedn)
-            else:
-                user_container_dn = host_container_dn = None
-            checkmembers = set(DN(x) for x in members)
-            checked = set()
-            while checkmembers:
-                member_dn = checkmembers.pop()
-                checked.add(member_dn)
-
-                # No need to check entry types that are not nested for
-                # additional members
-                if user_container_dn and (
-                        member_dn.endswith(user_container_dn) or
-                        member_dn.endswith(host_container_dn)):
-                    results.append([member_dn, {}])
-                    continue
-                try:
-                    result, truncated = self.find_entries(
-                        searchfilter, attr_list, member_dn,
-                        time_limit=time_limit, size_limit=size_limit,
-                        scope=ldap.SCOPE_BASE)
-                    if truncated:
-                        raise errors.LimitsExceeded()
-                    results.append(list(result[0]))
-                    for m in result[0][1].get('member', []):
-                        # This member may contain other members, add it to our
-                        # candidate list
-                        if m not in checked:
-                            checkmembers.add(m)
-                except errors.NotFound:
-                    pass
-
-        if membertype == MEMBERS_ALL:
-            entries = []
-            for e in results:
-                entries.append(e[0])
-
-            return entries
-
-        dn, group = self.get_entry(
-            group_dn, ['member'],
-            size_limit=size_limit, time_limit=time_limit)
-        real_members = group.get('member', [])
-
-        entries = []
-        for e in results:
-            if e[0] not in real_members and e[0] not in entries:
-                if membertype == MEMBERS_INDIRECT:
-                    entries.append(e[0])
-            else:
-                if membertype == MEMBERS_DIRECT:
-                    entries.append(e[0])
-
-        self.log.debug("get_members: result=%s", entries)
-        return entries
-
     def _get_dn_and_attrs(self, entry_or_dn, entry_attrs):
         """Helper for legacy calling style for {add,update}_entry
         """
diff --git a/ipaserver/plugins/ldap2.py b/ipaserver/plugins/ldap2.py
index 048e2c5..e62f74b 100644
--- a/ipaserver/plugins/ldap2.py
+++ b/ipaserver/plugins/ldap2.py
@@ -56,6 +56,11 @@ from ipalib import api, errors
 from ipalib.crud import CrudBackend
 from ipalib.request import context
 
+# Group Member types
+MEMBERS_ALL = 0
+MEMBERS_DIRECT = 1
+MEMBERS_INDIRECT = 2
+
 
 class ldap2(LDAPClient, CrudBackend):
     """
@@ -176,6 +181,205 @@ class ldap2(LDAPClient, CrudBackend):
             # ignore when trying to unbind multiple times
             pass
 
+    def find_entries(self, filter=None, attrs_list=None, base_dn=None,
+                     scope=_ldap.SCOPE_SUBTREE, time_limit=None,
+                     size_limit=None, search_refs=False):
+        if time_limit is None or size_limit is None:
+            config = self.get_ipa_config()
+            if time_limit is None:
+                time_limit = config.get('ipasearchtimelimit', [None])[0]
+            if size_limit is None:
+                size_limit = config.get('ipasearchrecordslimit', [None])[0]
+
+        res, truncated = super(ldap2, self).find_entries(
+            filter=filter, attrs_list=attrs_list, base_dn=base_dn, scope=scope,
+            time_limit=time_limit, size_limit=size_limit,
+            search_refs=search_refs)
+
+        if attrs_list and (
+                'memberindirect' in attrs_list or '*' in attrs_list):
+            for r in res:
+                if not 'member' in r[1]:
+                    continue
+                else:
+                    members = r[1]['member']
+                    indirect = self.get_members(
+                        r[0], members, membertype=MEMBERS_INDIRECT,
+                        time_limit=time_limit, size_limit=size_limit)
+                    if len(indirect) > 0:
+                        r[1]['memberindirect'] = indirect
+        if attrs_list and (
+                'memberofindirect' in attrs_list or '*' in attrs_list):
+            for r in res:
+                if 'memberof' in r[1]:
+                    memberof = r[1]['memberof']
+                    del r[1]['memberof']
+                elif 'memberOf' in r[1]:
+                    memberof = r[1]['memberOf']
+                    del r[1]['memberOf']
+                else:
+                    continue
+                direct, indirect = self.get_memberof(
+                    r[0], memberof, time_limit=time_limit,
+                    size_limit=size_limit)
+                if len(direct) > 0:
+                    r[1]['memberof'] = direct
+                if len(indirect) > 0:
+                    r[1]['memberofindirect'] = indirect
+
+        return (res, truncated)
+
+    def get_members(self, group_dn, members, attr_list=[],
+                    membertype=MEMBERS_ALL, time_limit=None, size_limit=None):
+        """Do a memberOf search of groupdn and return the attributes in
+           attr_list (an empty list returns all attributes).
+
+           membertype = MEMBERS_ALL all members returned
+           membertype = MEMBERS_DIRECT only direct members are returned
+           membertype = MEMBERS_INDIRECT only inherited members are returned
+
+           Members may be included in a group as a result of being a member
+           of a group that is a member of the group being queried.
+
+           Returns a list of DNs.
+        """
+
+        assert isinstance(group_dn, DN)
+
+        if membertype not in [MEMBERS_ALL, MEMBERS_DIRECT, MEMBERS_INDIRECT]:
+            return None
+
+        self.log.debug(
+            "get_members: group_dn=%s members=%s membertype=%s",
+            group_dn, members, membertype)
+        search_group_dn = ldap.filter.escape_filter_chars(str(group_dn))
+        searchfilter = "(memberof=%s)" % search_group_dn
+
+        attr_list.append("member")
+
+        # Verify group membership
+
+        results = []
+        if membertype == MEMBERS_ALL or membertype == MEMBERS_INDIRECT:
+            api = self.get_api()
+            if api:
+                user_container_dn = DN(api.env.container_user, api.env.basedn)
+                host_container_dn = DN(api.env.container_host, api.env.basedn)
+            else:
+                user_container_dn = host_container_dn = None
+            checkmembers = set(DN(x) for x in members)
+            checked = set()
+            while checkmembers:
+                member_dn = checkmembers.pop()
+                checked.add(member_dn)
+
+                # No need to check entry types that are not nested for
+                # additional members
+                if user_container_dn and (
+                        member_dn.endswith(user_container_dn) or
+                        member_dn.endswith(host_container_dn)):
+                    results.append([member_dn, {}])
+                    continue
+                try:
+                    result, truncated = self.find_entries(
+                        searchfilter, attr_list, member_dn,
+                        time_limit=time_limit, size_limit=size_limit,
+                        scope=ldap.SCOPE_BASE)
+                    if truncated:
+                        raise errors.LimitsExceeded()
+                    results.append(list(result[0]))
+                    for m in result[0][1].get('member', []):
+                        # This member may contain other members, add it to our
+                        # candidate list
+                        if m not in checked:
+                            checkmembers.add(m)
+                except errors.NotFound:
+                    pass
+
+        if membertype == MEMBERS_ALL:
+            entries = []
+            for e in results:
+                entries.append(e[0])
+
+            return entries
+
+        dn, group = self.get_entry(
+            group_dn, ['member'],
+            size_limit=size_limit, time_limit=time_limit)
+        real_members = group.get('member', [])
+
+        entries = []
+        for e in results:
+            if e[0] not in real_members and e[0] not in entries:
+                if membertype == MEMBERS_INDIRECT:
+                    entries.append(e[0])
+            else:
+                if membertype == MEMBERS_DIRECT:
+                    entries.append(e[0])
+
+        self.log.debug("get_members: result=%s", entries)
+        return entries
+
+    def get_memberof(self, entry_dn, memberof, time_limit=None,
+                     size_limit=None):
+        """
+        Examine the objects that an entry is a member of and determine if they
+        are a direct or indirect member of that group.
+
+        entry_dn: dn of the entry we want the direct/indirect members of
+        memberof: the memberOf attribute for entry_dn
+
+        Returns two memberof lists: (direct, indirect)
+        """
+
+        assert isinstance(entry_dn, DN)
+
+        self.log.debug(
+            "get_memberof: entry_dn=%s memberof=%s", entry_dn, memberof)
+        if not type(memberof) in (list, tuple):
+            return ([], [])
+        if len(memberof) == 0:
+            return ([], [])
+
+        search_entry_dn = ldap.filter.escape_filter_chars(str(entry_dn))
+        attr_list = ["memberof"]
+        searchfilter = "(|(member=%s)(memberhost=%s)(memberuser=%s))" % (
+            search_entry_dn, search_entry_dn, search_entry_dn)
+
+        # Search only the groups for which the object is a member to
+        # determine if it is directly or indirectly associated.
+
+        results = []
+        for group in memberof:
+            assert isinstance(group, DN)
+            try:
+                result, truncated = self.find_entries(
+                    searchfilter, attr_list,
+                    group, time_limit=time_limit, size_limit=size_limit,
+                    scope=ldap.SCOPE_BASE)
+                results.extend(list(result))
+            except errors.NotFound:
+                pass
+
+        direct = []
+        # If there is an exception here, it is likely due to a failure in
+        # referential integrity. All members should have corresponding
+        # memberOf entries.
+        indirect = list(memberof)
+        for r in results:
+            direct.append(r[0])
+            try:
+                indirect.remove(r[0])
+            except ValueError, e:
+                self.log.info(
+                    'Failed to remove indirect entry %s from %s',
+                    r[0], entry_dn)
+                raise e
+
+        self.log.debug(
+            "get_memberof: result direct=%s indirect=%s", direct, indirect)
+        return (direct, indirect)
+
     config_defaults = {'ipasearchtimelimit': [2], 'ipasearchrecordslimit': [0]}
     def get_ipa_config(self, attrs_list=None):
         """Returns the IPA configuration entry (dn, entry_attrs)."""
-- 
1.8.3.1

>From 3fe04ca525718c7b311a448d47a9178640e2c0f8 Mon Sep 17 00:00:00 2001
From: Jan Cholasta <jchol...@redhat.com>
Date: Thu, 31 Oct 2013 12:29:26 +0000
Subject: [PATCH 2/5] Add wrapper for result3 to IPASimpleLDAPObject.

https://fedorahosted.org/freeipa/ticket/3971
---
 ipapython/ipaldap.py | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/ipapython/ipaldap.py b/ipapython/ipaldap.py
index 7bc4cd9..998b56b 100644
--- a/ipapython/ipaldap.py
+++ b/ipapython/ipaldap.py
@@ -529,6 +529,11 @@ class IPASimpleLDAPObject(object):
         resp_data = self.convert_result(resp_data)
         return resp_type, resp_data
 
+    def result3(self, msgid=ldap.RES_ANY, all=1, timeout=None):
+        rtype, rdata, rmsgid, rctrls = self.conn.result3(msgid, all, timeout)
+        rdata = self.convert_result(rdata)
+        return rtype, rdata, rmsgid, rctrls
+
     def sasl_interactive_bind_s(self, who, auth, serverctrls=None,
                                 clientctrls=None, sasl_flags=ldap.SASL_QUIET):
         self.flush_cached_schema()
-- 
1.8.3.1

>From 6f4f0fde6c21eaa0aff01fc4ea3faee014906b78 Mon Sep 17 00:00:00 2001
From: Jan Cholasta <jchol...@redhat.com>
Date: Thu, 31 Oct 2013 12:31:24 +0000
Subject: [PATCH 3/5] Support searches with paged results control in
 LDAPClient.

https://fedorahosted.org/freeipa/ticket/3971
---
 ipapython/ipaldap.py       | 75 ++++++++++++++++++++++++++++++++++++----------
 ipaserver/plugins/ldap2.py |  4 +--
 2 files changed, 61 insertions(+), 18 deletions(-)

diff --git a/ipapython/ipaldap.py b/ipapython/ipaldap.py
index 998b56b..c2c566a 100644
--- a/ipapython/ipaldap.py
+++ b/ipapython/ipaldap.py
@@ -31,6 +31,7 @@ import ldap
 import ldap.sasl
 import ldap.filter
 from ldap.ldapobject import SimpleLDAPObject
+from ldap.controls import SimplePagedResultsControl
 import ldapurl
 
 from ipalib import errors, _
@@ -1378,7 +1379,7 @@ class LDAPClient(object):
 
     def find_entries(self, filter=None, attrs_list=None, base_dn=None,
                      scope=ldap.SCOPE_SUBTREE, time_limit=None,
-                     size_limit=None, search_refs=False):
+                     size_limit=None, search_refs=False, paged_search=False):
         """
         Return a list of entries and indication of whether the results were
         truncated ([(dn, entry_attrs)], truncated) matching specified search
@@ -1394,6 +1395,7 @@ class LDAPClient(object):
             (default unlimited)
         search_refs -- allow search references to be returned
             (default skips these entries)
+        paged_search -- search using paged results control
         """
         if base_dn is None:
             base_dn = DN()
@@ -1415,24 +1417,65 @@ class LDAPClient(object):
         if attrs_list:
             attrs_list = [a.lower() for a in set(attrs_list)]
 
+        sctrls = None
+        cookie = ''
+        page_size = (size_limit if size_limit > 0 else 2000) - 1
+        if page_size == 0:
+            paged_search = False
+
         # pass arguments to python-ldap
         with self.error_handler():
-            try:
-                id = self.conn.search_ext(
-                    base_dn, scope, filter, attrs_list, timeout=time_limit,
-                    sizelimit=size_limit
-                )
-                while True:
-                    (objtype, res_list) = self.conn.result(id, 0)
-                    if not res_list:
+            while True:
+                if paged_search:
+                    sctrls = [SimplePagedResultsControl(0, page_size, cookie)]
+
+                try:
+                    id = self.conn.search_ext(
+                        base_dn, scope, filter, attrs_list,
+                        serverctrls=sctrls, timeout=time_limit,
+                        sizelimit=size_limit
+                    )
+                    while True:
+                        result = self.conn.result3(id, 0)
+                        objtype, res_list, res_id, res_ctrls = result
+                        if not res_list:
+                            break
+                        if (objtype == ldap.RES_SEARCH_ENTRY or
+                                (search_refs and
+                                    objtype == ldap.RES_SEARCH_REFERENCE)):
+                            res.append(res_list[0])
+
+                    if paged_search:
+                        # Get cookie for the next page
+                        for ctrl in res_ctrls:
+                            if isinstance(ctrl, SimplePagedResultsControl):
+                                cookie = ctrl.cookie
+                                break
+                        else:
+                            cookie = ''
+                except ldap.LDAPError, e:
+                    # If paged search is in progress, try to cancel it
+                    if paged_search and cookie:
+                        sctrls = [SimplePagedResultsControl(0, 0, cookie)]
+                        try:
+                            self.conn.search_ext_s(
+                                base_dn, scope, filter, attrs_list,
+                                serverctrls=sctrls, timeout=time_limit,
+                                sizelimit=size_limit)
+                        except ldap.LDAPError, e:
+                            self.log.warning(
+                                "Error cancelling paged search: %s", e)
+                        cookie = ''
+
+                    try:
+                        raise e
+                    except (ldap.ADMINLIMIT_EXCEEDED, ldap.TIMELIMIT_EXCEEDED,
+                            ldap.SIZELIMIT_EXCEEDED):
+                        truncated = True
                         break
-                    if (objtype == ldap.RES_SEARCH_ENTRY or
-                            (search_refs and
-                                objtype == ldap.RES_SEARCH_REFERENCE)):
-                        res.append(res_list[0])
-            except (ldap.ADMINLIMIT_EXCEEDED, ldap.TIMELIMIT_EXCEEDED,
-                    ldap.SIZELIMIT_EXCEEDED), e:
-                truncated = True
+
+                if not paged_search or not cookie:
+                    break
 
         if not res and not truncated:
             raise errors.NotFound(reason='no such entry')
diff --git a/ipaserver/plugins/ldap2.py b/ipaserver/plugins/ldap2.py
index e62f74b..d05ab39 100644
--- a/ipaserver/plugins/ldap2.py
+++ b/ipaserver/plugins/ldap2.py
@@ -183,7 +183,7 @@ class ldap2(LDAPClient, CrudBackend):
 
     def find_entries(self, filter=None, attrs_list=None, base_dn=None,
                      scope=_ldap.SCOPE_SUBTREE, time_limit=None,
-                     size_limit=None, search_refs=False):
+                     size_limit=None, search_refs=False, paged_search=False):
         if time_limit is None or size_limit is None:
             config = self.get_ipa_config()
             if time_limit is None:
@@ -194,7 +194,7 @@ class ldap2(LDAPClient, CrudBackend):
         res, truncated = super(ldap2, self).find_entries(
             filter=filter, attrs_list=attrs_list, base_dn=base_dn, scope=scope,
             time_limit=time_limit, size_limit=size_limit,
-            search_refs=search_refs)
+            search_refs=search_refs, paged_search=paged_search)
 
         if attrs_list and (
                 'memberindirect' in attrs_list or '*' in attrs_list):
-- 
1.8.3.1

>From 8b544e5918bb09ebd170747d2c2e1610c684babd Mon Sep 17 00:00:00 2001
From: Jan Cholasta <jchol...@redhat.com>
Date: Thu, 31 Oct 2013 12:34:08 +0000
Subject: [PATCH 4/5] Refactor indirect membership processing.

A single LDAP search is now used instead of one search per member.

https://fedorahosted.org/freeipa/ticket/3971
---
 ipaserver/plugins/ldap2.py | 249 ++++++++++++---------------------------------
 1 file changed, 67 insertions(+), 182 deletions(-)

diff --git a/ipaserver/plugins/ldap2.py b/ipaserver/plugins/ldap2.py
index d05ab39..e3952b4 100644
--- a/ipaserver/plugins/ldap2.py
+++ b/ipaserver/plugins/ldap2.py
@@ -56,11 +56,6 @@ from ipalib import api, errors
 from ipalib.crud import CrudBackend
 from ipalib.request import context
 
-# Group Member types
-MEMBERS_ALL = 0
-MEMBERS_DIRECT = 1
-MEMBERS_INDIRECT = 2
-
 
 class ldap2(LDAPClient, CrudBackend):
     """
@@ -191,194 +186,84 @@ class ldap2(LDAPClient, CrudBackend):
             if size_limit is None:
                 size_limit = config.get('ipasearchrecordslimit', [None])[0]
 
+        has_memberindirect = False
+        has_memberofindirect = False
+        if attrs_list:
+            if 'memberindirect' in attrs_list:
+                has_memberindirect = True
+                attrs_list.remove('memberindirect')
+            if 'memberofindirect' in attrs_list:
+                has_memberofindirect = True
+                attrs_list.remove('memberofindirect')
+
         res, truncated = super(ldap2, self).find_entries(
             filter=filter, attrs_list=attrs_list, base_dn=base_dn, scope=scope,
             time_limit=time_limit, size_limit=size_limit,
             search_refs=search_refs, paged_search=paged_search)
 
-        if attrs_list and (
-                'memberindirect' in attrs_list or '*' in attrs_list):
-            for r in res:
-                if not 'member' in r[1]:
-                    continue
-                else:
-                    members = r[1]['member']
-                    indirect = self.get_members(
-                        r[0], members, membertype=MEMBERS_INDIRECT,
-                        time_limit=time_limit, size_limit=size_limit)
-                    if len(indirect) > 0:
-                        r[1]['memberindirect'] = indirect
-        if attrs_list and (
-                'memberofindirect' in attrs_list or '*' in attrs_list):
-            for r in res:
-                if 'memberof' in r[1]:
-                    memberof = r[1]['memberof']
-                    del r[1]['memberof']
-                elif 'memberOf' in r[1]:
-                    memberof = r[1]['memberOf']
-                    del r[1]['memberOf']
-                else:
-                    continue
-                direct, indirect = self.get_memberof(
-                    r[0], memberof, time_limit=time_limit,
-                    size_limit=size_limit)
-                if len(direct) > 0:
-                    r[1]['memberof'] = direct
-                if len(indirect) > 0:
-                    r[1]['memberofindirect'] = indirect
+        if has_memberindirect or has_memberofindirect:
+            for entry in res:
+                if has_memberindirect:
+                    self._process_memberindirect(
+                        entry, time_limit=time_limit, size_limit=size_limit)
+                if has_memberofindirect:
+                    self._process_memberofindirect(
+                        entry, time_limit=time_limit, size_limit=size_limit)
 
         return (res, truncated)
 
-    def get_members(self, group_dn, members, attr_list=[],
-                    membertype=MEMBERS_ALL, time_limit=None, size_limit=None):
-        """Do a memberOf search of groupdn and return the attributes in
-           attr_list (an empty list returns all attributes).
-
-           membertype = MEMBERS_ALL all members returned
-           membertype = MEMBERS_DIRECT only direct members are returned
-           membertype = MEMBERS_INDIRECT only inherited members are returned
-
-           Members may be included in a group as a result of being a member
-           of a group that is a member of the group being queried.
-
-           Returns a list of DNs.
-        """
-
-        assert isinstance(group_dn, DN)
-
-        if membertype not in [MEMBERS_ALL, MEMBERS_DIRECT, MEMBERS_INDIRECT]:
-            return None
-
-        self.log.debug(
-            "get_members: group_dn=%s members=%s membertype=%s",
-            group_dn, members, membertype)
-        search_group_dn = ldap.filter.escape_filter_chars(str(group_dn))
-        searchfilter = "(memberof=%s)" % search_group_dn
-
-        attr_list.append("member")
-
-        # Verify group membership
-
-        results = []
-        if membertype == MEMBERS_ALL or membertype == MEMBERS_INDIRECT:
-            api = self.get_api()
-            if api:
-                user_container_dn = DN(api.env.container_user, api.env.basedn)
-                host_container_dn = DN(api.env.container_host, api.env.basedn)
-            else:
-                user_container_dn = host_container_dn = None
-            checkmembers = set(DN(x) for x in members)
-            checked = set()
-            while checkmembers:
-                member_dn = checkmembers.pop()
-                checked.add(member_dn)
-
-                # No need to check entry types that are not nested for
-                # additional members
-                if user_container_dn and (
-                        member_dn.endswith(user_container_dn) or
-                        member_dn.endswith(host_container_dn)):
-                    results.append([member_dn, {}])
-                    continue
-                try:
-                    result, truncated = self.find_entries(
-                        searchfilter, attr_list, member_dn,
-                        time_limit=time_limit, size_limit=size_limit,
-                        scope=ldap.SCOPE_BASE)
-                    if truncated:
-                        raise errors.LimitsExceeded()
-                    results.append(list(result[0]))
-                    for m in result[0][1].get('member', []):
-                        # This member may contain other members, add it to our
-                        # candidate list
-                        if m not in checked:
-                            checkmembers.add(m)
-                except errors.NotFound:
-                    pass
-
-        if membertype == MEMBERS_ALL:
-            entries = []
-            for e in results:
-                entries.append(e[0])
-
-            return entries
-
-        dn, group = self.get_entry(
-            group_dn, ['member'],
-            size_limit=size_limit, time_limit=time_limit)
-        real_members = group.get('member', [])
-
-        entries = []
-        for e in results:
-            if e[0] not in real_members and e[0] not in entries:
-                if membertype == MEMBERS_INDIRECT:
-                    entries.append(e[0])
-            else:
-                if membertype == MEMBERS_DIRECT:
-                    entries.append(e[0])
-
-        self.log.debug("get_members: result=%s", entries)
-        return entries
-
-    def get_memberof(self, entry_dn, memberof, time_limit=None,
-                     size_limit=None):
-        """
-        Examine the objects that an entry is a member of and determine if they
-        are a direct or indirect member of that group.
-
-        entry_dn: dn of the entry we want the direct/indirect members of
-        memberof: the memberOf attribute for entry_dn
-
-        Returns two memberof lists: (direct, indirect)
-        """
+    def _process_memberindirect(self, group_entry, time_limit=None,
+                                size_limit=None):
+        filter = self.make_filter({'memberof': group_entry.dn})
+        try:
+            result, truncated = self.find_entries(
+                base_dn=self.api.env.basedn,
+                filter=filter,
+                attrs_list=['member'],
+                time_limit=time_limit,
+                size_limit=size_limit,
+                paged_search=True)
+            if truncated:
+                raise errors.LimitsExceeded()
+        except errors.NotFound:
+            result = []
 
-        assert isinstance(entry_dn, DN)
+        indirect = set()
+        for entry in result:
+            indirect.update(entry.get('member', []))
+        indirect.difference_update(group_entry.get('member', []))
 
-        self.log.debug(
-            "get_memberof: entry_dn=%s memberof=%s", entry_dn, memberof)
-        if not type(memberof) in (list, tuple):
-            return ([], [])
-        if len(memberof) == 0:
-            return ([], [])
-
-        search_entry_dn = ldap.filter.escape_filter_chars(str(entry_dn))
-        attr_list = ["memberof"]
-        searchfilter = "(|(member=%s)(memberhost=%s)(memberuser=%s))" % (
-            search_entry_dn, search_entry_dn, search_entry_dn)
-
-        # Search only the groups for which the object is a member to
-        # determine if it is directly or indirectly associated.
-
-        results = []
-        for group in memberof:
-            assert isinstance(group, DN)
-            try:
-                result, truncated = self.find_entries(
-                    searchfilter, attr_list,
-                    group, time_limit=time_limit, size_limit=size_limit,
-                    scope=ldap.SCOPE_BASE)
-                results.extend(list(result))
-            except errors.NotFound:
-                pass
-
-        direct = []
-        # If there is an exception here, it is likely due to a failure in
-        # referential integrity. All members should have corresponding
-        # memberOf entries.
-        indirect = list(memberof)
-        for r in results:
-            direct.append(r[0])
-            try:
-                indirect.remove(r[0])
-            except ValueError, e:
-                self.log.info(
-                    'Failed to remove indirect entry %s from %s',
-                    r[0], entry_dn)
-                raise e
+        if indirect:
+            group_entry['memberindirect'] = list(indirect)
 
-        self.log.debug(
-            "get_memberof: result direct=%s indirect=%s", direct, indirect)
-        return (direct, indirect)
+    def _process_memberofindirect(self, entry, time_limit=None,
+                                  size_limit=None):
+        dn = entry.dn
+        filter = self.make_filter(
+            {'member': dn, 'memberuser': dn, 'memberhost': dn})
+        try:
+            result, truncated = self.find_entries(
+                base_dn=self.api.env.basedn,
+                filter=filter,
+                attrs_list=[''],
+                time_limit=time_limit,
+                size_limit=size_limit)
+            if truncated:
+                raise errors.LimitsExceeded()
+        except errors.NotFound:
+            result = []
+
+        direct = set()
+        indirect = set(entry.get('memberof', []))
+        for group_entry in result:
+            dn = group_entry.dn
+            if dn in indirect:
+                indirect.remove(dn)
+                direct.add(dn)
+
+        if indirect:
+            entry['memberof'] = list(direct)
+            entry['memberofindirect'] = list(indirect)
 
     config_defaults = {'ipasearchtimelimit': [2], 'ipasearchrecordslimit': [0]}
     def get_ipa_config(self, attrs_list=None):
-- 
1.8.3.1

>From 06af0034d17418dab611bda9da40eb21cd07ab54 Mon Sep 17 00:00:00 2001
From: Jan Cholasta <jchol...@redhat.com>
Date: Thu, 31 Oct 2013 12:36:46 +0000
Subject: [PATCH 5/5] Remove unused method get_api of the ldap2 plugin.

https://fedorahosted.org/freeipa/ticket/3971
---
 ipaserver/plugins/ldap2.py | 3 ---
 1 file changed, 3 deletions(-)

diff --git a/ipaserver/plugins/ldap2.py b/ipaserver/plugins/ldap2.py
index e3952b4..fa58b22 100644
--- a/ipaserver/plugins/ldap2.py
+++ b/ipaserver/plugins/ldap2.py
@@ -85,9 +85,6 @@ class ldap2(LDAPClient, CrudBackend):
         # do not set it
         pass
 
-    def get_api(self):
-        return api
-
     def __del__(self):
         if self.isconnected():
             self.disconnect()
-- 
1.8.3.1

_______________________________________________
Freeipa-devel mailing list
Freeipa-devel@redhat.com
https://www.redhat.com/mailman/listinfo/freeipa-devel

Reply via email to