[Freeipa-devel] [PATCH] 0037 make-lint fails on Fedora 16/Rawhide

Alexander Bokovoy abokovoy at redhat.com
Tue Nov 29 11:54:41 UTC 2011


On Tue, 29 Nov 2011, Alexander Bokovoy wrote:
> On Tue, 29 Nov 2011, Martin Kosek wrote:
> > > Conservative patch is attached.
> > > 
> > 
> > What about other add_s(entry) calls? I see we call it this way on more
> > places, especially in replication.py:
> > 
> > $ git grep "add_s(e"
> > ipaserver/install/cainstance.py:        ld.add_s(entry_dn, entry)
> > ipaserver/install/krbinstance.py:            self.admin_conn.add_s(entry) #pylint: disable=E1120
> > ipaserver/install/krbinstance.py:            self.admin_conn.add_s(entry) #pylint: disable=E1120
> > ipaserver/install/replication.py:            conn.add_s(ent)
> > ipaserver/install/replication.py:        conn.add_s(entry)
> > ipaserver/install/replication.py:            conn.add_s(entry)
> > ipaserver/install/replication.py:                self.conn.add_s(entry)
> > ipaserver/install/replication.py:        conn.add_s(entry)
> > ipaserver/install/replication.py:        a_conn.add_s(entry)
> > ipaserver/install/replication.py:            self.conn.add_s(entry)
> > ipaserver/install/service.py:            conn.add_s(entry) #pylint: disable=E1120
> > 
> > Should we patch ipa-2-1 branch as well? If we do another release for
> > F-16 we want to have pylint check clean. We would need a rebased patch
> > for ipa-2-1 branch in this case.
> I think this patch also can go away, we found out with Rob yesterday 
> night that John has addressed these issues in his patches 48 and 49 in 
> September. The patches were ACKed but not merged. I'm doing their 
> testing now and will push them after that.
> 
> Look at John's 48/49 patches in meanwhile.
Ok, I've tested patches 48 and 49 and also made versions for ipa-2-1 
branch. All attached.

-- 
/ Alexander Bokovoy
-------------- next part --------------
>From dd2ab5e265da7eead7c8bd1f09551401d968e784 Mon Sep 17 00:00:00 2001
From: John Dennis <jdennis at redhat.com>
Date: Mon, 26 Sep 2011 10:39:15 -0400
Subject: [PATCH 1/3] Ticket #1879 - IPAdmin undefined anonymous parameter
 lists

The IPAdmin class in ipaserver/ipaldap.py has methods with anonymous
undefined parameter lists.

For example:

    def getList(self,*args):

In Python syntax this means you can call getList with any positional
parameter list you want.

This is bad because:

1) It's not true, *args gets passed to an ldap function with a well
defined parameter list, so you really do have to call it with a
defined parameter list. *args will let you pass anything, but once it
gets passed to the ldap function it will blow up if the parameters do
not match (what parameters are those you're wondering? see item 2).

2) The programmer does not know what the valid parameters are unless
they are defined in the formal parameter list.

3) Without a formal parameter list automatic documentation generators
cannot produce API documentation (see item 2)

4) The Python interpreter cannot validate the parameters being passed
because there is no formal parameter list. Note, Python does not
validate the type of parameters, but it does validate the correct
number of postitional parameters are passed and only defined keyword
parameters are passed. Bypassing the language support facilities leads
to programming errors.

5) Without a formal parameter list program checkers such as pylint
cannot validate the program which leads to progamming errors.

6) Without a formal parameter list which includes default keyword
parameters it's not possible to use keyword arguments nor to know what
their default values are (see item 2). One is forced to pass a keyword
argument as a positional argument, plus you must then pass every
keyword argument between the end of the positional argument list and
keyword arg of interest even of the other keyword arguments are not of
interest. This also demands you know what the default value of the
intermediate keyword arguments are (see item 2) and hope they don't
change.

Also the *args anonymous tuple get passed into the error handling code
so it can report what the called values were. But because the tuple is
anonymous the error handler cannot not describe what it was passed. In
addition the error handling code makes assumptions about the possible
contents of the anonymous tuple based on current practice instead of
actual defined values. Things like "if the number of items in the
tuple is 2 or less then the first tuple item must be a dn
(Distinguished Name)" or "if the number of items in the tuple is
greater than 2 then the 3rd item must be an ldap search filter". These
are constructs which are not robust and will fail at some point in the
future.

This patch also fixes the use of IPAdmin.addEntry(). It was sometimes
being called with (dn, modlist), sometimes a Entry object, or
sometimes a Entity object. Now it's always called with either a Entry
or Entity object and IPAdmin.addEntry() validates the type of the
parameter passed.
---
 ipaserver/install/ldapupdate.py |    4 +-
 ipaserver/ipaldap.py            |   94 ++++++++++++++++++---------------------
 2 files changed, 45 insertions(+), 53 deletions(-)

diff --git a/ipaserver/install/ldapupdate.py b/ipaserver/install/ldapupdate.py
index e1f6b1f43929ed77e1896e5fb37515e7ad70aabb..1b6a6a5f4d572100504c28ebc628569c6bce85a8 100644
--- a/ipaserver/install/ldapupdate.py
+++ b/ipaserver/install/ldapupdate.py
@@ -357,7 +357,7 @@ class LDAPUpdate:
         logging.debug("Task id: %s", dn)
 
         if self.live_run:
-            self.conn.addEntry(e.dn, e.toTupleList())
+            self.conn.addEntry(e)
 
         return dn
 
@@ -657,7 +657,7 @@ class LDAPUpdate:
                         # addifexist may result in an entry with only a
                         # dn defined. In that case there is nothing to do.
                         # It means the entry doesn't exist, so skip it.
-                        self.conn.addEntry(entry.dn, entry.toTupleList())
+                        self.conn.addEntry(entry)
                 self.modified = True
             except Exception, e:
                 logging.error("Add failure %s", e)
diff --git a/ipaserver/ipaldap.py b/ipaserver/ipaldap.py
index 74cfbfda911facbf6f3bddf5972b3f035a9cfde0..fdcbe62495805f76397e7588fe24041ce540b8c0 100644
--- a/ipaserver/ipaldap.py
+++ b/ipaserver/ipaldap.py
@@ -1,5 +1,6 @@
 # Authors: Rich Megginson <richm at redhat.com>
-#          Rob Crittenden <rcritten at redhat.com
+#          Rob Crittenden <rcritten at redhat.com>
+#          John Dennis <jdennis at redhat.com>
 #
 # Copyright (C) 2007  Red Hat
 # see file 'COPYING' for use and warranty information
@@ -35,6 +36,7 @@ from ldap.ldapobject import SimpleLDAPObject
 from ipaserver import ipautil
 from ipalib import errors
 from ipapython.ipautil import format_netloc
+from ipapython.entity import Entity
 
 # Global variable to define SASL auth
 SASL_AUTH = ldap.sasl.sasl({},'GSSAPI')
@@ -263,7 +265,7 @@ class IPAdmin(SimpleLDAPObject):
 
             self.dbdir = os.path.dirname(ent.getValue('nsslapd-directory'))
         except ldap.LDAPError, e:
-            self.__handle_errors(e, **{})
+            self.__handle_errors(e)
 
     def __str__(self):
         return self.host + ":" + str(self.port)
@@ -304,8 +306,8 @@ class IPAdmin(SimpleLDAPObject):
             # re-raise the error so we can handle it
             raise e
         except ldap.NO_SUCH_OBJECT, e:
-            args = kw.get('args', ["entry not found"])
-            raise errors.NotFound(reason=notfound(args))
+            arg_desc = kw.get('arg_desc', "entry not found")
+            raise errors.NotFound(reason=arg_desc)
         except ldap.ALREADY_EXISTS, e:
             raise errors.DuplicateEntry()
         except ldap.CONSTRAINT_VIOLATION, e:
@@ -344,7 +346,7 @@ class IPAdmin(SimpleLDAPObject):
                 self.principal = principal
             self.proxydn = None
         except ldap.LDAPError, e:
-            self.__handle_errors(e, **{})
+            self.__handle_errors(e)
 
     def do_simple_bind(self, binddn="cn=directory manager", bindpw=""):
         self.binddn = binddn
@@ -361,7 +363,7 @@ class IPAdmin(SimpleLDAPObject):
         self.sasl_interactive_bind_s("", auth_tokens)
         self.__lateinit()
 
-    def getEntry(self,*args):
+    def getEntry(self, base, scope, filterstr='(objectClass=*)', attrlist=None, attrsonly=0):
         """This wraps the search function.  It is common to just get one entry"""
 
         sctrl = self.__get_server_controls()
@@ -370,21 +372,22 @@ class IPAdmin(SimpleLDAPObject):
             self.set_option(ldap.OPT_SERVER_CONTROLS, sctrl)
 
         try:
-            res = self.search(*args)
+            res = self.search(base, scope, filterstr, attrlist, attrsonly)
             objtype, obj = self.result(res)
         except ldap.LDAPError, e:
-            kw = {'args': args}
-            self.__handle_errors(e, **kw)
+            arg_desc = 'base="%s", scope=%s, filterstr="%s"' % (base, scope, filterstr)
+            self.__handle_errors(e, arg_desc=arg_desc)
 
         if not obj:
-            raise errors.NotFound(reason=notfound(args))
+            arg_desc = 'base="%s", scope=%s, filterstr="%s"' % (base, scope, filterstr)
+            raise errors.NotFound(reason=arg_desc)
 
         elif isinstance(obj,Entry):
             return obj
         else: # assume list/tuple
             return obj[0]
 
-    def getList(self,*args):
+    def getList(self, base, scope, filterstr='(objectClass=*)', attrlist=None, attrsonly=0):
         """This wraps the search function to find multiple entries."""
 
         sctrl = self.__get_server_controls()
@@ -392,14 +395,15 @@ class IPAdmin(SimpleLDAPObject):
             self.set_option(ldap.OPT_SERVER_CONTROLS, sctrl)
 
         try:
-            res = self.search(*args)
+            res = self.search(base, scope, filterstr, attrlist, attrsonly)
             objtype, obj = self.result(res)
         except ldap.LDAPError, e:
