Handle optional arguments by using the value __NONE__ over XML-RPC.

  rpcclient.py must call XML-RPC functions with all arguments.
Removed encode_args and decode_args. They were the source of most of the
  argument pain. Now opts is alwyas appended to the end of the arguments
  so MUST be the last argument in any server-side function (can be None)
Allow the User object to handle unicode data
Small fixes to command-line tools to be friendlier
Broke out get_user() into get_user_by_dn() and get_user_by_uid()
Need to request more than just 'nsAccountLock' attribute when trying to
  see if a user is already marked deleted. If it is blank the record
  coming back is empty. Add 'uid' to the list to guarantee something coming
  back (dn is handled specially)
Added user_container attribute to get_user_* and add_user so the caller
  can specify where in the tree the user will be searched for/added.
Added global default value for user_container

3 files changed, 47 insertions, 119 deletions

diff --git a/ipa-server/ipaserver/__init__.py b/ipa-server/ipaserver/__init__.py
index 6d254d6af..6aced0db7 100644
--- a/ipa-server/ipaserver/__init__.py
+++ b/ipa-server/ipaserver/__init__.py
@@ -20,38 +20,3 @@
 #
 
 __all__ = ["dsinstance", "krbinstance"]
-
-#
-# Functions common for the XML RPC client and server
-#
-# Authors:
-#       Mike McLean <mikem@redhat.com> (from koji)
-
-# functions for encoding/decoding optional arguments
-
-def encode_args(*args,**opts):
-    """The function encodes optional arguments as regular arguments.
-
-    This is used to allow optional arguments in xmlrpc calls
-    Returns a tuple of args
-    """
-    if opts:
-        opts['__starstar'] = True
-        args = args + (opts,)
-    return args
-
-def decode_args(*args):
-    """Decodes optional arguments from a flat argument list
-
-    Complementary to encode_args
-    Returns a tuple (args,opts) where args is a tuple and opts is a dict
-    """
-    opts = {}
-    if len(args) > 0:
-        last = args[-1]
-        if type(last) == dict and last.get('__starstar',False):
-            del last['__starstar']
-            opts = last
-            args = args[:-1]
-    return args,opts
-
diff --git a/ipa-server/xmlrpc-server/funcs.py b/ipa-server/xmlrpc-server/funcs.py
index ebf28af39..0af0f5ef4 100644
--- a/ipa-server/xmlrpc-server/funcs.py
+++ b/ipa-server/xmlrpc-server/funcs.py
@@ -36,6 +36,8 @@ import re
 # Need a global to store this between requests
 _LDAPPool = None
 
+DefaultContainer = "ou=users,ou=default"
+
 #
 # Apache runs in multi-process mode so each process will have its own
 # connection. This could theoretically drive the total number of connections
@@ -96,10 +98,10 @@ class IPAServer:
     
         # Convert to LDIF
         entry = str(ent) 
-    
+
         # Strip off any junk
         entry = entry.strip()
-    
+
         # Don't need to identify binary fields and this breaks the parser so
         # remove double colons
         entry = entry.replace('::', ':')
@@ -123,67 +125,49 @@ class IPAServer:
     
         return user
     
-    def get_user (self, args, sattrs=None, opts=None):
+    def __get_user (self, base, filter, sattrs=None, opts=None):
         """Get a specific user's entry. Return as a dict of values.
            Multi-valued fields are represented as lists.
         """
         global _LDAPPool
         ent=""
 
-        # The XML-RPC server marshals the arguments into one variable
-        # while the direct caller has them separate. So do a little
-        # bit of gymnastics to figure things out. There has to be a
-        # better way, so FIXME
-        if isinstance(args,tuple):
-            opts = sattrs
-            if len(args) == 2:
-                username = args[0]
-                sattrs = args[1]
-            else:
-                username = args
-                sattrs = None
-        else:
-            username = args
-
         if opts:
             self.set_principal(opts['remoteuser'])
-        if (isinstance(username, tuple)):
-            username = username[0]
     
         dn = self.get_dn_from_principal(self.princ)
     
-        filter = "(uid=" + username + ")"
-
         m1 = _LDAPPool.getConn(self.host,self.port,self.bindca,self.bindcert,self.bindkey,dn)
-        ent = m1.getEntry(self.basedn, self.scope, filter, sattrs)
+        ent = m1.getEntry(base, self.scope, filter, sattrs)
         _LDAPPool.releaseConn(m1)
     
         return self.convert_entry(ent)
+ 
+    def get_user_by_uid (self, uid, sattrs=None, opts=None):
+        """Get a specific user's entry. Return as a dict of values.
+           Multi-valued fields are represented as lists.
+        """
+
+        filter = "(uid=" + uid + ")"
+        return self.__get_user(self.basedn, filter, sattrs, opts)
     
-    def add_user (self, args, user_container="ou=users,ou=default",opts=None):
+    def get_user_by_dn (self, dn, sattrs=None, opts=None):
+        """Get a specific user's entry. Return as a dict of values.
+           Multi-valued fields are represented as lists.
+        """
+
+        filter = "(objectClass=*)"
+        return self.__get_user(dn, filter, sattrs, opts)
+    
+    def add_user (self, user, user_container=None, opts=None):
         """Add a user in LDAP. Takes as input a dict where the key is the
            attribute name and the value is either a string or in the case
            of a multi-valued field a list of values. user_container sets
            where in the tree the user is placed."""
         global _LDAPPool
 
