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.

1 files changed, 114 insertions, 10 deletions

diff --git a/ipaserver/plugins/ldap2.py b/ipaserver/plugins/ldap2.py
index 1229e5bbc..4bfc849d8 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