-            kw = {'args': args}
-            self.__handle_errors(e, **kw)
+            arg_desc = 'base="%s", scope=%s, filterstr="%s"' % (base, scope, filterstr)
+            self.__handle_errors(e, arg_desc=arg_desc)
 
         if not obj:
-            raise errors.NotFound(reason=notfound(args))
+            arg_desc = 'base="%s", scope=%s, filterstr="%s"' % (base, scope, filterstr)
+            raise errors.NotFound(reason=arg_desc)
 
         entries = []
         for s in obj:
@@ -407,7 +411,8 @@ class IPAdmin(SimpleLDAPObject):
 
         return entries
 
-    def getListAsync(self,*args):
+    def getListAsync(self, base, scope, filterstr='(objectClass=*)', attrlist=None, attrsonly=0,
+                     serverctrls=None, clientctrls=None, timeout=-1, sizelimit=0):
         """This version performs an asynchronous search, to allow
            results even if we hit a limit.
 
@@ -423,7 +428,8 @@ class IPAdmin(SimpleLDAPObject):
         partial = 0
 
         try:
-            msgid = self.search_ext(*args)
+            msgid = self.search_ext(base, scope, filterstr, attrlist, attrsonly,
+                                    serverctrls, clientctrls, timeout, sizelimit)
             objtype, result_list = self.result(msgid, 0)
             while result_list:
                 for result in result_list:
@@ -433,11 +439,13 @@ class IPAdmin(SimpleLDAPObject):
                 ldap.TIMELIMIT_EXCEEDED), e:
             partial = 1
         except ldap.LDAPError, e:
-            kw = {'args': args}
-            self.__handle_errors(e, **kw)
+            arg_desc = 'base="%s", scope=%s, filterstr="%s", timeout=%s, sizelimit=%s' % \
+                       (base, scope, filterstr, timeout, sizelimit)
+            self.__handle_errors(e, arg_desc=arg_desc)
 
         if not entries:
-            raise errors.NotFound(reason=notfound(args))
+            arg_desc = 'base="%s", scope=%s, filterstr="%s"' % (base, scope, filterstr)
+            raise errors.NotFound(reason=arg_desc)
 
         if partial == 1:
             counter = -1
@@ -446,19 +454,22 @@ class IPAdmin(SimpleLDAPObject):
 
         return [counter] + entries
 
-    def addEntry(self,*args):
+    def addEntry(self, entry):
         """This wraps the add function. It assumes that the entry is already
            populated with all of the desired objectclasses and attributes"""
 
+        if not isinstance(entry, (Entry, Entity)):
+            raise TypeError('addEntry expected an Entry or Entity object, got %s instead' % entry.__class__)
+
         sctrl = self.__get_server_controls()
 
         try:
             if sctrl is not None:
                 self.set_option(ldap.OPT_SERVER_CONTROLS, sctrl)
-            self.add_s(*args)
+            self.add_s(entry.dn, entry.toTupleList())
         except ldap.LDAPError, e:
-            kw = {'args': args}
-            self.__handle_errors(e, **kw)
+            arg_desc = 'entry=%s, modlist=%s' % (entry)
+            self.__handle_errors(e, arg_desc=arg_desc)
         return True
 
     def updateRDN(self, dn, newrdn):
@@ -475,7 +486,7 @@ class IPAdmin(SimpleLDAPObject):
                 self.set_option(ldap.OPT_SERVER_CONTROLS, sctrl)
             self.modrdn_s(dn, newrdn, delold=1)
         except ldap.LDAPError, e:
-            self.__handle_errors(e, **{})
+            self.__handle_errors(e)
         return True
 
     def updateEntry(self,dn,oldentry,newentry):
@@ -494,7 +505,7 @@ class IPAdmin(SimpleLDAPObject):
                 self.set_option(ldap.OPT_SERVER_CONTROLS, sctrl)
             self.modify_s(dn, modlist)
         except ldap.LDAPError, e:
-            self.__handle_errors(e, **{})
+            self.__handle_errors(e)
         return True
 
     def generateModList(self, old_entry, new_entry):
@@ -575,10 +586,10 @@ class IPAdmin(SimpleLDAPObject):
                 self.set_option(ldap.OPT_SERVER_CONTROLS, sctrl)
             self.modify_s(dn, modlist)
         except ldap.LDAPError, e:
-            self.__handle_errors(e, **{})
+            self.__handle_errors(e)
         return True
 
-    def deleteEntry(self,*args):
+    def deleteEntry(self, dn):
         """This wraps the delete function. Use with caution."""
 
         sctrl = self.__get_server_controls()
@@ -586,10 +597,10 @@ class IPAdmin(SimpleLDAPObject):
         try:
             if sctrl is not None:
                 self.set_option(ldap.OPT_SERVER_CONTROLS, sctrl)
-            self.delete_s(*args)
+            self.delete_s(dn)
         except ldap.LDAPError, e:
-            kw = {'args': args}
-            self.__handle_errors(e, **kw)
+            arg_desc = 'dn=%s' % (dn)
+            self.__handle_errors(e, arg_desc=arg_desc)
         return True
 
     def modifyPassword(self,dn,oldpass,newpass):
@@ -607,7 +618,7 @@ class IPAdmin(SimpleLDAPObject):
                 self.set_option(ldap.OPT_SERVER_CONTROLS, sctrl)
             self.passwd_s(dn, oldpass, newpass)
         except ldap.LDAPError, e:
-            self.__handle_errors(e, **{})
+            self.__handle_errors(e)
         return True
 
     def __wrapmethods(self):
@@ -737,22 +748,3 @@ class IPAdmin(SimpleLDAPObject):
         keys.sort(reverse=reverse)
 
         return map(res.get, keys)
-
-
-def notfound(args):
-    """Return a string suitable for displaying as an error when a
-       search returns no results.
-
-       This just returns whatever is after the equals sign"""
-    if len(args) > 2:
-        searchfilter = args[2]
-        try:
-            # Python re doesn't do paren counting so the string could
-            # have a trailing paren "foo)"
-            target = re.match(r'\(.*=(.*)\)', searchfilter).group(1)
-            target = target.replace(")","")
-        except:
-            target = searchfilter
-        return "%s not found" % str(target)
-    else:
-        return args[0]
-- 
1.7.7.3

-------------- next part --------------
>From ef26fcd8c0909e6fd0b57a43575f4ea0006ac3f5 Mon Sep 17 00:00:00 2001
From: John Dennis <jdennis at redhat.com>
Date: Mon, 26 Sep 2011 10:39:15 -0400
Subject: [PATCH 1/2] Ticket #1879 - IPAdmin undefined anonymous parameter
 lists

The IPAdmin class in ipaserver/ipaldap.py has methods with anonymous
undefined parameter lists.

For example:

    def getList(self,*args):

In Python syntax this means you can call getList with any positional
parameter list you want.

This is bad because:

1) It's not true, *args gets passed to an ldap function with a well
defined parameter list, so you really do have to call it with a
defined parameter list. *args will let you pass anything, but once it
gets passed to the ldap function it will blow up if the parameters do
not match (what parameters are those you're wondering? see item 2).

2) The programmer does not know what the valid parameters are unless
they are defined in the formal parameter list.

3) Without a formal parameter list automatic documentation generators
cannot produce API documentation (see item 2)

4) The Python interpreter cannot validate the parameters being passed
because there is no formal parameter list. Note, Python does not
validate the type of parameters, but it does validate the correct
number of postitional parameters are passed and only defined keyword
parameters are passed. Bypassing the language support facilities leads
to programming errors.

5) Without a formal parameter list program checkers such as pylint
cannot validate the program which leads to progamming errors.

6) Without a formal parameter list which includes default keyword
parameters it's not possible to use keyword arguments nor to know what
their default values are (see item 2). One is forced to pass a keyword
argument as a positional argument, plus you must then pass every
keyword argument between the end of the positional argument list and
keyword arg of interest even of the other keyword arguments are not of
interest. This also demands you know what the default value of the
intermediate keyword arguments are (see item 2) and hope they don't
change.

Also the *args anonymous tuple get passed into the error handling code
so it can report what the called values were. But because the tuple is
anonymous the error handler cannot not describe what it was passed. In
addition the error handling code makes assumptions about the possible
contents of the anonymous tuple based on current practice instead of
actual defined values. Things like "if the number of items in the
tuple is 2 or less then the first tuple item must be a dn
(Distinguished Name)" or "if the number of items in the tuple is
greater than 2 then the 3rd item must be an ldap search filter". These
are constructs which are not robust and will fail at some point in the
future.

This patch also fixes the use of IPAdmin.addEntry(). It was sometimes
being called with (dn, modlist), sometimes a Entry object, or
sometimes a Entity object. Now it's always called with either a Entry
or Entity object and IPAdmin.addEntry() validates the type of the
parameter passed.
---
 ipaserver/install/ldapupdate.py |    4 +-
 ipaserver/ipaldap.py            |   94 ++++++++++++++++++---------------------
 2 files changed, 45 insertions(+), 53 deletions(-)

diff --git a/ipaserver/install/ldapupdate.py b/ipaserver/install/ldapupdate.py
index 575f22421b6e310d35a0ac52dc85b8a559f54e93..91d3d83cffd9e53073dc53aad63f9f973e8933e1 100644
--- a/ipaserver/install/ldapupdate.py
+++ b/ipaserver/install/ldapupdate.py
@@ -344,7 +344,7 @@ class LDAPUpdate:
         root_logger.debug("Task id: %s", dn)
 
         if self.live_run:
-            self.conn.addEntry(e.dn, e.toTupleList())
+            self.conn.addEntry(e)
 
         return dn
 
@@ -644,7 +644,7 @@ class LDAPUpdate:
                         # addifexist may result in an entry with only a
                         # dn defined. In that case there is nothing to do.
                         # It means the entry doesn't exist, so skip it.
-                        self.conn.addEntry(entry.dn, entry.toTupleList())
+                        self.conn.addEntry(entry)
                 self.modified = True
             except Exception, e:
                 root_logger.error("Add failure %s", e)
diff --git a/ipaserver/ipaldap.py b/ipaserver/ipaldap.py
index 74cfbfda911facbf6f3bddf5972b3f035a9cfde0..fdcbe62495805f76397e7588fe24041ce540b8c0 100644
--- a/ipaserver/ipaldap.py
+++ b/ipaserver/ipaldap.py
@@ -1,5 +1,6 @@
 # Authors: Rich Megginson <richm at redhat.com>
-#          Rob Crittenden <rcritten at redhat.com
+#          Rob Crittenden <rcritten at redhat.com>
+#          John Dennis <jdennis at redhat.com>
 #
 # Copyright (C) 2007  Red Hat
 # see file 'COPYING' for use and warranty information
@@ -35,6 +36,7 @@ from ldap.ldapobject import SimpleLDAPObject
 from ipaserver import ipautil
 from ipalib import errors
 from ipapython.ipautil import format_netloc
+from ipapython.entity import Entity
 
 # Global variable to define SASL auth
 SASL_AUTH = ldap.sasl.sasl({},'GSSAPI')
@@ -263,7 +265,7 @@ class IPAdmin(SimpleLDAPObject):
 
             self.dbdir = os.path.dirname(ent.getValue('nsslapd-directory'))
         except ldap.LDAPError, e:
-            self.__handle_errors(e, **{})
+            self.__handle_errors(e)
 
     def __str__(self):
         return self.host + ":" + str(self.port)
@@ -304,8 +306,8 @@ class IPAdmin(SimpleLDAPObject):
             # re-raise the error so we can handle it
             raise e
         except ldap.NO_SUCH_OBJECT, e:
-            args = kw.get('args', ["entry not found"])
-            raise errors.NotFound(reason=notfound(args))
+            arg_desc = kw.get('arg_desc', "entry not found")
+            raise errors.NotFound(reason=arg_desc)
         except ldap.ALREADY_EXISTS, e:
             raise errors.DuplicateEntry()
         except ldap.CONSTRAINT_VIOLATION, e:
@@ -344,7 +346,7 @@ class IPAdmin(SimpleLDAPObject):
                 self.principal = principal
             self.proxydn = None
         except ldap.LDAPError, e:
-            self.__handle_errors(e, **{})
+            self.__handle_errors(e)
 
     def do_simple_bind(self, binddn="cn=directory manager", bindpw=""):
         self.binddn = binddn
@@ -361,7 +363,7 @@ class IPAdmin(SimpleLDAPObject):
         self.sasl_interactive_bind_s("", auth_tokens)
         self.__lateinit()
 
-    def getEntry(self,*args):
+    def getEntry(self, base, scope, filterstr='(objectClass=*)', attrlist=None, attrsonly=0):
         """This wraps the search function.  It is common to just get one entry"""
 
         sctrl = self.__get_server_controls()
@@ -370,21 +372,22 @@ class IPAdmin(SimpleLDAPObject):
             self.set_option(ldap.OPT_SERVER_CONTROLS, sctrl)
 
         try:
-            res = self.search(*args)
+            res = self.search(base, scope, filterstr, attrlist, attrsonly)
             objtype, obj = self.result(res)
         except ldap.LDAPError, e:
-            kw = {'args': args}
-            self.__handle_errors(e, **kw)
+            arg_desc = 'base="%s", scope=%s, filterstr="%s"' % (base, scope, filterstr)
+            self.__handle_errors(e, arg_desc=arg_desc)
 
         if not obj:
-            raise errors.NotFound(reason=notfound(args))
+            arg_desc = 'base="%s", scope=%s, filterstr="%s"' % (base, scope, filterstr)
+            raise errors.NotFound(reason=arg_desc)
 
         elif isinstance(obj,Entry):
             return obj
         else: # assume list/tuple
             return obj[0]
 
-    def getList(self,*args):
+    def getList(self, base, scope, filterstr='(objectClass=*)', attrlist=None, attrsonly=0):
         """This wraps the search function to find multiple entries."""
 
         sctrl = self.__get_server_controls()
@@ -392,14 +395,15 @@ class IPAdmin(SimpleLDAPObject):
             self.set_option(ldap.OPT_SERVER_CONTROLS, sctrl)
 
         try:
-            res = self.search(*args)
+            res = self.search(base, scope, filterstr, attrlist, attrsonly)
             objtype, obj = self.result(res)
         except ldap.LDAPError, e:
-            kw = {'args': args}
-            self.__handle_errors(e, **kw)
+            arg_desc = 'base="%s", scope=%s, filterstr="%s"' % (base, scope, filterstr)
+            self.__handle_errors(e, arg_desc=arg_desc)
 
         if not obj:
-            raise errors.NotFound(reason=notfound(args))
+            arg_desc = 'base="%s", scope=%s, filterstr="%s"' % (base, scope, filterstr)
+            raise errors.NotFound(reason=arg_desc)
 
         entries = []
         for s in obj:
@@ -407,7 +411,8 @@ class IPAdmin(SimpleLDAPObject):
 
         return entries
 
-    def getListAsync(self,*args):
+    def getListAsync(self, base, scope, filterstr='(objectClass=*)', attrlist=None, attrsonly=0,
+                     serverctrls=None, clientctrls=None, timeout=-1, sizelimit=0):
         """This version performs an asynchronous search, to allow
            results even if we hit a limit.
 