-        # The XML-RPC server marshals the arguments into one variable
-        # while the direct caller has them separate. So do a little
-        # bit of gymnastics to figure things out. There has to be a
-        # better way, so FIXME
-        if isinstance(args,tuple):
-            opts = user_container
-            if len(args) == 2:
-                user = args[0]
-                user_container = args[1]
-            else:
-                user = args
-                user_container = "ou=users,ou=default"
-        else:
-            user = args
-
-        if (isinstance(user, tuple)):
-            user = user[0]
+        if user_container is None:
+            user_container = DefaultContainer
 
         dn="uid=%s,%s,%s" % (user['uid'], user_container,self.basedn)
         entry = ipaserver.ipaldap.Entry(dn)
@@ -283,26 +267,14 @@ class IPAServer:
     
         return users
 
-    def find_users (self, args, sattrs=None, opts=None):
+    def find_users (self, criteria, sattrs=None, user_container=None, opts=None):
         """Return a list containing a User object for each
         existing user that matches the criteria.
         """
         global _LDAPPool
 
-        # The XML-RPC server marshals the arguments into one variable
-        # while the direct caller has them separate. So do a little
-        # bit of gymnastics to figure things out. There has to be a
-        # better way, so FIXME
-        if isinstance(args,tuple):
-            opts = sattrs
-            if len(args) == 2:
-                criteria = args[0]
-                sattrs = args[1]
-            else:
-                criteria = args
-                sattrs = None
-        else:
-            criteria = args
+        if user_container is None:
+            user_container = DefaultContainer
 
         if opts:
             self.set_principal(opts['remoteuser'])
@@ -319,9 +291,10 @@ class IPAServer:
         # FIXME: Is this the filter we want or do we want to do searches of
         # cn as well? Or should the caller pass in the filter?
         filter = "(|(uid=%s)(cn=%s))" % (criteria, criteria)
+        basedn = user_container + "," +  self.basedn
         try:
             m1 = _LDAPPool.getConn(self.host,self.port,self.bindca,self.bindcert,self.bindkey,dn)
-            results = m1.getList(self.basedn, self.scope, filter, sattrs)
+            results = m1.getList(basedn, self.scope, filter, sattrs)
             _LDAPPool.releaseConn(m1)
         except ipaerror.exception_for(ipaerror.LDAP_NOT_FOUND):
             results = []
@@ -343,27 +316,10 @@ class IPAServer:
 
         return new_dict
 
-    def update_user (self, args, newuser=None, opts=None):
+    def update_user (self, olduser, newuser, opts=None):
         """Update a user in LDAP"""
         global _LDAPPool
 
-        # The XML-RPC server marshals the arguments into one variable
-        # while the direct caller has them separate. So do a little
-        # bit of gymnastics to figure things out. There has to be a
-        # better way, so FIXME
-        if isinstance(args,tuple):
-            opts = newuser
-            if len(args) == 2:
-                olduser = args[0]
-                newuser = args[1]
-        else:
-            olduser = args
-
-        if (isinstance(olduser, tuple)):
-            olduser = olduser[0]
-        if (isinstance(newuser, tuple)):
-            newuser = newuser[0]
-
         olduser = self.convert_scalar_values(olduser)
         newuser = self.convert_scalar_values(newuser)
 
@@ -385,22 +341,22 @@ class IPAServer:
         _LDAPPool.releaseConn(m1)
         return res
 
-    def mark_user_deleted (self, args, opts=None):
+    def mark_user_deleted (self, uid, opts=None):
         """Mark a user as inactive in LDAP. We aren't actually deleting
            users here, just making it so they can't log in, etc."""
         global _LDAPPool
 
-        uid = args[0]
-
         if opts:
             self.set_principal(opts['remoteuser'])
 
         proxydn = self.get_dn_from_principal(self.princ)
 
-        user = self.get_user(uid, ['dn', 'nsAccountlock'], opts)
+        user = self.get_user_by_uid(uid, ['dn', 'uid', 'nsAccountlock'], opts)
 
         # Are we doing an add or replace operation?
         if user.has_key('nsaccountlock'):
+            if user['nsaccountlock'] == "true":
+                return "already marked as deleted"
             has_key = True
         else:
             has_key = False
diff --git a/ipa-server/xmlrpc-server/ipaxmlrpc.py b/ipa-server/xmlrpc-server/ipaxmlrpc.py
index be4762c35..7998dd7e0 100644
--- a/ipa-server/xmlrpc-server/ipaxmlrpc.py
+++ b/ipa-server/xmlrpc-server/ipaxmlrpc.py
@@ -133,7 +133,9 @@ class ModXMLRPCRequestHandler(object):
         opts={}
         opts['remoteuser'] = remoteuser
 
-        params = ipaserver.encode_args(params, opts)
+        # Tack onto the end of the passed-in arguments any options we also
+        # need
+        params = params + (opts,)
 
         # special case
 #        if method == "get_user":
@@ -164,9 +166,13 @@ class ModXMLRPCRequestHandler(object):
         func = self.funcs.get(method,None)
         if func is None:
              raise Fault(1, "Invalid method: %s" % method)
-        params,opts = ipaserver.decode_args(*params)
-        
-        ret = func(*params,**opts)
+
+        args = list(params)
+        for i in range(len(args)):
+          if args[i] == '__NONE__':
+              args[i] = None 
+
+        ret = func(*args)
 
         return ret
 
@@ -281,7 +287,8 @@ def handler(req, profiling=False):
         try:
             f = funcs.IPAServer()
             h = ModXMLRPCRequestHandler()
-            h.register_function(f.get_user)
+            h.register_function(f.get_user_by_uid)
+            h.register_function(f.get_user_by_dn)
             h.register_function(f.add_user)
             h.register_function(f.get_add_schema)
             h.register_function(f.get_all_users)