These patches implement behavior agreed upon during discussion of https://fedorahosted.org/freeipa/ticket/5677

However I'm not sure if we want to push them into 4-3 branch (the ticket is triaged into 4.3.2 milestone) since they modify the framework behavior quite a bit.

If there is no need to have it there (CC'ing Milan since he is the reporter), I would retriage it into 4.4 milestone.

--
Martin^3 Babinsky
From 540033f2672056ca9faa5ea639d9ee10f1b89b65 Mon Sep 17 00:00:00 2001
From: Martin Babinsky <mbabi...@redhat.com>
Date: Fri, 18 Mar 2016 09:49:41 +0100
Subject: [PATCH 1/2] differentiate between limit types when LDAP search
 exceeds configured limits

When LDAP search fails on exceeded limits, we should raise an specific
exception for the type of limit raised (size, time, administrative) so that
the consumer can distinguish between e.g. searches returning too many entries
and those timing out.

https://fedorahosted.org/freeipa/ticket/5677
---
 install/tools/ipa-httpd-kdcproxy |  6 ++--
 install/tools/ipactl             |  6 ++--
 ipalib/errors.py                 | 28 ++++++++++++++++++
 ipalib/plugins/automount.py      |  6 ++--
 ipalib/plugins/baseldap.py       | 14 ++++-----
 ipapython/ipaldap.py             | 62 ++++++++++++++++++++++++++++------------
 ipaserver/plugins/ldap2.py       |  5 ++--
 7 files changed, 86 insertions(+), 41 deletions(-)

diff --git a/install/tools/ipa-httpd-kdcproxy b/install/tools/ipa-httpd-kdcproxy
index 5e67f61a6e2b3fe26532323d773bd502ac52f454..c14b3a721ca739c3412aaaeef0b0d5c71b0e0cf2 100755
--- a/install/tools/ipa-httpd-kdcproxy
+++ b/install/tools/ipa-httpd-kdcproxy
@@ -97,10 +97,8 @@ class KDCProxyConfig(object):
     def _find_entry(self, dn, attrs, filter, scope=IPAdmin.SCOPE_BASE):
         """Find an LDAP entry, handles NotFound and Limit"""
         try:
-            entries, truncated = self.con.find_entries(
-                filter, attrs, dn, scope, time_limit=self.time_limit)
-            if truncated:
-                raise errors.LimitsExceeded()
+            entries = self.con.get_entries(
+                dn, scope, filter, attrs, time_limit=self.time_limit)
         except errors.NotFound:
             self.log.debug('Entry not found: %s', dn)
             return None
diff --git a/install/tools/ipactl b/install/tools/ipactl
index d27ada56556c58c42242cf0add11ef47d4440f56..8c312ad6b001d27ca586aee90288c4f402f68489 100755
--- a/install/tools/ipactl
+++ b/install/tools/ipactl
@@ -160,14 +160,12 @@ def get_config(dirsrv):
             wait_for_open_ports(host, [int(port)], timeout=api.env.startup_timeout)
         con = IPAdmin(ldap_uri=api.env.ldap_uri)
         con.do_external_bind()
-        res, truncated = con.find_entries(
+        res = con.get_entries(
+            base,
             filter=srcfilter,
             attrs_list=attrs,
-            base_dn=base,
             scope=con.SCOPE_SUBTREE,
             time_limit=10)
-        if truncated:
-            raise errors.LimitsExceeded()
     except errors.NetworkError:
         # LSB status code 3: program is not running
         raise IpactlError("Failed to get list of services to probe status:\n" +
diff --git a/ipalib/errors.py b/ipalib/errors.py
index 52b770027081448827007d8af00143046d59de0a..01e0e79a1ada6ea398b3f954bd62f0d9b64b4a34 100644
--- a/ipalib/errors.py
+++ b/ipalib/errors.py
@@ -1612,6 +1612,34 @@ class TaskTimeout(DatabaseError):
     format = _("%(task)s LDAP task timeout, Task DN: '%(task_dn)s'")
 
 
+class TimeLimitExceeded(LimitsExceeded):
+    """
+    **4214** Raised when time limit for the operation is exceeded.
+    """
+
+    errno = 4214
+    format = _('Configured time limit exceeded %(desc)s')
+
+
+class SizeLimitExceeded(LimitsExceeded):
+    """
+    **4215** Raised when size limit for the operation is exceeded.
+    """
+
+    errno = 4215
+    format = _('Configured size limit exceeded %(desc)s')
+
+
+class AdminLimitExceeded(LimitsExceeded):
+    """
+    **4216** Raised when server limit imposed by administrative authority was
+    exceeded
+    """
+
+    errno = 4216
+    format = _('Configured administrative server limit exceeded %(desc)s')
+
+
 class CertificateError(ExecutionError):
     """
     **4300** Base class for Certificate execution errors (*4300 - 4399*).
diff --git a/ipalib/plugins/automount.py b/ipalib/plugins/automount.py
index 7dc00224e845e12a699ea263abbb0edd45ea4bc0..c0a55d5dabec72ace99ab178ff3dec7b15ede744 100644
--- a/ipalib/plugins/automount.py
+++ b/ipalib/plugins/automount.py
@@ -803,12 +803,10 @@ class automountkey(LDAPObject):
                         ('cn', parent_keys[0]), self.container_dn,
                         api.env.basedn)
             attrs_list = ['*']
-            entries, truncated = ldap.find_entries(
-                sfilter, attrs_list, basedn, ldap.SCOPE_ONELEVEL)
+            entries = ldap.get_entries(
+                basedn, ldap.SCOPE_ONELEVEL, sfilter, attrs_list)
             if len(entries) > 1:
                 raise errors.NotFound(reason=_('More than one entry with key %(key)s found, use --info to select specific entry.') % dict(key=pkey))
-            if truncated:
-                raise errors.LimitsExceeded()
             dn = entries[0].dn
 
         return dn
diff --git a/ipalib/plugins/baseldap.py b/ipalib/plugins/baseldap.py
index ffc0008a65d3ea52a084870591b867648c8dea36..0eae45dbd3cce0e5113c07cf27ea1da6c1218d83 100644
--- a/ipalib/plugins/baseldap.py
+++ b/ipalib/plugins/baseldap.py
@@ -684,14 +684,12 @@ class LDAPObject(Object):
         filter = self.backend.combine_filters(
             ('(member=*)', mo_filter), self.backend.MATCH_ALL)
         try:
-            result, truncated = self.backend.find_entries(
-                base_dn=self.api.env.basedn,
+            result = self.backend.get_entries(
+                self.api.env.basedn,
                 filter=filter,
                 attrs_list=['member'],
                 size_limit=-1, # paged search will get everything anyway
                 paged_search=True)
-            if truncated:
-                raise errors.LimitsExceeded()
         except errors.NotFound:
             result = []
 
@@ -709,12 +707,10 @@ class LDAPObject(Object):
         filter = self.backend.make_filter(
             {'member': dn, 'memberuser': dn, 'memberhost': dn})
         try:
-            result, truncated = self.backend.find_entries(
-                base_dn=self.api.env.basedn,
+            result = self.backend.get_entries(
+                self.api.env.basedn,
                 filter=filter,
                 attrs_list=[''])
-            if truncated:
-                raise errors.LimitsExceeded()
         except errors.NotFound:
             result = []
 
@@ -2105,7 +2101,7 @@ class LDAPSearch(BaseLDAPCommand, crud.Search):
         result = dict(
             result=entries,
             count=len(entries),
-            truncated=truncated,
+            truncated=bool(truncated),
         )
 
         if truncated:
diff --git a/ipapython/ipaldap.py b/ipapython/ipaldap.py
index 2965ba4a5509eb16589bc9dde9485147d9114032..4cbeb93c16bc7952739faae21f802b69fda59272 100644
--- a/ipapython/ipaldap.py
+++ b/ipapython/ipaldap.py
@@ -60,6 +60,11 @@ AUTOBIND_AUTO = 1
 AUTOBIND_ENABLED = 2
 AUTOBIND_DISABLED = 3
 
+TRUNCATED_SIZE_LIMIT = object()
+TRUNCATED_TIME_LIMIT = object()
+TRUNCATED_ADMIN_LIMIT = object()
+
+
 def unicode_from_utf8(val):
     '''
     val is a UTF-8 encoded string, return a unicode object.
@@ -971,11 +976,11 @@ class LDAPClient(object):
         except ldap.OBJECT_CLASS_VIOLATION:
             raise errors.ObjectclassViolation(info=info)
         except ldap.ADMINLIMIT_EXCEEDED:
-            raise errors.LimitsExceeded()
+            raise errors.AdminLimitExceeded(desc=desc)
         except ldap.SIZELIMIT_EXCEEDED:
-            raise errors.LimitsExceeded()
+            raise errors.SizeLimitExceeded(desc=desc)
         except ldap.TIMELIMIT_EXCEEDED:
-            raise errors.LimitsExceeded()
+            raise errors.TimeLimitExceeded(desc=desc)
         except ldap.NOT_ALLOWED_ON_RDN:
             raise errors.NotAllowedOnRDN(attr=info)
         except ldap.FILTER_ERROR:
@@ -1003,6 +1008,20 @@ class LDAPClient(object):
                 'Unhandled LDAPError: %s: %s' % (type(e).__name__, str(e)))
             raise errors.DatabaseError(desc=desc, info=info)
 
+    @staticmethod
+    def handle_truncated_result(truncated, desc=''):
+        if not truncated:
+            return
+
+        if truncated is TRUNCATED_ADMIN_LIMIT:
+            raise errors.AdminLimitExceeded(desc=desc)
+        elif truncated is TRUNCATED_SIZE_LIMIT:
+            raise errors.SizeLimitExceeded(desc=desc)
+        elif truncated is TRUNCATED_TIME_LIMIT:
+            raise errors.TimeLimitExceeded(desc=desc)
+        else:
+            raise errors.LimitsExceeded()
+
     @property
     def schema(self):
         """schema associated with this LDAP server"""
@@ -1249,7 +1268,7 @@ class LDAPClient(object):
         return self.combine_filters(flts, rules)
 
     def get_entries(self, base_dn, scope=ldap.SCOPE_SUBTREE, filter=None,
-                    attrs_list=None):
+                    attrs_list=None, **kwargs):
         """Return a list of matching entries.
 
         :raises: errors.LimitsExceeded if the list is truncated by the server
@@ -1260,13 +1279,15 @@ class LDAPClient(object):
         :param scope: search scope, see LDAP docs (default ldap2.SCOPE_SUBTREE)
         :param filter: LDAP filter to apply
         :param attrs_list: ist of attributes to return, all if None (default)
-
-        Use the find_entries method for more options.
+        :param kwargs: additional keyword arguments. See find_entries method
+        for their description.
         """
         entries, truncated = self.find_entries(
             base_dn=base_dn, scope=scope, filter=filter, attrs_list=attrs_list)
-        if truncated:
-            raise errors.LimitsExceeded()
+        self.handle_truncated_result(
+            truncated,
+            desc="while getting entries (search base: '{}',"
+                 "filter: {})".format(base_dn, filter))
         return entries
 
     def find_entries(self, filter=None, attrs_list=None, base_dn=None,
@@ -1357,6 +1378,15 @@ class LDAPClient(object):
                                 break
                         else:
                             cookie = ''
+                except ldap.ADMINLIMIT_EXCEEDED:
+                    truncated = TRUNCATED_ADMIN_LIMIT
+                    break
+                except ldap.SIZELIMIT_EXCEEDED:
+                    truncated = TRUNCATED_SIZE_LIMIT
+                    break
+                except ldap.TIMELIMIT_EXCEEDED:
+                    truncated = TRUNCATED_TIME_LIMIT
+                    break
                 except ldap.LDAPError as e:
                     # If paged search is in progress, try to cancel it
                     if paged_search and cookie:
@@ -1402,15 +1432,13 @@ class LDAPClient(object):
 
         search_kw = {attr: value, 'objectClass': object_class}
         filter = self.make_filter(search_kw, rules=self.MATCH_ALL)
-        (entries, truncated) = self.find_entries(filter, attrs_list, base_dn)
+        entries = self.get_entries(
+            base_dn, filter=filter, attrs_list=attrs_list)
 
         if len(entries) > 1:
             raise errors.SingleMatchExpected(found=len(entries))
-        else:
-            if truncated:
-                raise errors.LimitsExceeded()
-            else:
-                return entries[0]
+
+        return entries[0]
 
     def get_entry(self, dn, attrs_list=None, time_limit=None,
                   size_limit=None):
@@ -1423,13 +1451,11 @@ class LDAPClient(object):
 
         assert isinstance(dn, DN)
 
-        (entries, truncated) = self.find_entries(
-            None, attrs_list, dn, self.SCOPE_BASE, time_limit=time_limit,
+        entries = self.get_entries(
+            dn, self.SCOPE_BASE, None, attrs_list, time_limit=time_limit,
             size_limit=size_limit
         )
 
-        if truncated:
-            raise errors.LimitsExceeded()
         return entries[0]
 
     def add_entry(self, entry):
diff --git a/ipaserver/plugins/ldap2.py b/ipaserver/plugins/ldap2.py
index a1f1e1982980f1a913d7f5f40ee4e7c93cff64e7..d50ae1698176c9ec59f4e09461e7377e90ed72e4 100644
--- a/ipaserver/plugins/ldap2.py
+++ b/ipaserver/plugins/ldap2.py
@@ -230,12 +230,13 @@ class ldap2(CrudBackend, LDAPClient):
             # Not in our context yet
             pass
         try:
+            # use find_entries here lest we hit an infinite recursion when
+            # ldap2.get_entries tries to determine default time/size limits
             (entries, truncated) = self.find_entries(
                 None, attrs_list, base_dn=dn, scope=self.SCOPE_BASE,
                 time_limit=2, size_limit=10
             )
-            if truncated:
-                raise errors.LimitsExceeded()
+            self.handle_truncated_result(truncated)
             config_entry = entries[0]
         except errors.NotFound:
             config_entry = self.make_entry(dn)
-- 
2.5.0

From 705d42fa6ad477db2d5c9b3792822de437c70731 Mon Sep 17 00:00:00 2001
From: Martin Babinsky <mbabi...@redhat.com>
Date: Fri, 18 Mar 2016 09:56:08 +0100
Subject: [PATCH 2/2] specify type of exceeded limit when warning about
 truncated search results

API commands inheriting from LDAPSearch should mention which limit was
exceeded in the warning message sent with truncated results.

https://fedorahosted.org/freeipa/ticket/5677
---
 ipalib/messages.py                                 | 2 +-
 ipalib/plugins/baseldap.py                         | 7 +++++--
 ipatests/test_xmlrpc/test_old_permission_plugin.py | 8 ++++----
 ipatests/test_xmlrpc/test_permission_plugin.py     | 8 ++++----
 4 files changed, 14 insertions(+), 11 deletions(-)

diff --git a/ipalib/messages.py b/ipalib/messages.py
index 5cd0ea1769920c076c62729ed5fe359cd1680723..2c9d0660d7364840c04b9ad4933371629e896590 100644
--- a/ipalib/messages.py
+++ b/ipalib/messages.py
@@ -338,7 +338,7 @@ class SearchResultTruncated(PublicMessage):
 
     errno = 13017
     type = "warning"
-    format = _("Search result has been truncated to configured search limit.")
+    format = _("Search result has been truncated: %(reason)s")
 
 
 class BrokenTrust(PublicMessage):
diff --git a/ipalib/plugins/baseldap.py b/ipalib/plugins/baseldap.py
index 0eae45dbd3cce0e5113c07cf27ea1da6c1218d83..dc61fe45438260cb6286bcfd0935cb1a610366a1 100644
--- a/ipalib/plugins/baseldap.py
+++ b/ipalib/plugins/baseldap.py
@@ -2104,8 +2104,11 @@ class LDAPSearch(BaseLDAPCommand, crud.Search):
             truncated=bool(truncated),
         )
 
-        if truncated:
-            add_message(options['version'], result, SearchResultTruncated())
+        try:
+            ldap.handle_truncated_result(truncated)
+        except errors.LimitsExceeded as e:
+            add_message(options['version'], result, SearchResultTruncated(
+                reason=e))
 
         return result
 
diff --git a/ipatests/test_xmlrpc/test_old_permission_plugin.py b/ipatests/test_xmlrpc/test_old_permission_plugin.py
index 09f43fee8c60e4e97d233d256f046fef44d31acf..4b0959b383911162372e9e5cb332f6217272d496 100644
--- a/ipatests/test_xmlrpc/test_old_permission_plugin.py
+++ b/ipatests/test_xmlrpc/test_old_permission_plugin.py
@@ -525,8 +525,8 @@ class test_old_permission(Declarative):
                     },
                 ],
                 messages=({
-                    'message': (u'Search result has been truncated to '
-                                'configured search limit.'),
+                    'message': (u'Search result has been truncated: '
+                                u'Configured size limit exceeded '),
                     'code': 13017,
                     'type': u'warning',
                     'name': u'SearchResultTruncated'
@@ -585,8 +585,8 @@ class test_old_permission(Declarative):
                                               api.env.basedn)) and
                     'ipapermission' in res['objectclass']],
                 messages=({
-                    'message': (u'Search result has been truncated to '
-                                'configured search limit.'),
+                    'message': (u'Search result has been truncated: '
+                                u'Configured size limit exceeded '),
                     'code': 13017,
                     'type': u'warning',
                     'name': u'SearchResultTruncated'
diff --git a/ipatests/test_xmlrpc/test_permission_plugin.py b/ipatests/test_xmlrpc/test_permission_plugin.py
index 8026e84366e3b9056ed6e2ff6d76c58bdc95140e..bc651e0c556829b10ef4d5dc06f3bd1d6100bc0f 100644
--- a/ipatests/test_xmlrpc/test_permission_plugin.py
+++ b/ipatests/test_xmlrpc/test_permission_plugin.py
@@ -818,8 +818,8 @@ class test_permission(Declarative):
                 ],
                 messages=(
                     {
-                        'message': (u'Search result has been truncated to '
-                                    'configured search limit.'),
+                        'message': (u'Search result has been truncated: '
+                                    u'Configured size limit exceeded '),
                         'code': 13017,
                         'type': u'warning',
                         'name': u'SearchResultTruncated'
@@ -882,8 +882,8 @@ class test_permission(Declarative):
                     'ipapermission' in res['objectclass']],
                 messages=(
                     {
-                        'message': (u'Search result has been truncated to '
-                                    'configured search limit.'),
+                        'message': (u'Search result has been truncated: '
+                                    u'Configured size limit exceeded '),
                         'code': 13017,
                         'type': u'warning',
                         'name': u'SearchResultTruncated'
-- 
2.5.0

-- 
Manage your subscription for the Freeipa-devel mailing list:
https://www.redhat.com/mailman/listinfo/freeipa-devel
Contribute to FreeIPA: http://www.freeipa.org/page/Contribute/Code

Reply via email to