@@ -423,7 +428,8 @@ class IPAdmin(SimpleLDAPObject):
         partial = 0
 
         try:
-            msgid = self.search_ext(*args)
+            msgid = self.search_ext(base, scope, filterstr, attrlist, attrsonly,
+                                    serverctrls, clientctrls, timeout, sizelimit)
             objtype, result_list = self.result(msgid, 0)
             while result_list:
                 for result in result_list:
@@ -433,11 +439,13 @@ class IPAdmin(SimpleLDAPObject):
                 ldap.TIMELIMIT_EXCEEDED), e:
             partial = 1
         except ldap.LDAPError, e:
-            kw = {'args': args}
-            self.__handle_errors(e, **kw)
+            arg_desc = 'base="%s", scope=%s, filterstr="%s", timeout=%s, sizelimit=%s' % \
+                       (base, scope, filterstr, timeout, sizelimit)
+            self.__handle_errors(e, arg_desc=arg_desc)
 
         if not entries:
-            raise errors.NotFound(reason=notfound(args))
+            arg_desc = 'base="%s", scope=%s, filterstr="%s"' % (base, scope, filterstr)
+            raise errors.NotFound(reason=arg_desc)
 
         if partial == 1:
             counter = -1
@@ -446,19 +454,22 @@ class IPAdmin(SimpleLDAPObject):
 
         return [counter] + entries
 
-    def addEntry(self,*args):
+    def addEntry(self, entry):
         """This wraps the add function. It assumes that the entry is already
            populated with all of the desired objectclasses and attributes"""
 
+        if not isinstance(entry, (Entry, Entity)):
+            raise TypeError('addEntry expected an Entry or Entity object, got %s instead' % entry.__class__)
+
         sctrl = self.__get_server_controls()
 
         try:
             if sctrl is not None:
                 self.set_option(ldap.OPT_SERVER_CONTROLS, sctrl)
-            self.add_s(*args)
+            self.add_s(entry.dn, entry.toTupleList())
         except ldap.LDAPError, e:
-            kw = {'args': args}
-            self.__handle_errors(e, **kw)
+            arg_desc = 'entry=%s, modlist=%s' % (entry)
+            self.__handle_errors(e, arg_desc=arg_desc)
         return True
 
     def updateRDN(self, dn, newrdn):
@@ -475,7 +486,7 @@ class IPAdmin(SimpleLDAPObject):
                 self.set_option(ldap.OPT_SERVER_CONTROLS, sctrl)
             self.modrdn_s(dn, newrdn, delold=1)
         except ldap.LDAPError, e:
-            self.__handle_errors(e, **{})
+            self.__handle_errors(e)
         return True
 
     def updateEntry(self,dn,oldentry,newentry):
@@ -494,7 +505,7 @@ class IPAdmin(SimpleLDAPObject):
                 self.set_option(ldap.OPT_SERVER_CONTROLS, sctrl)
             self.modify_s(dn, modlist)
         except ldap.LDAPError, e:
-            self.__handle_errors(e, **{})
+            self.__handle_errors(e)
         return True
 
     def generateModList(self, old_entry, new_entry):
@@ -575,10 +586,10 @@ class IPAdmin(SimpleLDAPObject):
                 self.set_option(ldap.OPT_SERVER_CONTROLS, sctrl)
             self.modify_s(dn, modlist)
         except ldap.LDAPError, e:
-            self.__handle_errors(e, **{})
+            self.__handle_errors(e)
         return True
 
-    def deleteEntry(self,*args):
+    def deleteEntry(self, dn):
         """This wraps the delete function. Use with caution."""
 
         sctrl = self.__get_server_controls()
@@ -586,10 +597,10 @@ class IPAdmin(SimpleLDAPObject):
         try:
             if sctrl is not None:
                 self.set_option(ldap.OPT_SERVER_CONTROLS, sctrl)
-            self.delete_s(*args)
+            self.delete_s(dn)
         except ldap.LDAPError, e:
-            kw = {'args': args}
-            self.__handle_errors(e, **kw)
+            arg_desc = 'dn=%s' % (dn)
+            self.__handle_errors(e, arg_desc=arg_desc)
         return True
 
     def modifyPassword(self,dn,oldpass,newpass):
@@ -607,7 +618,7 @@ class IPAdmin(SimpleLDAPObject):
                 self.set_option(ldap.OPT_SERVER_CONTROLS, sctrl)
             self.passwd_s(dn, oldpass, newpass)
         except ldap.LDAPError, e:
-            self.__handle_errors(e, **{})
+            self.__handle_errors(e)
         return True
 
     def __wrapmethods(self):
@@ -737,22 +748,3 @@ class IPAdmin(SimpleLDAPObject):
         keys.sort(reverse=reverse)
 
         return map(res.get, keys)
-
-
-def notfound(args):
-    """Return a string suitable for displaying as an error when a
-       search returns no results.
-
-       This just returns whatever is after the equals sign"""
-    if len(args) > 2:
-        searchfilter = args[2]
-        try:
-            # Python re doesn't do paren counting so the string could
-            # have a trailing paren "foo)"
-            target = re.match(r'\(.*=(.*)\)', searchfilter).group(1)
-            target = target.replace(")","")
-        except:
-            target = searchfilter
-        return "%s not found" % str(target)
-    else:
-        return args[0]
-- 
1.7.7.3

-------------- next part --------------
>From 66c3473b0b7231dc58e84f1715640e665b7580b8 Mon Sep 17 00:00:00 2001
From: John Dennis <jdennis at redhat.com>
Date: Mon, 26 Sep 2011 17:33:32 -0400
Subject: [PATCH 2/3] ticket #1870 - subclass SimpleLDAPObject

We use convenience types (classes) in IPA which make working with LDAP
easier and more robust. It would be really nice if the basic python-ldap
library understood our utility types and could accept them as parameters
to the basic ldap functions and/or the basic ldap functions returned our
utility types.

Normally such a requirement would trivially be handled in an object-
oriented language (which Python is) by subclassing to extend and modify
the functionality. For some reason we didn't do this with the python-ldap
classes.

python-ldap objects are primarily used in two different places in our
code, ipaserver.ipaldap.py for the IPAdmin class and in
ipaserver/plugins/ldap2.py for the ldap2 class's .conn member.

In IPAdmin we use a IPA utility class called Entry to make it easier to
use the results returned by LDAP. The IPAdmin class is derived from
python-ldap.SimpleLDAPObject. But for some reason when we added the
support for the use of the Entry class in SimpleLDAPObject we didn't
subclass SimpleLDAPObject and extend it for use with the Entry class as
would be the normal expected methodology in an object-oriented language,
rather we used an obscure feature of the Python language to override all
methods of the SimpleLDAPObject class by wrapping those class methods in
another function call. The reason why this isn't a good approach is:

* It violates object-oriented methodology.

* Other classes cannot be derived and inherit the customization (because
the method wrapping occurs in a class instance, not within the class
type).

* It's non-obvious and obscure

* It's inefficient.

Here is a summary of what the code was doing:

It iterated over every member of the SimpleLDAPObject class and if it was
callable it wrapped the method. The wrapper function tested the name of
the method being wrapped, if it was one of a handful of methods we wanted
to customize we modified a parameter and called the original method. If
the method wasn't of interest to use we still wrapped the method.

It was inefficient because every non-customized method (the majority)
executed a function call for the wrapper, the wrapper during run-time used
logic to determine if the method was being overridden and then called the
original method. So every call to ldap was doing extra function calls and
logic processing which for the majority of cases produced nothing useful
(and was non-obvious from brief code reading some methods were being
overridden).

Object-orientated languages have support built in for calling the right
method for a given class object that do not involve extra function call
overhead to realize customized class behaviour. Also when programmers look
for customized class behaviour they look for derived classes. They might
also want to utilize the customized class as the base class for their use.

Also the wrapper logic was fragile, it did things like: if the method name
begins with "add" I'll unconditionally modify the first and second
argument. It would be some much cleaner if the "add", "add_s", etc.
methods were overridden in a subclass where the logic could be seen and
where it would apply to only the explicit functions and parameters being
overridden.

Also we would really benefit if there were classes which could be used as
a base class which had specific ldap customization.

At the moment our ldap customization needs are:

1) Support DN objects being passed to ldap operations

2) Support Entry & Entity objects being passed into and returned from
ldap operations.

We want to subclass the ldap SimpleLDAPObject class, that is the base
ldap class with all the ldap methods we're using. IPASimpleLDAPObject
class would subclass SimpleLDAPObject class which knows about DN
objects (and possilby other IPA specific types that are universally
used in IPA). Then  IPAEntrySimpleLDAPObject would subclass
IPASimpleLDAPObject which knows about Entry objects.

The reason for the suggested class hierarchy is because DN objects will be
used whenever we talk to LDAP (in the future we may want to add other IPA
specific classes which will always be used). We don't add Entry support to
the the IPASimpleLDAPObject class because Entry objects are (currently)
only used in IPAdmin.

What this patch does is:

* Introduce IPASimpleLDAPObject derived from
  SimpleLDAPObject. IPASimpleLDAPObject is DN object aware.

* Introduce IPAEntryLDAPObject derived from
  IPASimpleLDAPObject. IPAEntryLDAPObject is Entry object aware.

* Derive IPAdmin from IPAEntryLDAPObject and remove the funky method
  wrapping from IPAdmin.

* Code which called add_s() with an Entry or Entity object now calls
  addEntry(). addEntry() always existed, it just wasn't always
  used. add_s() had been modified to accept Entry or Entity object
  (why didn't we just call addEntry()?). The add*() ldap routine in
  IPAEntryLDAPObject have been subclassed to accept Entry and Entity
  objects, but that should proably be removed in the future and just
  use addEntry().

* Replace the call to ldap.initialize() in ldap2.create_connection()
  with a class constructor for IPASimpleLDAPObject. The
  ldap.initialize() is a convenience function in python-ldap, but it
  always returns a SimpleLDAPObject created via the SimpleLDAPObject
  constructor, thus ldap.initialize() did not allow subclassing, yet
  has no particular ease-of-use advantage thus we better off using the
  obvious class constructor mechanism.

* Fix the use of _handle_errors(), it's not necessary to construct an
  empty dict to pass to it.

If we follow the standard class derivation pattern for ldap we can make us
of our own ldap utilities in a far easier, cleaner and more efficient
manner.
---
 ipaserver/install/krbinstance.py |    4 +-
 ipaserver/install/service.py     |    2 +-
 ipaserver/ipaldap.py             |  105 ++++++++++++++------------------
 ipaserver/plugins/ldap2.py       |  124 ++++++++++++++++++++++++++++++++++---
 4 files changed, 164 insertions(+), 71 deletions(-)

diff --git a/ipaserver/install/krbinstance.py b/ipaserver/install/krbinstance.py
index ce70c231dfb7e7b6b59c0496721cced0d09f1604..df6fc5a6ea6fbc4d9c207122dbb3c1ce1f5b4f50 100644
--- a/ipaserver/install/krbinstance.py
+++ b/ipaserver/install/krbinstance.py
@@ -284,7 +284,7 @@ class KrbInstance(service.Service):
         entry.setValues("nsSaslMapFilterTemplate", '(krbPrincipalName=\\1@\\2)')
 
         try:
-            self.admin_conn.add_s(entry)
+            self.admin_conn.addEntry(entry)
         except ldap.ALREADY_EXISTS:
             logging.critical("failed to add Full Principal Sasl mapping")
             raise e
@@ -297,7 +297,7 @@ class KrbInstance(service.Service):
         entry.setValues("nsSaslMapFilterTemplate", '(krbPrincipalName=&@%s)' % self.realm)
 
         try:
-            self.admin_conn.add_s(entry)
+            self.admin_conn.addEntry(entry)
         except ldap.ALREADY_EXISTS:
             logging.critical("failed to add Name Only Sasl mapping")
             raise e
diff --git a/ipaserver/install/service.py b/ipaserver/install/service.py
index 2fd15d8f8010114914549871fc5d0a228561fe1c..9fcc095b64f1abc121f1960d7c7ec15dbe53821f 100644
--- a/ipaserver/install/service.py
+++ b/ipaserver/install/service.py
@@ -287,7 +287,7 @@ class Service(object):
                         "enabledService", "startOrder " + str(order))
 
         try:
-            conn.add_s(entry)
+            conn.addEntry(entry)
         except ldap.ALREADY_EXISTS, e:
             logging.critical("failed to add %s Service startup entry" % name)
             raise e
diff --git a/ipaserver/ipaldap.py b/ipaserver/ipaldap.py
index fdcbe62495805f76397e7588fe24041ce540b8c0..82feacbc1c69d543be1ede9a5dc6ffdba45b50e5 100644
--- a/ipaserver/ipaldap.py
+++ b/ipaserver/ipaldap.py
@@ -37,10 +37,52 @@ from ipaserver import ipautil
 from ipalib import errors
 from ipapython.ipautil import format_netloc
 from ipapython.entity import Entity
+from ipaserver.plugins.ldap2 import IPASimpleLDAPObject
 
 # Global variable to define SASL auth
 SASL_AUTH = ldap.sasl.sasl({},'GSSAPI')
 
+class IPAEntryLDAPObject(IPASimpleLDAPObject):
+    def __init__(self, *args, **kwds):
+        IPASimpleLDAPObject.__init__(self, *args, **kwds)
+
+    def result(self, msgid=ldap.RES_ANY, all=1, timeout=None):
+        objtype, data = IPASimpleLDAPObject.result(self, msgid, all, timeout)
+        # data is either a 2-tuple or a list of 2-tuples
+        if data:
+            if isinstance(data, tuple):
+                return objtype, Entry(data)
+            elif isinstance(data, list):
+                return objtype, [Entry(x) for x in data]
+            else:
+                raise TypeError, "unknown data type %s returned by result" % type(data)
+        else:
+            return objtype, data
+
+    def add(self, dn, modlist):
+        if isinstance(dn, Entry):
+            return IPASimpleLDAPObject.add(self, dn.dn, dn.toTupleList())
+        else:
+            return IPASimpleLDAPObject.add(self, dn, modlist)
+
+    def add_s(self, dn, modlist):
+        if isinstance(dn, Entry):
+            return IPASimpleLDAPObject.add_s(self, dn.dn, dn.toTupleList())
+        else:
+            return IPASimpleLDAPObject.add_s(self, dn, modlist)
+
+    def add_ext(self, dn, modlist, serverctrls=None, clientctrls=None):
+        if isinstance(dn, Entry):
+            return IPASimpleLDAPObject.add_ext(self, dn.dn, dn.toTupleList(), serverctrls, clientctrls)
+        else:
+            return IPASimpleLDAPObject.add_ext(self, dn, modlist, serverctrls, clientctrls)
+
+    def add_ext_s(self, dn, modlist, serverctrls=None, clientctrls=None):
+        if isinstance(dn, Entry):
+            return IPASimpleLDAPObject.add_ext_s(self, dn.dn, dn.toTupleList(), serverctrls, clientctrls)
+        else:
+            return IPASimpleLDAPObject.add_ext_s(self, dn, modlist, serverctrls, clientctrls)
+
 class Entry:
     """
     This class represents an LDAP Entry object.  An LDAP entry consists of
@@ -165,49 +207,7 @@ class Entry:
         ldif.LDIFWriter(sio,Entry.base64_attrs,1000).unparse(self.dn,newdata)
         return sio.getvalue()
 
-def wrapper(f,name):
-    """This is the method that wraps all of the methods of the superclass.
-       This seems to need to be an unbound method, that's why it's outside
-       of IPAdmin.  Perhaps there is some way to do this with the new
-       classmethod or staticmethod of 2.4. Basically, we replace every call
-       to a method in SimpleLDAPObject (the superclass of IPAdmin) with a
-       call to inner.  The f argument to wrapper is the bound method of
-       IPAdmin (which is inherited from the superclass).  Bound means that it
-       will implicitly be called with the self argument, it is not in the
-       args list.  name is the name of the method to call.  If name is a
-       method that returns entry objects (e.g. result), we wrap the data
-       returned by an Entry class.  If name is a method that takes an entry
-       argument, we extract the raw data from the entry object to pass in.
-    """
-    def inner(*args, **kargs):
-        if name == 'result':
-            objtype, data = f(*args, **kargs)
-            # data is either a 2-tuple or a list of 2-tuples
-            # print data
-            if data:
-                if isinstance(data,tuple):
-                    return objtype, Entry(data)
-                elif isinstance(data,list):
-                    return objtype, [Entry(x) for x in data]
-                else:
-                    raise TypeError, "unknown data type %s returned by result" % type(data)
-            else:
-                return objtype, data
-        elif name.startswith('add'):
-            # the first arg is self
-            # the second and third arg are the dn and the data to send
-            # We need to convert the Entry into the format used by
-            # python-ldap
-            ent = args[0]
-            if isinstance(ent,Entry):
-                return f(ent.dn, ent.toTupleList(), *args[2:])
-            else:
-                return f(*args, **kargs)
-        else:
-            return f(*args, **kargs)
-    return inner
-
-class IPAdmin(SimpleLDAPObject):
+class IPAdmin(IPAEntryLDAPObject):
 
     def __localinit(self):
         """If a CA certificate is provided then it is assumed that we are
@@ -218,12 +218,12 @@ class IPAdmin(SimpleLDAPObject):
            its own encryption.
         """
         if self.cacert is not None:
-            SimpleLDAPObject.__init__(self,'ldaps://%s' % format_netloc(self.host, self.port))
+            IPAEntryLDAPObject.__init__(self,'ldaps://%s' % format_netloc(self.host, self.port))
         else:
             if self.ldapi:
-                SimpleLDAPObject.__init__(self,'ldapi://%%2fvar%%2frun%%2fslapd-%s.socket' % "-".join(self.realm.split(".")))
+                IPAEntryLDAPObject.__init__(self,'ldapi://%%2fvar%%2frun%%2fslapd-%s.socket' % "-".join(self.realm.split(".")))
             else:
-                SimpleLDAPObject.__init__(self,'ldap://%s' % format_netloc(self.host, self.port))
+                IPAEntryLDAPObject.__init__(self,'ldap://%s' % format_netloc(self.host, self.port))
 
     def __init__(self,host='',port=389,cacert=None,bindcert=None,bindkey=None,proxydn=None,debug=None,ldapi=False,realm=None):
         """We just set our instance variables and wrap the methods - the real
@@ -240,7 +240,6 @@ class IPAdmin(SimpleLDAPObject):
         if bindkey is not None:
             ldap.set_option(ldap.OPT_X_TLS_KEYFILE,bindkey)
 
-        self.__wrapmethods()
         self.port = port
         self.host = host
         self.cacert = cacert
@@ -468,7 +467,7 @@ class IPAdmin(SimpleLDAPObject):
                 self.set_option(ldap.OPT_SERVER_CONTROLS, sctrl)
             self.add_s(entry.dn, entry.toTupleList())
         except ldap.LDAPError, e:
-            arg_desc = 'entry=%s, modlist=%s' % (entry)
+            arg_desc = 'entry=%s' % (entry)
             self.__handle_errors(e, arg_desc=arg_desc)
         return True
 
@@ -621,16 +620,6 @@ class IPAdmin(SimpleLDAPObject):
             self.__handle_errors(e)
         return True
 
-    def __wrapmethods(self):
-        """This wraps all methods of SimpleLDAPObject, so that we can intercept
-        the methods that deal with entries.  Instead of using a raw list of tuples
-        of lists of hashes of arrays as the entry object, we want to wrap entries
-        in an Entry class that provides some useful methods"""
-        for name in dir(self.__class__.__bases__[0]):
-            attr = getattr(self, name)
-            if callable(attr):
-                setattr(self, name, wrapper(attr, name))
-
     def waitForEntry(self, dn, timeout=7200, attr='', quiet=True):
         scope = ldap.SCOPE_BASE
         filter = "(objectclass=*)"
diff --git a/ipaserver/plugins/ldap2.py b/ipaserver/plugins/ldap2.py
index 5c40182933143021abf817bff4ed12287697e4ea..d014c3f67f5b6c87fc7fec2443994cc5d59e6b5b 100644
--- a/ipaserver/plugins/ldap2.py
+++ b/ipaserver/plugins/ldap2.py
@@ -38,6 +38,7 @@ import re
 import krbV
 import logging
 import ldap as _ldap
+from ldap.ldapobject import SimpleLDAPObject
 import ldap.filter as _ldap_filter
 import ldap.sasl as _ldap_sasl
 from ldap.controls import LDAPControl
@@ -53,7 +54,6 @@ from ipalib.crud import CrudBackend
 from ipalib.encoder import Encoder, encode_args, decode_retval
 from ipalib.request import context
 
-
 # Group Member types
 MEMBERS_ALL = 0
 MEMBERS_DIRECT = 1
@@ -62,6 +62,110 @@ MEMBERS_INDIRECT = 2
 # SASL authentication mechanism
 SASL_AUTH = _ldap_sasl.sasl({}, 'GSSAPI')
 
+class IPASimpleLDAPObject(SimpleLDAPObject):
+    '''
+    This is a thin layer over SimpleLDAPObject which allows us to utilize
+    IPA specific types with the python-ldap API without the IPA caller needing
+    to perform the type translation, consider this a convenience layer for the
+    IPA programmer.
+
+    This subclass performs the following translations:
+
+    * DN objects may be passed into any ldap function expecting a dn. The DN
+      object will be converted to a string before being passed to the python-ldap
+      function. This allows us to maintain DN objects as DN objects in the rest
+      of the code (useful for DN manipulation and DN information) and not have
+      to worry about conversion to a string prior to passing it ldap.
+
+    '''
+    def __init__(self, *args, **kwds):
+        SimpleLDAPObject.__init__(self, *args, **kwds)
+
+    def add(self, dn, modlist):
+        return SimpleLDAPObject.add(self, str(dn), modlist)
+
+    def add_ext(self, dn, modlist, serverctrls=None, clientctrls=None):
+        return SimpleLDAPObject.add_ext(self, str(dn), modlist, serverctrls, clientctrls)
+
+    def add_ext_s(self, dn, modlist, serverctrls=None, clientctrls=None):
+        return SimpleLDAPObject.add_ext_s(self, str(dn), modlist, serverctrls, clientctrls)
+
+    def add_s(self, dn, modlist):
+        return SimpleLDAPObject.add_s(self, str(dn), modlist)
+
+    def compare(self, dn, attr, value):
+        return SimpleLDAPObject.compare(self, str(dn), attr, value)
+
+    def compare_ext(self, dn, attr, value, serverctrls=None, clientctrls=None):
+        return SimpleLDAPObject.compare_ext(self, str(dn), attr, value, serverctrls, clientctrls)
+
+    def compare_ext_s(self, dn, attr, value, serverctrls=None, clientctrls=None):
+        return SimpleLDAPObject.compare_ext_s(self, str(dn), attr, value, serverctrls, clientctrls)
+
+    def compare_s(self, dn, attr, value):
+        return SimpleLDAPObject.compare_s(self, str(dn), attr, value)
+
+    def delete(self, dn):
+        return SimpleLDAPObject.delete(self, str(dn))
+
+    def delete_ext(self, dn, serverctrls=None, clientctrls=None):
+        return SimpleLDAPObject.delete_ext(self, str(dn), serverctrls, clientctrls)
+
+    def delete_ext_s(self, dn, serverctrls=None, clientctrls=None):
+        return SimpleLDAPObject.delete_ext_s(self, str(dn), serverctrls, clientctrls)
+
+    def delete_s(self, dn):
+        return SimpleLDAPObject.delete_s(self, str(dn))
+
+    def modify(self, dn, modlist):
+        return SimpleLDAPObject.modify(self, str(dn), modlist)
+
+    def modify_ext(self, dn, modlist, serverctrls=None, clientctrls=None):
+        return SimpleLDAPObject.modify_ext(self, str(dn), modlist, serverctrls, clientctrls)
+
+    def modify_ext_s(self, dn, modlist, serverctrls=None, clientctrls=None):
+        return SimpleLDAPObject.modify_ext_s(self, str(dn), modlist, serverctrls, clientctrls)
+
+    def modify_s(self, dn, modlist):
+        return SimpleLDAPObject.modify_s(self, str(dn), modlist)
+
+    def modrdn(self, dn, newrdn, delold=1):
+        return SimpleLDAPObject.modrdn(self, str(dn), str(newrdn), delold)
+
+    def modrdn_s(self, dn, newrdn, delold=1):
+        return SimpleLDAPObject.modrdn_s(self, str(dn), str(newrdn), delold)
+
+    def read_subschemasubentry_s(self, subschemasubentry_dn, attrs=None):
+        return SimpleLDAPObject.read_subschemasubentry_s(self, str(subschemasubentry_dn), attrs)
+
+    def rename(self, dn, newrdn, newsuperior=None, delold=1, serverctrls=None, clientctrls=None):
+        return SimpleLDAPObject.rename(self, str(dn), str(newrdn), newsuperior, delold, serverctrls, clientctrls)
+
+    def rename_s(self, dn, newrdn, newsuperior=None, delold=1, serverctrls=None, clientctrls=None):
+        return SimpleLDAPObject.rename_s(self, str(dn), str(newrdn), newsuperior, delold, serverctrls, clientctrls)
+
+    def search(self, base, scope, filterstr='(objectClass=*)', attrlist=None, attrsonly=0):
+        return SimpleLDAPObject.search(self, str(base), scope, filterstr, attrlist, attrsonly)
+
+    def search_ext(self, base, scope, filterstr='(objectClass=*)', attrlist=None, attrsonly=0,
+                   serverctrls=None, clientctrls=None, timeout=-1, sizelimit=0):
+        return SimpleLDAPObject.search_ext(self, str(base), scope, filterstr, attrlist, attrsonly,
+                                           serverctrls, clientctrls, timeout, sizelimit)
+
+    def search_ext_s(self, base, scope, filterstr='(objectClass=*)', attrlist=None, attrsonly=0,
+                     serverctrls=None, clientctrls=None, timeout=-1, sizelimit=0):
+        return SimpleLDAPObject.search_ext_s(self, str(base), scope, filterstr, attrlist, attrsonly,
+                                             serverctrls, clientctrls, timeout, sizelimit)
+
+    def search_s(self, base, scope, filterstr='(objectClass=*)', attrlist=None, attrsonly=0):
+        return SimpleLDAPObject.search_s(self, str(base), scope, filterstr, attrlist, attrsonly)
+
+    def search_st(self, base, scope, filterstr='(objectClass=*)', attrlist=None, attrsonly=0, timeout=-1):
+        return SimpleLDAPObject.search_st(self, str(base), scope, filterstr, attrlist, attrsonly, timeout)
+
+    def search_subschemasubentry_s(self, dn=''):
+        return SimpleLDAPObject.search_subschemasubentry_s(self, str(dn))
+
 # universal LDAPError handler
 def _handle_errors(e, **kw):
     """
@@ -162,7 +266,7 @@ def get_schema(url, conn=None):
                 raise StandardError('Unable to retrieve LDAP schema. Error initializing principal %s in %s: %s' % (principal.name, '/etc/httpd/conf/ipa.keytab', str(e)))
 
         if conn is None:
-            conn = _ldap.initialize(url)
+            conn = IPASimpleLDAPObject(url)
             if url.startswith('ldapi://'):
                 conn.set_option(_ldap.OPT_HOST_NAME, api.env.host)
             conn.sasl_interactive_bind_s('', SASL_AUTH)
@@ -337,7 +441,7 @@ class ldap2(CrudBackend, Encoder):
                 # no kerberos ccache, use simple bind
                 conn.simple_bind_s(bind_dn, bind_pw)
         except _ldap.LDAPError, e:
-            _handle_errors(e, **{})
+            _handle_errors(e)
 
         if _schema:
             object.__setattr__(self, 'schema', _schema)
@@ -425,7 +529,7 @@ class ldap2(CrudBackend, Encoder):
         try:
             self.conn.add_s(dn, list(entry_attrs.iteritems()))
         except _ldap.LDAPError, e:
-            _handle_errors(e, **{})
+            _handle_errors(e)
 
     # generating filters for find_entry
     # some examples:
@@ -599,7 +703,7 @@ class ldap2(CrudBackend, Encoder):
                 _ldap.SIZELIMIT_EXCEEDED), e:
             truncated = True
         except _ldap.LDAPError, e:
-            _handle_errors(e, **{})
+            _handle_errors(e)
 
         if not res:
             raise errors.NotFound(reason='no such entry')
@@ -818,7 +922,7 @@ class ldap2(CrudBackend, Encoder):
             self.conn.rename_s(dn, new_rdn, delold=int(del_old))
             time.sleep(.3) # Give memberOf plugin a chance to work
         except _ldap.LDAPError, e:
-            _handle_errors(e, **{})
+            _handle_errors(e)
 
     def _generate_modlist(self, dn, entry_attrs, normalize):
         # get original entry
@@ -886,7 +990,7 @@ class ldap2(CrudBackend, Encoder):
         try:
             self.conn.modify_s(dn, modlist)
         except _ldap.LDAPError, e:
-            _handle_errors(e, **{})
+            _handle_errors(e)
 
     @encode_args(1)
     def delete_entry(self, dn, normalize=True):
@@ -896,7 +1000,7 @@ class ldap2(CrudBackend, Encoder):
         try:
             self.conn.delete_s(dn)
         except _ldap.LDAPError, e:
-            _handle_errors(e, **{})
+            _handle_errors(e)
 
     @encode_args(1, 2, 3)
     def modify_password(self, dn, new_pass, old_pass=''):
@@ -916,7 +1020,7 @@ class ldap2(CrudBackend, Encoder):
         try:
             self.conn.passwd_s(dn, old_pass, new_pass)
         except _ldap.LDAPError, e:
-            _handle_errors(e, **{})
+            _handle_errors(e)
 
     def add_entry_to_group(self, dn, group_dn, member_attr='member', allow_same=False):
         """
@@ -1132,7 +1236,7 @@ class ldap2(CrudBackend, Encoder):
         try:
             self.conn.modify_s(dn, mod)
         except _ldap.LDAPError, e:
-            _handle_errors(e, **{})
+            _handle_errors(e)
 
     # CrudBackend methods
 
-- 
1.7.7.3

-------------- next part --------------
>From b75d08773d848fab585d43c7f023d5c9af658707 Mon Sep 17 00:00:00 2001
From: John Dennis <jdennis at redhat.com>
Date: Mon, 26 Sep 2011 17:33:32 -0400
Subject: [PATCH 2/2] ticket #1870 - subclass SimpleLDAPObject

We use convenience types (classes) in IPA which make working with LDAP
easier and more robust. It would be really nice if the basic python-ldap
library understood our utility types and could accept them as parameters
to the basic ldap functions and/or the basic ldap functions returned our
utility types.

Normally such a requirement would trivially be handled in an object-
oriented language (which Python is) by subclassing to extend and modify
the functionality. For some reason we didn't do this with the python-ldap
classes.

python-ldap objects are primarily used in two different places in our
code, ipaserver.ipaldap.py for the IPAdmin class and in
ipaserver/plugins/ldap2.py for the ldap2 class's .conn member.

In IPAdmin we use a IPA utility class called Entry to make it easier to
use the results returned by LDAP. The IPAdmin class is derived from
python-ldap.SimpleLDAPObject. But for some reason when we added the
support for the use of the Entry class in SimpleLDAPObject we didn't
subclass SimpleLDAPObject and extend it for use with the Entry class as
would be the normal expected methodology in an object-oriented language,
rather we used an obscure feature of the Python language to override all
methods of the SimpleLDAPObject class by wrapping those class methods in
another function call. The reason why this isn't a good approach is:

* It violates object-oriented methodology.

* Other classes cannot be derived and inherit the customization (because
the method wrapping occurs in a class instance, not within the class
type).

* It's non-obvious and obscure

* It's inefficient.

Here is a summary of what the code was doing:

It iterated over every member of the SimpleLDAPObject class and if it was
callable it wrapped the method. The wrapper function tested the name of
the method being wrapped, if it was one of a handful of methods we wanted
to customize we modified a parameter and called the original method. If
the method wasn't of interest to use we still wrapped the method.

It was inefficient because every non-customized method (the majority)
executed a function call for the wrapper, the wrapper during run-time used
logic to determine if the method was being overridden and then called the
original method. So every call to ldap was doing extra function calls and
logic processing which for the majority of cases produced nothing useful
(and was non-obvious from brief code reading some methods were being
overridden).

Object-orientated languages have support built in for calling the right
method for a given class object that do not involve extra function call
overhead to realize customized class behaviour. Also when programmers look
for customized class behaviour they look for derived classes. They might
also want to utilize the customized class as the base class for their use.

Also the wrapper logic was fragile, it did things like: if the method name
begins with "add" I'll unconditionally modify the first and second
argument. It would be some much cleaner if the "add", "add_s", etc.
methods were overridden in a subclass where the logic could be seen and
where it would apply to only the explicit functions and parameters being
overridden.

Also we would really benefit if there were classes which could be used as
a base class which had specific ldap customization.

At the moment our ldap customization needs are:

1) Support DN objects being passed to ldap operations

2) Support Entry & Entity objects being passed into and returned from
ldap operations.

We want to subclass the ldap SimpleLDAPObject class, that is the base
ldap class with all the ldap methods we're using. IPASimpleLDAPObject
class would subclass SimpleLDAPObject class which knows about DN
objects (and possilby other IPA specific types that are universally
used in IPA). Then  IPAEntrySimpleLDAPObject would subclass
IPASimpleLDAPObject which knows about Entry objects.

The reason for the suggested class hierarchy is because DN objects will be
used whenever we talk to LDAP (in the future we may want to add other IPA
specific classes which will always be used). We don't add Entry support to
the the IPASimpleLDAPObject class because Entry objects are (currently)
only used in IPAdmin.

What this patch does is:

* Introduce IPASimpleLDAPObject derived from
  SimpleLDAPObject. IPASimpleLDAPObject is DN object aware.

* Introduce IPAEntryLDAPObject derived from
  IPASimpleLDAPObject. IPAEntryLDAPObject is Entry object aware.

* Derive IPAdmin from IPAEntryLDAPObject and remove the funky method
  wrapping from IPAdmin.

* Code which called add_s() with an Entry or Entity object now calls
  addEntry(). addEntry() always existed, it just wasn't always
  used. add_s() had been modified to accept Entry or Entity object
  (why didn't we just call addEntry()?). The add*() ldap routine in
  IPAEntryLDAPObject have been subclassed to accept Entry and Entity
  objects, but that should proably be removed in the future and just
  use addEntry().

* Replace the call to ldap.initialize() in ldap2.create_connection()
  with a class constructor for IPASimpleLDAPObject. The
  ldap.initialize() is a convenience function in python-ldap, but it
  always returns a SimpleLDAPObject created via the SimpleLDAPObject
  constructor, thus ldap.initialize() did not allow subclassing, yet
  has no particular ease-of-use advantage thus we better off using the
  obvious class constructor mechanism.

* Fix the use of _handle_errors(), it's not necessary to construct an
  empty dict to pass to it.

If we follow the standard class derivation pattern for ldap we can make us
of our own ldap utilities in a far easier, cleaner and more efficient
manner.
---
 ipaserver/install/krbinstance.py |    4 +-
 ipaserver/install/service.py     |    2 +-
 ipaserver/ipaldap.py             |  105 ++++++++++++++------------------
 ipaserver/plugins/ldap2.py       |  124 ++++++++++++++++++++++++++++++++++---
 4 files changed, 164 insertions(+), 71 deletions(-)

diff --git a/ipaserver/install/krbinstance.py b/ipaserver/install/krbinstance.py
index 6ed38516223a56e723bcc9cf643ac2b362200da4..6566d8a3a4fb0743818b887f3d995a7ff3da8a65 100644
--- a/ipaserver/install/krbinstance.py
+++ b/ipaserver/install/krbinstance.py
@@ -259,7 +259,7 @@ class KrbInstance(service.Service):
         entry.setValues("nsSaslMapFilterTemplate", '(krbPrincipalName=\\1@\\2)')
 
         try:
-            self.admin_conn.add_s(entry)
+            self.admin_conn.addEntry(entry)
         except ldap.ALREADY_EXISTS:
             root_logger.critical("failed to add Full Principal Sasl mapping")
             raise e
@@ -272,7 +272,7 @@ class KrbInstance(service.Service):
         entry.setValues("nsSaslMapFilterTemplate", '(krbPrincipalName=&@%s)' % self.realm)
 
         try:
-            self.admin_conn.add_s(entry)
+            self.admin_conn.addEntry(entry)
         except ldap.ALREADY_EXISTS:
             root_logger.critical("failed to add Name Only Sasl mapping")
             raise e
diff --git a/ipaserver/install/service.py b/ipaserver/install/service.py
index 249727b15b6f3c879336c36d43410fc7234d9d9d..d9e6def370567f550a3dbe2c90637a7bbc8129ff 100644
--- a/ipaserver/install/service.py
+++ b/ipaserver/install/service.py
@@ -289,7 +289,7 @@ class Service(object):
                         "enabledService", "startOrder " + str(order))
 
         try:
-            conn.add_s(entry)
+            conn.addEntry(entry)
         except ldap.ALREADY_EXISTS, e:
             root_logger.critical("failed to add %s Service startup entry" % name)
             raise e
diff --git a/ipaserver/ipaldap.py b/ipaserver/ipaldap.py
index fdcbe62495805f76397e7588fe24041ce540b8c0..82feacbc1c69d543be1ede9a5dc6ffdba45b50e5 100644
--- a/ipaserver/ipaldap.py
+++ b/ipaserver/ipaldap.py
@@ -37,10 +37,52 @@ from ipaserver import ipautil
 from ipalib import errors
 from ipapython.ipautil import format_netloc
 from ipapython.entity import Entity
+from ipaserver.plugins.ldap2 import IPASimpleLDAPObject
 
 # Global variable to define SASL auth
 SASL_AUTH = ldap.sasl.sasl({},'GSSAPI')
 
+class IPAEntryLDAPObject(IPASimpleLDAPObject):
+    def __init__(self, *args, **kwds):
+        IPASimpleLDAPObject.__init__(self, *args, **kwds)
+
+    def result(self, msgid=ldap.RES_ANY, all=1, timeout=None):
+        objtype, data = IPASimpleLDAPObject.result(self, msgid, all, timeout)
+        # data is either a 2-tuple or a list of 2-tuples
+        if data:
+            if isinstance(data, tuple):
+                return objtype, Entry(data)
+            elif isinstance(data, list):
+                return objtype, [Entry(x) for x in data]
+            else:
+                raise TypeError, "unknown data type %s returned by result" % type(data)
+        else:
+            return objtype, data
+
+    def add(self, dn, modlist):
+        if isinstance(dn, Entry):
+            return IPASimpleLDAPObject.add(self, dn.dn, dn.toTupleList())
+        else:
+            return IPASimpleLDAPObject.add(self, dn, modlist)
+
+    def add_s(self, dn, modlist):
+        if isinstance(dn, Entry):
+            return IPASimpleLDAPObject.add_s(self, dn.dn, dn.toTupleList())
+        else:
+            return IPASimpleLDAPObject.add_s(self, dn, modlist)
+
+    def add_ext(self, dn, modlist, serverctrls=None, clientctrls=None):
+        if isinstance(dn, Entry):
+            return IPASimpleLDAPObject.add_ext(self, dn.dn, dn.toTupleList(), serverctrls, clientctrls)
+        else:
+            return IPASimpleLDAPObject.add_ext(self, dn, modlist, serverctrls, clientctrls)
+
+    def add_ext_s(self, dn, modlist, serverctrls=None, clientctrls=None):
+        if isinstance(dn, Entry):
+            return IPASimpleLDAPObject.add_ext_s(self, dn.dn, dn.toTupleList(), serverctrls, clientctrls)
+        else:
+            return IPASimpleLDAPObject.add_ext_s(self, dn, modlist, serverctrls, clientctrls)
+
 class Entry:
     """
     This class represents an LDAP Entry object.  An LDAP entry consists of
@@ -165,49 +207,7 @@ class Entry:
         ldif.LDIFWriter(sio,Entry.base64_attrs,1000).unparse(self.dn,newdata)
         return sio.getvalue()
 
-def wrapper(f,name):
-    """This is the method that wraps all of the methods of the superclass.
-       This seems to need to be an unbound method, that's why it's outside
-       of IPAdmin.  Perhaps there is some way to do this with the new
-       classmethod or staticmethod of 2.4. Basically, we replace every call
-       to a method in SimpleLDAPObject (the superclass of IPAdmin) with a
-       call to inner.  The f argument to wrapper is the bound method of
-       IPAdmin (which is inherited from the superclass).  Bound means that it
-       will implicitly be called with the self argument, it is not in the
-       args list.  name is the name of the method to call.  If name is a
-       method that returns entry objects (e.g. result), we wrap the data
-       returned by an Entry class.  If name is a method that takes an entry
-       argument, we extract the raw data from the entry object to pass in.
-    """
-    def inner(*args, **kargs):
-        if name == 'result':
-            objtype, data = f(*args, **kargs)
-            # data is either a 2-tuple or a list of 2-tuples
-            # print data
-            if data:
-                if isinstance(data,tuple):
-                    return objtype, Entry(data)
-                elif isinstance(data,list):
-                    return objtype, [Entry(x) for x in data]
-                else:
-                    raise TypeError, "unknown data type %s returned by result" % type(data)
-            else:
-                return objtype, data
-        elif name.startswith('add'):
-            # the first arg is self
-            # the second and third arg are the dn and the data to send
-            # We need to convert the Entry into the format used by
-            # python-ldap
-            ent = args[0]
-            if isinstance(ent,Entry):
-                return f(ent.dn, ent.toTupleList(), *args[2:])
-            else:
-                return f(*args, **kargs)
-        else:
-            return f(*args, **kargs)
-    return inner
-
-class IPAdmin(SimpleLDAPObject):
+class IPAdmin(IPAEntryLDAPObject):
 
     def __localinit(self):
         """If a CA certificate is provided then it is assumed that we are
@@ -218,12 +218,12 @@ class IPAdmin(SimpleLDAPObject):
            its own encryption.
         """
         if self.cacert is not None:
-            SimpleLDAPObject.__init__(self,'ldaps://%s' % format_netloc(self.host, self.port))
+            IPAEntryLDAPObject.__init__(self,'ldaps://%s' % format_netloc(self.host, self.port))
         else:
             if self.ldapi:
-                SimpleLDAPObject.__init__(self,'ldapi://%%2fvar%%2frun%%2fslapd-%s.socket' % "-".join(self.realm.split(".")))
+                IPAEntryLDAPObject.__init__(self,'ldapi://%%2fvar%%2frun%%2fslapd-%s.socket' % "-".join(self.realm.split(".")))
             else:
-                SimpleLDAPObject.__init__(self,'ldap://%s' % format_netloc(self.host, self.port))
+                IPAEntryLDAPObject.__init__(self,'ldap://%s' % format_netloc(self.host, self.port))
 
     def __init__(self,host='',port=389,cacert=None,bindcert=None,bindkey=None,proxydn=None,debug=None,ldapi=False,realm=None):
         """We just set our instance variables and wrap the methods - the real
@@ -240,7 +240,6 @@ class IPAdmin(SimpleLDAPObject):
         if bindkey is not None:
             ldap.set_option(ldap.OPT_X_TLS_KEYFILE,bindkey)
 
-        self.__wrapmethods()
         self.port = port
         self.host = host
         self.cacert = cacert
@@ -468,7 +467,7 @@ class IPAdmin(SimpleLDAPObject):
                 self.set_option(ldap.OPT_SERVER_CONTROLS, sctrl)
             self.add_s(entry.dn, entry.toTupleList())
         except ldap.LDAPError, e:
-            arg_desc = 'entry=%s, modlist=%s' % (entry)
+            arg_desc = 'entry=%s' % (entry)
             self.__handle_errors(e, arg_desc=arg_desc)
         return True
 
@@ -621,16 +620,6 @@ class IPAdmin(SimpleLDAPObject):
             self.__handle_errors(e)
         return True
 
-    def __wrapmethods(self):
-        """This wraps all methods of SimpleLDAPObject, so that we can intercept
-        the methods that deal with entries.  Instead of using a raw list of tuples
-        of lists of hashes of arrays as the entry object, we want to wrap entries
-        in an Entry class that provides some useful methods"""
-        for name in dir(self.__class__.__bases__[0]):
-            attr = getattr(self, name)
-            if callable(attr):
-                setattr(self, name, wrapper(attr, name))
-
     def waitForEntry(self, dn, timeout=7200, attr='', quiet=True):
         scope = ldap.SCOPE_BASE
         filter = "(objectclass=*)"
diff --git a/ipaserver/plugins/ldap2.py b/ipaserver/plugins/ldap2.py
index 1229e5bbcca0cd9ca8b06338b0e52da97d14611b..4bfc849d82cd4384afe9bad7b244184265315f7d 100644
--- a/ipaserver/plugins/ldap2.py
+++ b/ipaserver/plugins/ldap2.py
@@ -39,6 +39,7 @@ import pwd
 import krbV
 from ipapython.ipa_log_manager import *
 import ldap as _ldap
+from ldap.ldapobject import SimpleLDAPObject
 import ldap.filter as _ldap_filter
 import ldap.sasl as _ldap_sasl
 from ldap.controls import LDAPControl
@@ -55,7 +56,6 @@ from ipalib.crud import CrudBackend
 from ipalib.encoder import Encoder, encode_args, decode_retval
 from ipalib.request import context
 
-
 # Group Member types
 MEMBERS_ALL = 0
 MEMBERS_DIRECT = 1
@@ -80,6 +80,110 @@ def _encode_bool(self, value):
 # set own Bool parameter encoder
 Bool._encode = _encode_bool
 
+class IPASimpleLDAPObject(SimpleLDAPObject):
+    '''
+    This is a thin layer over SimpleLDAPObject which allows us to utilize
+    IPA specific types with the python-ldap API without the IPA caller needing
+    to perform the type translation, consider this a convenience layer for the
+    IPA programmer.
+
+    This subclass performs the following translations:
+
+    * DN objects may be passed into any ldap function expecting a dn. The DN
+      object will be converted to a string before being passed to the python-ldap
+      function. This allows us to maintain DN objects as DN objects in the rest
+      of the code (useful for DN manipulation and DN information) and not have
+      to worry about conversion to a string prior to passing it ldap.
+
+    '''
+    def __init__(self, *args, **kwds):
+        SimpleLDAPObject.__init__(self, *args, **kwds)
+
+    def add(self, dn, modlist):
+        return SimpleLDAPObject.add(self, str(dn), modlist)
+
+    def add_ext(self, dn, modlist, serverctrls=None, clientctrls=None):
+        return SimpleLDAPObject.add_ext(self, str(dn), modlist, serverctrls, clientctrls)
+
+    def add_ext_s(self, dn, modlist, serverctrls=None, clientctrls=None):
+        return SimpleLDAPObject.add_ext_s(self, str(dn), modlist, serverctrls, clientctrls)
+
+    def add_s(self, dn, modlist):
+        return SimpleLDAPObject.add_s(self, str(dn), modlist)
+
+    def compare(self, dn, attr, value):
+        return SimpleLDAPObject.compare(self, str(dn), attr, value)
+
+    def compare_ext(self, dn, attr, value, serverctrls=None, clientctrls=None):
+        return SimpleLDAPObject.compare_ext(self, str(dn), attr, value, serverctrls, clientctrls)
+
+    def compare_ext_s(self, dn, attr, value, serverctrls=None, clientctrls=None):
+        return SimpleLDAPObject.compare_ext_s(self, str(dn), attr, value, serverctrls, clientctrls)
+
+    def compare_s(self, dn, attr, value):
+        return SimpleLDAPObject.compare_s(self, str(dn), attr, value)
+
+    def delete(self, dn):
+        return SimpleLDAPObject.delete(self, str(dn))
+
+    def delete_ext(self, dn, serverctrls=None, clientctrls=None):
+        return SimpleLDAPObject.delete_ext(self, str(dn), serverctrls, clientctrls)
+
+    def delete_ext_s(self, dn, serverctrls=None, clientctrls=None):
+        return SimpleLDAPObject.delete_ext_s(self, str(dn), serverctrls, clientctrls)
+
+    def delete_s(self, dn):
+        return SimpleLDAPObject.delete_s(self, str(dn))
+
+    def modify(self, dn, modlist):
+        return SimpleLDAPObject.modify(self, str(dn), modlist)
+
+    def modify_ext(self, dn, modlist, serverctrls=None, clientctrls=None):
+        return SimpleLDAPObject.modify_ext(self, str(dn), modlist, serverctrls, clientctrls)
+
+    def modify_ext_s(self, dn, modlist, serverctrls=None, clientctrls=None):
+        return SimpleLDAPObject.modify_ext_s(self, str(dn), modlist, serverctrls, clientctrls)
+
+    def modify_s(self, dn, modlist):
+        return SimpleLDAPObject.modify_s(self, str(dn), modlist)
+
+    def modrdn(self, dn, newrdn, delold=1):
+        return SimpleLDAPObject.modrdn(self, str(dn), str(newrdn), delold)
+
+    def modrdn_s(self, dn, newrdn, delold=1):
+        return SimpleLDAPObject.modrdn_s(self, str(dn), str(newrdn), delold)
+
+    def read_subschemasubentry_s(self, subschemasubentry_dn, attrs=None):
+        return SimpleLDAPObject.read_subschemasubentry_s(self, str(subschemasubentry_dn), attrs)
+
+    def rename(self, dn, newrdn, newsuperior=None, delold=1, serverctrls=None, clientctrls=None):
+        return SimpleLDAPObject.rename(self, str(dn), str(newrdn), newsuperior, delold, serverctrls, clientctrls)
+
+    def rename_s(self, dn, newrdn, newsuperior=None, delold=1, serverctrls=None, clientctrls=None):
+        return SimpleLDAPObject.rename_s(self, str(dn), str(newrdn), newsuperior, delold, serverctrls, clientctrls)
+
+    def search(self, base, scope, filterstr='(objectClass=*)', attrlist=None, attrsonly=0):
+        return SimpleLDAPObject.search(self, str(base), scope, filterstr, attrlist, attrsonly)
+
+    def search_ext(self, base, scope, filterstr='(objectClass=*)', attrlist=None, attrsonly=0,
+                   serverctrls=None, clientctrls=None, timeout=-1, sizelimit=0):
+        return SimpleLDAPObject.search_ext(self, str(base), scope, filterstr, attrlist, attrsonly,
+                                           serverctrls, clientctrls, timeout, sizelimit)
+
+    def search_ext_s(self, base, scope, filterstr='(objectClass=*)', attrlist=None, attrsonly=0,
+                     serverctrls=None, clientctrls=None, timeout=-1, sizelimit=0):
+        return SimpleLDAPObject.search_ext_s(self, str(base), scope, filterstr, attrlist, attrsonly,
+                                             serverctrls, clientctrls, timeout, sizelimit)
+
+    def search_s(self, base, scope, filterstr='(objectClass=*)', attrlist=None, attrsonly=0):
+        return SimpleLDAPObject.search_s(self, str(base), scope, filterstr, attrlist, attrsonly)
+
+    def search_st(self, base, scope, filterstr='(objectClass=*)', attrlist=None, attrsonly=0, timeout=-1):
+        return SimpleLDAPObject.search_st(self, str(base), scope, filterstr, attrlist, attrsonly, timeout)
+
+    def search_subschemasubentry_s(self, dn=''):
+        return SimpleLDAPObject.search_subschemasubentry_s(self, str(dn))
+
 # universal LDAPError handler
 def _handle_errors(e, **kw):
     """
@@ -180,7 +284,7 @@ def get_schema(url, conn=None):
                 raise StandardError('Unable to retrieve LDAP schema. Error initializing principal %s in %s: %s' % (principal.name, '/etc/httpd/conf/ipa.keytab', str(e)))
 
         if conn is None:
-            conn = _ldap.initialize(url)
+            conn = IPASimpleLDAPObject(url)
             if url.startswith('ldapi://'):
                 conn.set_option(_ldap.OPT_HOST_NAME, api.env.host)
             conn.sasl_interactive_bind_s('', SASL_AUTH)
@@ -362,7 +466,7 @@ class ldap2(CrudBackend, Encoder):
                     conn.simple_bind_s(bind_dn, bind_pw)
 
         except _ldap.LDAPError, e:
-            _handle_errors(e, **{})
+            _handle_errors(e)
 
         if _schema:
             object.__setattr__(self, 'schema', _schema)
@@ -450,7 +554,7 @@ class ldap2(CrudBackend, Encoder):
         try:
             self.conn.add_s(dn, list(entry_attrs.iteritems()))
         except _ldap.LDAPError, e:
-            _handle_errors(e, **{})
+            _handle_errors(e)
 
     # generating filters for find_entry
     # some examples:
@@ -624,7 +728,7 @@ class ldap2(CrudBackend, Encoder):
                 _ldap.SIZELIMIT_EXCEEDED), e:
             truncated = True
         except _ldap.LDAPError, e:
-            _handle_errors(e, **{})
+            _handle_errors(e)
 
         if not res:
             raise errors.NotFound(reason='no such entry')
@@ -843,7 +947,7 @@ class ldap2(CrudBackend, Encoder):
             self.conn.rename_s(dn, new_rdn, delold=int(del_old))
             time.sleep(.3) # Give memberOf plugin a chance to work
         except _ldap.LDAPError, e:
-            _handle_errors(e, **{})
+            _handle_errors(e)
 
     def _generate_modlist(self, dn, entry_attrs, normalize):
         # get original entry
@@ -911,7 +1015,7 @@ class ldap2(CrudBackend, Encoder):
         try:
             self.conn.modify_s(dn, modlist)
         except _ldap.LDAPError, e:
-            _handle_errors(e, **{})
+            _handle_errors(e)
 
     @encode_args(1)
     def delete_entry(self, dn, normalize=True):
@@ -921,7 +1025,7 @@ class ldap2(CrudBackend, Encoder):
         try:
             self.conn.delete_s(dn)
         except _ldap.LDAPError, e:
-            _handle_errors(e, **{})
+            _handle_errors(e)
 
     @encode_args(1, 2, 3)
     def modify_password(self, dn, new_pass, old_pass=''):
@@ -941,7 +1045,7 @@ class ldap2(CrudBackend, Encoder):
         try:
             self.conn.passwd_s(dn, old_pass, new_pass)
         except _ldap.LDAPError, e:
-            _handle_errors(e, **{})
+            _handle_errors(e)
 
     def add_entry_to_group(self, dn, group_dn, member_attr='member', allow_same=False):
         """
@@ -1156,7 +1260,7 @@ class ldap2(CrudBackend, Encoder):
         try:
             self.conn.modify_s(dn, mod)
         except _ldap.LDAPError, e:
-            _handle_errors(e, **{})
+            _handle_errors(e)
 
     # CrudBackend methods
 
-- 
1.7.7.3



More information about the Freeipa-devel mailing list