Initial support for Groups

Create separate object for Users and Groups (using same base class)
Check for uniqueness before adding new users and groups
Remove user_container from everything but add operations
Abstract out a number of functions that are common across users and groups
Make sure all strings passed in to be in a filter are checked
Add new error message: No modifications specified

16 files changed, 1012 insertions, 229 deletions

diff --git a/ipa-admintools/ipa-addgroup b/ipa-admintools/ipa-addgroup
new file mode 100644
index 000000000..f590f5796
--- /dev/null
+++ b/ipa-admintools/ipa-addgroup
@@ -0,0 +1,83 @@
+#! /usr/bin/python -E
+# Authors: Rob Crittenden <rcritten@redhat.com>
+#
+# Copyright (C) 2007  Red Hat
+# see file 'COPYING' for use and warranty information
+#
+# This program is free software; you can redistribute it and/or
+# modify it under the terms of the GNU General Public License as
+# published by the Free Software Foundation; version 2 only
+#
+# This program is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+# GNU General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License
+# along with this program; if not, write to the Free Software
+# Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA 02111-1307 USA
+#
+
+import sys
+from optparse import OptionParser
+import ipa
+import ipa.group
+import ipa.ipaclient as ipaclient
+import ipa.config
+import ipa.ipaerror
+
+import xmlrpclib
+import kerberos
+import ldap
+
+def usage():
+    print "ipa-addgroup [-d|--description STRING] group"
+    sys.exit(1)
+
+def parse_options():
+    parser = OptionParser()
+    parser.add_option("-d", "--description", dest="desc",
+                      help="A description of this group")
+    parser.add_option("-g", "--gid", dest="gid",
+                      help="The gid to use for this group. If not included one is automatically set.")
+    parser.add_option("--usage", action="store_true",
+                      help="Program usage")
+
+    args = ipa.config.init_config(sys.argv)
+    options, args = parser.parse_args(args)
+
+    return options, args
+
+def main():
+    group=ipa.group.Group()
+    options, args = parse_options()
+
+    if len(args) != 2:
+        usage()
+
+    group.setValue('cn', args[1])
+    if options.desc:
+        group.setValue('description', options.desc)
+    if options.gid:
+        group.setValue('gidnumber', options.gid)
+
+    try:
+        client = ipaclient.IPAClient()
+        client.add_group(group)
+        print args[1] + " successfully added"
+    except xmlrpclib.Fault, f:
+        print f.faultString
+        return 1
+    except kerberos.GSSError, e:
+        print "Could not initialize GSSAPI: %s/%s" % (e[0][0][0], e[0][1][0])
+        return 1
+    except xmlrpclib.ProtocolError, e:
+        print "Unable to connect to IPA server: %s" % (e.errmsg)
+        return 1
+    except ipa.ipaerror.IPAError, e:
+        print "%s" % (e.message)
+        return 1
+
+    return 0
+
+main()
diff --git a/ipa-admintools/ipa-adduser b/ipa-admintools/ipa-adduser
index d2969036d..99aadee40 100644
--- a/ipa-admintools/ipa-adduser
+++ b/ipa-admintools/ipa-adduser
@@ -88,6 +88,9 @@ def main():
     except xmlrpclib.ProtocolError, e:
         print "Unable to connect to IPA server: %s" % (e.errmsg)
         return 1
+    except ipa.ipaerror.IPAError, e:
+        print "%s" % (e.message)
+        return 1
 
     return 0
 
diff --git a/ipa-admintools/ipa-findgroup b/ipa-admintools/ipa-findgroup
new file mode 100644
index 000000000..43171e424
--- /dev/null
+++ b/ipa-admintools/ipa-findgroup
@@ -0,0 +1,85 @@
+#! /usr/bin/python -E
+# Authors: Rob Crittenden <rcritten@redhat.com>
+#
+# Copyright (C) 2007  Red Hat
+# see file 'COPYING' for use and warranty information
+#
+# This program is free software; you can redistribute it and/or
+# modify it under the terms of the GNU General Public License as
+# published by the Free Software Foundation; version 2 only
+#
+# This program is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+# GNU General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License
+# along with this program; if not, write to the Free Software
+# Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA 02111-1307 USA
+#
+
+import sys
+from optparse import OptionParser
+import ipa.ipaclient as ipaclient
+import ipa.config
+
+import sys
+import xmlrpclib
+import kerberos
+
+def usage():
+    print "ipa-findgroup <uid>"
+    sys.exit()
+
+def parse_options():
+    parser = OptionParser()
+
+    args = ipa.config.init_config(sys.argv)
+    options, args = parser.parse_args(args)
+
+    return options, args
+
+def main():
+    group={}
+    options, args = parse_options()
+
+    if len(args) != 2:
+        usage()
+
+    try:
+        client = ipaclient.IPAClient()
+        groups = client.find_groups(args[1])
+
+        if len(groups) == 0:
+            print "No entries found for", args[1]
+            return 0
+
+        for ent in groups:
+            attr = ent.attrList()
+
+            print "dn: " + ent.dn
+
+            for a in attr:
+                value = ent.getValues(a)
+                if isinstance(value,str):
+                    print a + ": " + value
+                else:
+                    print a + ": "
+                    for l in value:
+                        print "\t" +  l
+            # blank line between results
+            print 
+
+    except xmlrpclib.Fault, fault:
+        print fault.faultString
+        return 1
+    except kerberos.GSSError, e:
+        print "Could not initialize GSSAPI: %s/%s" % (e[0][0][0], e[0][1][0])
+        return 1
+    except xmlrpclib.ProtocolError, e:
+        print "Unable to connect to IPA server: %s" % (e.errmsg)
+        return 1
+
+    return 0
+
+main()
diff --git a/ipa-admintools/ipa-groupmod b/ipa-admintools/ipa-groupmod
new file mode 100644
index 000000000..eea96b43a
--- /dev/null
+++ b/ipa-admintools/ipa-groupmod
@@ -0,0 +1,99 @@
+#! /usr/bin/python -E
+# Authors: Rob Crittenden <rcritten@redhat.com>
+#
+# Copyright (C) 2007  Red Hat
+# see file 'COPYING' for use and warranty information
+#
+# This program is free software; you can redistribute it and/or
+# modify it under the terms of the GNU General Public License as
+# published by the Free Software Foundation; version 2 only
+#
+# This program is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+# GNU General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License
+# along with this program; if not, write to the Free Software
+# Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA 02111-1307 USA
+#
+
+import sys
+from optparse import OptionParser
+import ipa
+import ipa.group
+import ipa.ipaclient as ipaclient
+import ipa.config
+import ipa.ipaerror
+
+import xmlrpclib
+import kerberos
+import ldap
+
+def usage():
+    print "ipa-groupmod [-a] [-r] user group"
+    print "ipa-groupmod [-d|--desc description STRING] group"
+    sys.exit(1)
+
+def parse_options():
+    parser = OptionParser()
+    parser.add_option("-a", "--add", dest="add", action="store_true",
+                      help="Add a user to the group")
+    parser.add_option("-r", "--remove", dest="remove", action="store_true",
+                      help="Remove a user from the group")
+    parser.add_option("-d", "--description", dest="desc", 
+                      help="Modify the description of the group")
+    parser.add_option("--usage", action="store_true",
+                      help="Program usage")
+
+    args = ipa.config.init_config(sys.argv)
+    options, args = parser.parse_args(args)
+
+    if (not options.add and not options.remove) and (not options.desc):
+        usage()
+
+    return options, args
+
+def main():
+    group=ipa.group.Group()
+    options, args = parse_options()
+
+    print "len = ", len(args)
+    if (options.add or options.remove) and (len(args) != 3):
+        usage()
+    if (options.desc and (len(args) != 2)):
+        usage()
+
+    try:
+        client = ipaclient.IPAClient()
+        if options.add:
+            client.add_user_to_group(args[1], args[2])
+            print args[1] + " successfully added"
+        elif options.remove:
+            client.remove_user_from_group(args[1], args[2])
+            print args[1] + " successfully removed"
+        elif options.desc:
+            try:
+                group = client.get_group_by_cn(args[1])
+            except ipa.ipaerror.IPAError, e:
+                print "%s" % e.message
+                return 1
+            group.setValue('description', options.desc)
+            client.update_group(group)
+            print args[1] + " successfully updated"
+    except xmlrpclib.Fault, f:
+        print f.faultString
+        return 1
+    except kerberos.GSSError, e:
+        print "Could not initialize GSSAPI: %s/%s" % (e[0][0][0], e[0][1][0])
+        return 1
+    except xmlrpclib.ProtocolError, e:
+        print "Unable to connect to IPA server: %s" % (e.errmsg)
+        return 1
+    except ipa.ipaerror.IPAError, e:
+        print "%s" % (e.message)
+        return 1
+
+    return 0
+
+main()
diff --git a/ipa-admintools/ipa-usermod b/ipa-admintools/ipa-usermod
index 3c9f8adc0..0c61f4097 100644
--- a/ipa-admintools/ipa-usermod
+++ b/ipa-admintools/ipa-usermod
@@ -54,7 +54,11 @@ def main():
         usage()
 
     client = ipaclient.IPAClient()
-    user = client.get_user_by_uid(args[1])
+    try:
+        user = client.get_user_by_uid(args[1])
+    except ipa.ipaerror.IPAError, e:
+        print "%s" % e.message
+        return 1
 
     if options.gecos:
         user.setValue('gecos', options.gecos)
diff --git a/ipa-python/entity.py b/ipa-python/entity.py
new file mode 100644
index 000000000..9153c4159
--- /dev/null
+++ b/ipa-python/entity.py
@@ -0,0 +1,153 @@
+import ldap
+import ldif
+import re
+import cStringIO
+
+def utf8_encode_value(value):
+    if isinstance(value,unicode):
+        return value.encode('utf-8')
+    return value
+
+def utf8_encode_values(values):
+    if isinstance(values,list) or isinstance(values,tuple):
+        return map(utf8_encode_value, values)
+    else:
+        return utf8_encode_value(values)
+
+class Entity:
+    """This class represents an IPA user.  An LDAP entry consists of a DN
+    and a list of attributes.  Each attribute consists of a name and a list of
+    values. For the time being I will maintain this.
+
+    In python-ldap, entries are returned as a list of 2-tuples.
+    Instance variables:
+    dn - string - the string DN of the entry
+    data - cidict - case insensitive dict of the attributes and values
+    orig_data - cidict - case insentiive dict of the original attributes and values"""
+
+    def __init__(self,entrydata=None):
+        """data is the raw data returned from the python-ldap result method,
+        which is a search result entry or a reference or None.
+        If creating a new empty entry, data is the string DN."""
+        if entrydata:
+            if isinstance(entrydata,tuple):
+                self.dn = entrydata[0]
+                self.data = ldap.cidict.cidict(entrydata[1])
+            elif isinstance(entrydata,str) or isinstance(entrydata,unicode):
+                self.dn = entrydata
+                self.data = ldap.cidict.cidict()
+            elif isinstance(entrydata,dict):
+                self.dn = entrydata['dn']
+                del entrydata['dn']
+                self.data = ldap.cidict.cidict(entrydata)
+        else:
+            self.dn = ''
+            self.data = ldap.cidict.cidict()
+
+        self.orig_data = dict(self.data)
+
+    def __nonzero__(self):
+        """This allows us to do tests like if entry: returns false if there is no data,
+        true otherwise"""
+        return self.data != None and len(self.data) > 0
+
+    def hasAttr(self,name):
+        """Return True if this entry has an attribute named name, False otherwise"""
+        return self.data and self.data.has_key(name)
+
+    def __setattr__(self,name,value):
+        """One should use setValue() or setValues() to set values except for
+           dn and data which are special."""
+        if name != 'dn' and name != 'data' and name != 'orig_data':
+            raise KeyError, 'use setValue() or setValues()'
+        else:
+            self.__dict__[name] = value
+
+    def __getattr__(self,name):
+        """If name is the name of an LDAP attribute, return the first value for that
+        attribute - equivalent to getValue - this allows the use of
+        entry.cn
+        instead of
+        entry.getValue('cn')
+        This also allows us to return None if an attribute is not found rather than
+        throwing an exception"""
+        return self.getValue(name)
+
+    def getValues(self,name):
+        """Get the list (array) of values for the attribute named name"""
+        return self.data.get(name)
+
+    def getValue(self,name):
+        """Get the first value for the attribute named name"""
+        value =  self.data.get(name,[None])
+        if isinstance(value,list) or isinstance(value,tuple):
+            return value[0]
+        else:
+            return value
+
+    def setValue(self,name,*value):
+        """Value passed in may be a single value, several values, or a single sequence.
+        For example:
+           ent.setValue('name', 'value')
+           ent.setValue('name', 'value1', 'value2', ..., 'valueN')
+           ent.setValue('name', ['value1', 'value2', ..., 'valueN'])
+           ent.setValue('name', ('value1', 'value2', ..., 'valueN'))
+        Since *value is a tuple, we may have to extract a list or tuple from that
+        tuple as in the last two examples above"""
+        if (len(value) < 1):
+            return
+        if (len(value) == 1):
+            self.data[name] = utf8_encode_values(value[0])
+        else:
+            self.data[name] = utf8_encode_values(value)
+
+    setValues = setValue
+
+    def toTupleList(self):
+        """Convert the attrs and values to a list of 2-tuples.  The first element
+        of the tuple is the attribute name.  The second element is either a
+        single value or a list of values."""
+        return self.data.items()
+
+    def toDict(self):
+        """Convert the attrs and values to a dict. The dict is keyed on the
+        attribute name.  The value is either single value or a list of values."""
+        result = {}
+        for k in self.data.keys():
+            result[k] = self.data[k]
+        result['dn'] = self.dn
+        return result
+
+    def attrList(self):
+        """Return a list of all attributes in the entry"""
+        return self.data.keys()
+
+    def origDataDict(self):
+        """Returns a dict of the original values of the user.  Used for updates."""
+        result = {}
+        for k in self.orig_data.keys():
+            result[k] = self.orig_data[k]
+        result['dn'] = self.dn
+        return result
+
+#    def __str__(self):
+#        """Convert the Entry to its LDIF representation"""
+#        return self.__repr__()
+#
+#    # the ldif class base64 encodes some attrs which I would rather see in raw form - to
+#    # encode specific attrs as base64, add them to the list below
+#    ldif.safe_string_re = re.compile('^$')
+#    base64_attrs = ['nsstate', 'krbprincipalkey', 'krbExtraData']
+#
+#    def __repr__(self):
+#        """Convert the Entry to its LDIF representation"""
+#        sio = cStringIO.StringIO()
+#        # what's all this then?  the unparse method will currently only accept
+#        # a list or a dict, not a class derived from them.  self.data is a
+#        # cidict, so unparse barfs on it.  I've filed a bug against python-ldap,
+#        # but in the meantime, we have to convert to a plain old dict for printing
+#        # I also don't want to see wrapping, so set the line width really high (1000)
+#        newdata = {}
+#        newdata.update(self.data)
+#        ldif.LDIFWriter(sio,User.base64_attrs,1000).unparse(self.dn,newdata)
+#        return sio.getvalue()
diff --git a/ipa-python/group.py b/ipa-python/group.py
new file mode 100644
index 000000000..d4a27624a
--- /dev/null
+++ b/ipa-python/group.py
@@ -0,0 +1,7 @@
+from ipa.entity import Entity
+
+class Group(Entity):
+
+    def __init2__(self):
+        pass
+        
diff --git a/ipa-python/ipaclient.py b/ipa-python/ipaclient.py
index dc8e1dc9b..28573acad 100644
--- a/ipa-python/ipaclient.py
+++ b/ipa-python/ipaclient.py
@@ -26,6 +26,7 @@ sys.path.append("/usr/share/ipa")
 from ipaserver import funcs
 import ipa.rpcclient as rpcclient
 import user
+import group
 import ipa
 import config
 
@@ -54,20 +55,23 @@ class IPAClient:
         if self.local:
             self.transport.set_principal(princ)
 
+# User support
     def get_user_by_uid(self,uid,sattrs=None):
         """Get a specific user by uid. If sattrs is set then only those
-           attributes will be returned."""
+           attributes will be returned, otherwise all available attributes
+           are returned."""
         result = self.transport.get_user_by_uid(uid,sattrs)
         return user.User(result)
 
     def get_user_by_dn(self,dn,sattrs=None):
-        """Get a specific user by uid. If sattrs is set then only those
-           attributes will be returned."""
+        """Get a specific user by dn. If sattrs is set then only those
+           attributes will be returned, otherwise all available attributes
+           are returned."""
         result = self.transport.get_user_by_dn(dn,sattrs)
         return user.User(result)
 
     def add_user(self,user,user_container=None):
-        """Add a user. user is a ipa.user object"""
+        """Add a user. user is a ipa.user.User object"""
 
         realm = config.config.get_realm()
 
@@ -97,10 +101,10 @@ class IPAClient:
         result = self.transport.get_add_schema()
         return result
 
-    def find_users(self, criteria, sattrs=None, user_container=None):
+    def find_users(self, criteria, sattrs=None):
         """Find users whose uid matches the criteria. Wildcards are 
            acceptable. Returns a list of User objects."""
-        result = self.transport.find_users(criteria, sattrs, user_container)
+        result = self.transport.find_users(criteria, sattrs)
 
         users = []
         for (attrs) in result:
@@ -124,3 +128,89 @@ class IPAClient:
 
         result = self.transport.mark_user_deleted(uid)
         return result
+
+# Groups support
+
+    def get_group_by_cn(self,cn,sattrs=None):
+        """Get a specific group by cn. If sattrs is set then only those
+           attributes will be returned, otherwise all available attributes
+           are returned."""
+        result = self.transport.get_group_by_cn(cn,sattrs)
+        return group.Group(result)
+
+    def get_group_by_dn(self,dn,sattrs=None):
+        """Get a specific group by cn. If sattrs is set then only those
+           attributes will be returned, otherwise all available attributes
+           are returned."""
+        result = self.transport.get_group_by_dn(dn,sattrs)
+        return group.Group(result)
+
+    def add_group(self,group,group_container=None):
+        """Add a group. group is a ipa.group.Group object"""
+
+        realm = config.config.get_realm()
+
+        group_dict = group.toDict()
+
+        # dn is set on the server-side
+        del group_dict['dn']
+
+        # convert to a regular dict before sending
+        result = self.transport.add_group(group_dict, group_container)
+        return result
+
+    def find_groups(self, criteria, sattrs=None):
+        """Find groups whose cn matches the criteria. Wildcards are 
+           acceptable. Returns a list of Group objects."""
+        result = self.transport.find_groups(criteria, sattrs)
+
+        groups = []
+        for (attrs) in result:
+            if attrs is not None:
+                groups.append(group.Group(attrs))
+
+        return groups
+
+    def add_user_to_group(self, user, group):
+        """Add a user to an existing group.
+           user is a uid of the user to add
+           group is the cn of the group to be added to
+        """
+
+        return self.transport.add_user_to_group(user, group)
+
+    def add_users_to_group(self, users, group):
+        """Add several users to an existing group.
+           user is a list of uids of the users to add
+           group is the cn of the group to be added to
+
+           Returns a list of the users that were not added.
+        """
+
+        return self.transport.add_users_to_group(users, group)
+
+    def remove_user_from_group(self, user, group):
+        """Remove a user from an existing group.
+           user is a uid of the user to remove
+           group is the cn of the group to be removed from
+        """
+
+        return self.transport.remove_user_from_group(user, group)
+
+    def remove_users_from_group(self, users, group):
+        """Remove several users from an existing group.
+           user is a list of uids of the users to remove
+           group is the cn of the group to be removed from
+
+           Returns a list of the users that were not removed.
+        """
+
+        return self.transport.remove_users_from_group(users, group)
+
+    def update_group(self,group):
+        """Update a group entry."""
+
+        realm = config.config.get_realm()
+
+        result = self.transport.update_group(group.origDataDict(), group.toDict())
+        return result
diff --git a/ipa-python/ipaerror.py b/ipa-python/ipaerror.py
index c637aeef0..efcd855f3 100644
--- a/ipa-python/ipaerror.py
+++ b/ipa-python/ipaerror.py
@@ -115,6 +115,11 @@ LDAP_MISSING_DN = gen_error_code(
         0x0005,
         "Entry missing dn")
 
+LDAP_EMPTY_MODLIST = gen_error_code(
+        LDAP_CATEGORY,
+        0x0006,
+        "No modifications to be performed")
+
 #
 # Input errors  (sample - replace me)
 #
diff --git a/ipa-python/rpcclient.py b/ipa-python/rpcclient.py
index cd94eb99c..2301edde8 100644
--- a/ipa-python/rpcclient.py
+++ b/ipa-python/rpcclient.py
@@ -51,25 +51,28 @@ class RPCClient:
     def convert_entry(self,ent):
         # Convert into a dict. We need to handle multi-valued attributes as well
         # so we'll convert those into lists.
-        user={}
+        obj={}
         for (k) in ent:
             k = k.lower()
-            if user.get(k) is not None:
-                if isinstance(user[k],list):
-                    user[k].append(ent[k].strip())
+            if obj.get(k) is not None:
+                if isinstance(obj[k],list):
+                    obj[k].append(ent[k].strip())
                 else:
-                    first = user[k]
-                    user[k] = ()
-                    user[k].append(first)
-                    user[k].append(ent[k].strip())
+                    first = obj[k]
+                    obj[k] = ()
+                    obj[k].append(first)
+                    obj[k].append(ent[k].strip())
             else:
-                 user[k] = ent[k]
+                 obj[k] = ent[k]
     
-        return user
+        return obj 
         
+# User support
+
     def get_user_by_uid(self,uid,sattrs=None):
         """Get a specific user. If sattrs is not None then only those
-           attributes will be returned. The result is a dict."""
+           attributes will be returned, otherwise all available
+           attributes are returned. The result is a dict."""
         server = self.setup_server()
         if sattrs is None:
             sattrs = "__NONE__"
@@ -84,7 +87,8 @@ class RPCClient:
         
     def get_user_by_dn(self,dn,sattrs=None):
         """Get a specific user. If sattrs is not None then only those
-           attributes will be returned. The result is a dict."""
+           attributes will be returned, otherwise all available
+           attributes are returned. The result is a dict."""
         server = self.setup_server()
         if sattrs is None:
             sattrs = "__NONE__"
@@ -145,7 +149,7 @@ class RPCClient:
     
         return result
 
-    def find_users (self, criteria, sattrs=None, user_container=None):
+    def find_users (self, criteria, sattrs=None):
         """Return a list containing a User object for each user that matches
            the criteria."""
     
@@ -154,9 +158,7 @@ class RPCClient:
             # None values are not allowed in XML-RPC
             if sattrs is None:
                 sattrs = "__NONE__"
-            if user_container is None:
-                user_container = "__NONE__"
-            result = server.find_users(criteria, sattrs, user_container)
+            result = server.find_users(criteria, sattrs)
         except xmlrpclib.Fault, fault:
             raise ipaerror.gen_exception(fault.faultCode, fault.faultString)
         except socket.error, (value, msg):
@@ -189,3 +191,147 @@ class RPCClient:
             raise xmlrpclib.Fault(value, msg)
 
         return result
+
+# Group support
+        
+    def get_group_by_cn(self,cn,sattrs=None):
+        """Get a specific group. If sattrs is not None then only those
+           attributes will be returned, otherwise all available
+           attributes are returned. The result is a dict."""
+        server = self.setup_server()
+        if sattrs is None:
+            sattrs = "__NONE__"
+        try:
+            result = server.get_group_by_cn(cn, sattrs)
+        except xmlrpclib.Fault, fault:
+            raise ipaerror.gen_exception(fault.faultCode, fault.faultString)
+        except socket.error, (value, msg):
+            raise xmlrpclib.Fault(value, msg)
+
+        return result
+        
+    def get_group_by_dn(self,dn,sattrs=None):
+        """Get a specific group. If sattrs is not None then only those
+           attributes will be returned, otherwise all available
+           attributes are returned. The result is a dict."""
+        server = self.setup_server()
+        if sattrs is None:
+            sattrs = "__NONE__"
+        try:
+            result = server.get_group_by_dn(dn, sattrs)
+        except xmlrpclib.Fault, fault:
+            raise ipaerror.gen_exception(fault.faultCode, fault.faultString)
+        except socket.error, (value, msg):
+            raise xmlrpclib.Fault(value, msg)
+
+        return result
+
+    def add_group(self,group,group_container=None):
+        """Add a new group. 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"""
+        server = self.setup_server()
+
+        if group_container is None:
+            group_container = "__NONE__"
+    
+        try:
+            result = server.add_group(group, group_container)
+        except xmlrpclib.Fault, fault:
+            raise ipaerror.gen_exception(fault.faultCode, fault.faultString)
+        except socket.error, (value, msg):
+            raise xmlrpclib.Fault(value, msg)
+
+    def find_groups (self, criteria, sattrs=None):
+        """Return a list containing a Group object for each group that matches
+           the criteria."""
+    
+        server = self.setup_server()
+        try:
+            # None values are not allowed in XML-RPC
+            if sattrs is None:
+                sattrs = "__NONE__"
+            result = server.find_groups(criteria, sattrs)
+        except xmlrpclib.Fault, fault:
+            raise ipaerror.gen_exception(fault.faultCode, fault.faultString)
+        except socket.error, (value, msg):
+            raise xmlrpclib.Fault(value, msg)
+    
+        return result
+
+    def add_user_to_group(self, user, group):
+        """Add a user to an existing group.
+           user is a uid of the user to add
+           group is the cn of the group to be added to
+        """
+        server = self.setup_server()
+        try:
+            result = server.add_user_to_group(user, group)
+        except xmlrpclib.Fault, fault:
+            raise ipaerror.gen_exception(fault.faultCode, fault.faultString)
+        except socket.error, (value, msg):
+            raise xmlrpclib.Fault(value, msg)
+    
+        return result
+
+    def add_users_to_group(self, users, group):
+        """Add several users to an existing group.
+           user is a list of the uids of the users to add
+           group is the cn of the group to be added to
+
+           Returns a list of the users that were not added.
+        """
+        server = self.setup_server()
+        try:
+            result = server.add_users_to_group(users, group)
+        except xmlrpclib.Fault, fault:
+            raise ipaerror.gen_exception(fault.faultCode, fault.faultString)
+        except socket.error, (value, msg):
+            raise xmlrpclib.Fault(value, msg)
+    
+        return result
+
+    def remove_user_from_group(self, user, group):
+        """Remove a user from an existing group.
+           user is a uid of the user to remove
+           group is the cn of the group to be removed from
+        """
+        server = self.setup_server()
+        try:
+            result = server.remove_user_from_group(user, group)
+        except xmlrpclib.Fault, fault:
+            raise ipaerror.gen_exception(fault.faultCode, fault.faultString)
+        except socket.error, (value, msg):
+            raise xmlrpclib.Fault(value, msg)
+    
+        return result
+
+    def remove_users_from_group(self, users, group):
+        """Remove several users from an existing group.
+           user is a list of the uids of the users to remove
+           group is the cn of the group to be removed from
+
+           Returns a list of the users that were not removed.
+        """
+        server = self.setup_server()
+        try:
+            result = server.remove_users_from_group(users, group)
+        except xmlrpclib.Fault, fault:
+            raise ipaerror.gen_exception(fault.faultCode, fault.faultString)
+        except socket.error, (value, msg):
+            raise xmlrpclib.Fault(value, msg)
+    
+        return result
+
+    def update_group(self,oldgroup,newgroup):
+        """Update an existing group. oldgroup and newgroup are dicts of attributes"""
+        server = self.setup_server()
+    
+        try:
+            result = server.update_group(oldgroup, newgroup)
+        except xmlrpclib.Fault, fault:
+            raise ipaerror.gen_exception(fault.faultCode, fault.faultString)
+        except socket.error, (value, msg):
+            raise xmlrpclib.Fault(value, msg)
+
+        return result
diff --git a/ipa-python/user.py b/ipa-python/user.py
index 9d547e80d..cd908e3af 100644
--- a/ipa-python/user.py
+++ b/ipa-python/user.py
@@ -1,153 +1,7 @@
-import ldap
-import ldif
-import re
-import cStringIO
+from ipa.entity import Entity
 
-def utf8_encode_value(value):
-    if isinstance(value,unicode):
-        return value.encode('utf-8')
-    return value
+class User(Entity):
 
-def utf8_encode_values(values):
-    if isinstance(values,list) or isinstance(values,tuple):
-        return map(utf8_encode_value, values)
-    else:
-        return utf8_encode_value(values)
-
-class User:
-    """This class represents an IPA user.  An LDAP entry consists of a DN
-    and a list of attributes.  Each attribute consists of a name and a list of
-    values. For the time being I will maintain this.
-
-    In python-ldap, entries are returned as a list of 2-tuples.
-    Instance variables:
-    dn - string - the string DN of the entry
-    data - cidict - case insensitive dict of the attributes and values
-    orig_data - cidict - case insentiive dict of the original attributes and values"""
-
-    def __init__(self,entrydata=None):
-        """data is the raw data returned from the python-ldap result method,
-        which is a search result entry or a reference or None.
-        If creating a new empty entry, data is the string DN."""
-        if entrydata:
-            if isinstance(entrydata,tuple):
-                self.dn = entrydata[0]
-                self.data = ldap.cidict.cidict(entrydata[1])
-            elif isinstance(entrydata,str) or isinstance(entrydata,unicode):
-                self.dn = entrydata
-                self.data = ldap.cidict.cidict()
-            elif isinstance(entrydata,dict):
-                self.dn = entrydata['dn']
-                del entrydata['dn']
-                self.data = ldap.cidict.cidict(entrydata)
-        else:
-            self.dn = ''
-            self.data = ldap.cidict.cidict()
-
-        self.orig_data = dict(self.data)
-
-    def __nonzero__(self):
-        """This allows us to do tests like if entry: returns false if there is no data,
-        true otherwise"""
-        return self.data != None and len(self.data) > 0
-
-    def hasAttr(self,name):
-        """Return True if this entry has an attribute named name, False otherwise"""
-        return self.data and self.data.has_key(name)
-
-    def __setattr__(self,name,value):
-        """One should use setValue() or setValues() to set values except for
-           dn and data which are special."""
-        if name != 'dn' and name != 'data' and name != 'orig_data':
-            raise KeyError, 'use setValue() or setValues()'
-        else:
-            self.__dict__[name] = value
-
-    def __getattr__(self,name):
-        """If name is the name of an LDAP attribute, return the first value for that
-        attribute - equivalent to getValue - this allows the use of
-        entry.cn
-        instead of
-        entry.getValue('cn')
-        This also allows us to return None if an attribute is not found rather than
-        throwing an exception"""
-        return self.getValue(name)
-
-    def getValues(self,name):
-        """Get the list (array) of values for the attribute named name"""
-        return self.data.get(name)
-
-    def getValue(self,name):
-        """Get the first value for the attribute named name"""
-        value =  self.data.get(name,[None])
-        if isinstance(value,list) or isinstance(value,tuple):
-            return value[0]
-        else:
-            return value
-
-    def setValue(self,name,*value):
-        """Value passed in may be a single value, several values, or a single sequence.
-        For example:
-           ent.setValue('name', 'value')
-           ent.setValue('name', 'value1', 'value2', ..., 'valueN')
-           ent.setValue('name', ['value1', 'value2', ..., 'valueN'])
-           ent.setValue('name', ('value1', 'value2', ..., 'valueN'))
-        Since *value is a tuple, we may have to extract a list or tuple from that
-        tuple as in the last two examples above"""
-        if (len(value) < 1):
-            return
-        if (len(value) == 1):
-            self.data[name] = utf8_encode_values(value[0])
-        else:
-            self.data[name] = utf8_encode_values(value)
-
-    setValues = setValue
-
-    def toTupleList(self):
-        """Convert the attrs and values to a list of 2-tuples.  The first element
-        of the tuple is the attribute name.  The second element is either a
-        single value or a list of values."""
-        return self.data.items()
-
-    def toDict(self):
-        """Convert the attrs and values to a dict. The dict is keyed on the
-        attribute name.  The value is either single value or a list of values."""
-        result = {}
-        for k in self.data.keys():
-            result[k] = self.data[k]
-        result['dn'] = self.dn
-        return result
-
-    def attrList(self):
-        """Return a list of all attributes in the entry"""
-        return self.data.keys()
-
-    def origDataDict(self):
-        """Returns a dict of the original values of the user.  Used for updates."""
-        result = {}
-        for k in self.orig_data.keys():
-            result[k] = self.orig_data[k]
-        result['dn'] = self.dn
-        return result
-
-#    def __str__(self):
-#        """Convert the Entry to its LDIF representation"""
-#        return self.__repr__()
-#
-#    # the ldif class base64 encodes some attrs which I would rather see in raw form - to
-#    # encode specific attrs as base64, add them to the list below
-#    ldif.safe_string_re = re.compile('^$')
-#    base64_attrs = ['nsstate', 'krbprincipalkey', 'krbExtraData']
-#
-#    def __repr__(self):
-#        """Convert the Entry to its LDIF representation"""
-#        sio = cStringIO.StringIO()
-#        # what's all this then?  the unparse method will currently only accept
-#        # a list or a dict, not a class derived from them.  self.data is a
-#        # cidict, so unparse barfs on it.  I've filed a bug against python-ldap,
-#        # but in the meantime, we have to convert to a plain old dict for printing
-#        # I also don't want to see wrapping, so set the line width really high (1000)
-#        newdata = {}
-#        newdata.update(self.data)
-#        ldif.LDIFWriter(sio,User.base64_attrs,1000).unparse(self.dn,newdata)
-#        return sio.getvalue()
+    def __init2__(self):
+        pass
+        
diff --git a/ipa-server/ipa-install/share/bootstrap-template.ldif b/ipa-server/ipa-install/share/bootstrap-template.ldif
index 444a29d44..2986f3ab0 100644
--- a/ipa-server/ipa-install/share/bootstrap-template.ldif
+++ b/ipa-server/ipa-install/share/bootstrap-template.ldif
@@ -53,4 +53,6 @@ changetype: add
 description: ou=users administrators
 objectClass: top
 objectClass: groupofuniquenames
+objectClass: posixGroup
+gidNumber: 500
 cn: admin
diff --git a/ipa-server/ipa-install/share/default-aci.ldif b/ipa-server/ipa-install/share/default-aci.ldif
index a32729a3b..2b05e102a 100644
--- a/ipa-server/ipa-install/share/default-aci.ldif
+++ b/ipa-server/ipa-install/share/default-aci.ldif
@@ -9,4 +9,6 @@ aci: (targetattr="*")(version 3.0; acl "Directory Administrators can manage all
 aci: (targetattr="userPassword || krbPrincipalKey ||sambaLMPassword || sambaNTPassword")(version 3.0; acl "Kpasswd access to passowrd hashes for passowrd changes"; allow (all) userdn="ldap:///krbprincipalname=kadmin/changepw@$REALM,cn=$REALM,cn=kerberos,$SUFFIX";)
 aci: (target="ldap:///uid=*,ou=users,ou=default,$SUFFIX")(targetattr="*")(version 3.0; acl "allowproxy-webservice"; allow (proxy) userdn="ldap:///uid=webservice,ou=special,$SUFFIX";)
 aci: (target="ldap:///uid=*,ou=users,ou=default,$SUFFIX")(targetattr="*")(version 3.0; acl "admins can write entries"; allow(add,delete,write)groupdn="ldap:///cn=admin,ou=groups,ou=default,$SUFFIX";) 
+aci: (target="ldap:///cn=*,ou=groups,ou=default,$SUFFIX")(targetattr="*")(version 3.0; acl "allowproxy-webservice"; allow (proxy) userdn="ldap:///uid=webservice,ou=special,$SUFFIX";)
+aci: (target="ldap:///cn=*,ou=groups,ou=default,$SUFFIX")(targetattr="*")(version 3.0; acl "admins can write entries"; allow(add,delete,write)groupdn="ldap:///cn=admin,ou=groups,ou=default,$SUFFIX";) 
 aci: (targetattr="userPrincipal")(version 3.0; acl "allow webservice to find users by kerberos principal name"; allow (read, search) userdn="ldap:///uid=webservice,ou=special,$SUFFIX";) 
diff --git a/ipa-server/ipaserver/ipaldap.py b/ipa-server/ipaserver/ipaldap.py
index aaa4a35a6..0b2a152e9 100644
--- a/ipa-server/ipaserver/ipaldap.py
+++ b/ipa-server/ipaserver/ipaldap.py
@@ -320,6 +320,9 @@ class IPAdmin(SimpleLDAPObject):
 
         modlist = self.generateModList(olduser, newuser)
 
+        if len(modlist) == 0:
+            raise ipaerror.gen_exception(ipaerror.LDAP_EMPTY_MODLIST)
+
         try:
             self.set_option(ldap.OPT_SERVER_CONTROLS, sctrl)
             self.modify_s(dn, modlist)
diff --git a/ipa-server/xmlrpc-server/funcs.py b/ipa-server/xmlrpc-server/funcs.py
index aa5633453..f388f3f4f 100644
--- a/ipa-server/xmlrpc-server/funcs.py
+++ b/ipa-server/xmlrpc-server/funcs.py
@@ -26,6 +26,7 @@ import ipaserver.ipaldap
 import ipaserver.util
 import xmlrpclib
 import ipa.config
+import copy
 from ipa import ipaerror
 
 import string
@@ -36,7 +37,8 @@ import re
 # Need a global to store this between requests
 _LDAPPool = None
 
-DefaultContainer = "ou=users,ou=default"
+DefaultUserContainer = "ou=users,ou=default"
+DefaultGroupContainer = "ou=groups,ou=default"
 
 #
 # Apache runs in multi-process mode so each process will have its own
@@ -85,7 +87,6 @@ class IPAServer:
         """Given a kerberls principal get the LDAP uid"""
         global _LDAPPool
 
-        # FIXME: should we search for this in a specific area of the tree?
         filter = "(krbPrincipalName=" + princ + ")"
         # The only anonymous search we should have
         m1 = _LDAPPool.getConn(self.host,self.port,self.bindca,self.bindcert,self.bindkey,None)
@@ -93,7 +94,7 @@ class IPAServer:
         _LDAPPool.releaseConn(m1)
     
         return "dn:" + ent.dn
-    
+
     def convert_entry(self, ent):
     
         # Convert to LDIF
@@ -109,24 +110,24 @@ class IPAServer:
     
         # Convert into a dict. We need to handle multi-valued attributes as well
         # so we'll convert those into lists.
-        user={}
+        obj={}
         for (k,v) in specs:
             k = k.lower()
-            if user.get(k) is not None:
-                if isinstance(user[k],list):
-                    user[k].append(v.strip())
+            if obj.get(k) is not None:
+                if isinstance(obj[k],list):
+                    obj[k].append(v.strip())
                 else:
-                    first = user[k]
-                    user[k] = []
-                    user[k].append(first)
-                    user[k].append(v.strip())
+                    first = obj[k]
+                    obj[k] = []
+                    obj[k].append(first)
+                    obj[k].append(v.strip())
             else:
-                    user[k] = v.strip()
+                    obj[k] = v.strip()
     
-        return user
+        return obj 
     
-    def __get_user (self, base, filter, sattrs=None, opts=None):
-        """Get a specific user's entry. Return as a dict of values.
+    def __get_entry (self, base, filter, sattrs=None, opts=None):
+        """Get a specific entry. Return as a dict of values.
            Multi-valued fields are represented as lists.
         """
         global _LDAPPool
@@ -142,14 +143,70 @@ class IPAServer:
         _LDAPPool.releaseConn(m1)
     
         return self.convert_entry(ent)
+
+    def __update_entry (self, oldentry, newentry, opts=None):
+        """Update an LDAP entry
+
+           oldentry is a dict
+           newentry is a dict
+        """
+        global _LDAPPool
+
+        oldentry = self.convert_scalar_values(oldentry)
+        newentry = self.convert_scalar_values(newentry)
+
+        # Should be able to get this from either the old or new entry
+        # but just in case someone has decided to try changing it, use the
+        # original
+        try:
+            moddn = oldentry['dn']
+        except KeyError, e:
+            raise ipaerror.gen_exception(ipaerror.LDAP_MISSING_DN)
+
+        if opts:
+            self.set_principal(opts['remoteuser'])
+    
+        proxydn = self.get_dn_from_principal(self.princ)
+
+        m1 = _LDAPPool.getConn(self.host,self.port,self.bindca,self.bindcert,self.bindkey,proxydn)
+        res = m1.updateEntry(moddn, oldentry, newentry)
+        _LDAPPool.releaseConn(m1)
+        return res
+
+    def __safe_filter(self, criteria):
+        """Make sure any arguments used when creating a filter are safe."""
+
+        # TODO: this escaper assumes the python-ldap library will error out
+        #       on invalid codepoints.  we need to check malformed utf-8 input
+        #       where the second byte in a multi-byte character
+        #       is (illegally) ')' and make sure python-ldap
+        #       bombs out.
+        criteria = re.sub(r'[\(\)\\]', ldap_search_escape, criteria)
+
+        return criteria
  
+# User support
+
+    def __is_user_unique(self, uid, opts):
+        """Return 1 if the uid is unique in the tree, 0 otherwise."""
+        uid = self.__safe_filter(uid)
+        filter = "(&(uid=%s)(objectclass=posixAccount))" % uid
+ 
+        entry = self.__get_entry(self.basedn, filter, ['dn','uid'], opts)
+
+        if entry is not None:
+            return 0
+        else:
+            return 1
+
     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.
         """
 
+        uid = self.__safe_filter(uid)
         filter = "(uid=" + uid + ")"
-        return self.__get_user(self.basedn, filter, sattrs, opts)
+        return self.__get_entry(self.basedn, filter, sattrs, opts)
     
     def get_user_by_dn (self, dn, sattrs=None, opts=None):
         """Get a specific user's entry. Return as a dict of values.
@@ -157,7 +214,7 @@ class IPAServer:
         """
 
         filter = "(objectClass=*)"
-        return self.__get_user(dn, filter, sattrs, opts)
+        return self.__get_entry(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
@@ -167,7 +224,10 @@ class IPAServer:
         global _LDAPPool
 
         if user_container is None:
-            user_container = DefaultContainer
+            user_container = DefaultUserContainer
+
+        if self.__is_user_unique(user['uid'], opts) == 0:
+            raise ipaerror.gen_exception(ipaerror.LDAP_DUPLICATE)
 
         dn="uid=%s,%s,%s" % (user['uid'], user_container,self.basedn)
         entry = ipaserver.ipaldap.Entry(dn)
@@ -282,7 +342,6 @@ class IPAServer:
 
         dn = self.get_dn_from_principal(self.princ)
     
-        # FIXME: Is this the filter we want or should it be more specific?
         filter = "(objectclass=posixAccount)"
 
         m1 = _LDAPPool.getConn(self.host,self.port,self.bindca,self.bindcert,self.bindkey,dn)
@@ -295,34 +354,23 @@ class IPAServer:
     
         return users
 
-    def find_users (self, criteria, sattrs=None, user_container=None, opts=None):
+    def find_users (self, criteria, sattrs=None, opts=None):
         """Return a list containing a User object for each
         existing user that matches the criteria.
         """
         global _LDAPPool
 
-        if user_container is None:
-            user_container = DefaultContainer
-
         if opts:
             self.set_principal(opts['remoteuser'])
 
         dn = self.get_dn_from_principal(self.princ)
 
-        # TODO: this escaper assumes the python-ldap library will error out
-        #       on invalid codepoints.  we need to check malformed utf-8 input
-        #       where the second byte in a multi-byte character
-        #       is (illegally) ')' and make sure python-ldap
-        #       bombs out.
-        criteria = re.sub(r'[\(\)\\]', ldap_search_escape, criteria)
+        criteria = self.__safe_filter(criteria)
 
-        # 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(basedn, self.scope, filter, sattrs)
+            results = m1.getList(self.basedn, self.scope, filter, sattrs)
             _LDAPPool.releaseConn(m1)
         except ipaerror.exception_for(ipaerror.LDAP_NOT_FOUND):
             results = []
@@ -346,28 +394,7 @@ class IPAServer:
 
     def update_user (self, olduser, newuser, opts=None):
         """Update a user in LDAP"""
-        global _LDAPPool
-
-        olduser = self.convert_scalar_values(olduser)
-        newuser = self.convert_scalar_values(newuser)
-
-        # Should be able to get this from either the old or new user
-        # but just in case someone has decided to try changing it, use the
-        # original
-        try:
-            moddn = olduser['dn']
-        except KeyError, e:
-            raise ipaerror.gen_exception(ipaerror.LDAP_MISSING_DN)
-
-        if opts:
-            self.set_principal(opts['remoteuser'])
-    
-        proxydn = self.get_dn_from_principal(self.princ)
-
-        m1 = _LDAPPool.getConn(self.host,self.port,self.bindca,self.bindcert,self.bindkey,proxydn)
-        res = m1.updateEntry(moddn, olduser, newuser)
-        _LDAPPool.releaseConn(m1)
-        return res
+        return self.__update_entry(olduser, newuser, opts)
 
     def mark_user_deleted (self, uid, opts=None):
         """Mark a user as inactive in LDAP. We aren't actually deleting
@@ -394,6 +421,217 @@ class IPAServer:
         _LDAPPool.releaseConn(m1)
         return res
 
+# Group support
+
+    def __is_group_unique(self, cn, opts):
+        """Return 1 if the cn is unique in the tree, 0 otherwise."""
+        cn = self.__safe_filter(cn)
+        filter = "(&(cn=%s)(objectclass=posixGroup))" % cn
+ 
+        entry = self.__get_entry(self.basedn, filter, ['dn','cn'], opts)
+
+        if entry is not None:
+            return 0
+        else:
+            return 1
+
+    def get_group_by_cn (self, cn, sattrs=None, opts=None):
+        """Get a specific group's entry. Return as a dict of values.
+           Multi-valued fields are represented as lists.
+        """
+
+        cn = self.__safe_filter(cn)
+        filter = "(cn=" + cn + ")"
+        return self.__get_entry(self.basedn, filter, sattrs, opts)
+    
+    def get_group_by_dn (self, dn, sattrs=None, opts=None):
+        """Get a specific group's entry. Return as a dict of values.
+           Multi-valued fields are represented as lists.
+        """
+
+        filter = "(objectClass=*)"
+        return self.__get_entry(dn, filter, sattrs, opts)
+    
+    def add_group (self, group, group_container=None, opts=None):
+        """Add a group 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. group_container sets
+           where in the tree the group is placed."""
+        global _LDAPPool
+
+        if group_container is None:
+            group_container = DefaultGroupContainer
+
+        if self.__is_group_unique(group['cn'], opts) == 0:
+            raise ipaerror.gen_exception(ipaerror.LDAP_DUPLICATE)
+
+        dn="cn=%s,%s,%s" % (group['cn'], group_container,self.basedn)
+        entry = ipaserver.ipaldap.Entry(dn)
+
+        # some required objectclasses
+        entry.setValues('objectClass', 'top', 'groupofuniquenames', 'posixGroup')
+
+        # FIXME, need a gidNumber generator
+        if group.get('gidnumber') is None:
+            entry.setValues('gidNumber', '501')
+
+        # fill in our new entry with everything sent by the user
+        for g in group:
+            entry.setValues(g, group[g])
+
+        if opts:
+            self.set_principal(opts['remoteuser'])
+    
+        dn = self.get_dn_from_principal(self.princ)
+
+        m1 = _LDAPPool.getConn(self.host,self.port,self.bindca,self.bindcert,self.bindkey,dn)
+        res = m1.addEntry(entry)
+        _LDAPPool.releaseConn(m1)
+
+    def find_groups (self, criteria, sattrs=None, opts=None):
+        """Return a list containing a User object for each
+        existing group that matches the criteria.
+        """
+        global _LDAPPool
+
+        if opts:
+            self.set_principal(opts['remoteuser'])
+
+        dn = self.get_dn_from_principal(self.princ)
+
+        criteria = self.__safe_filter(criteria)
+
+        filter = "(&(cn=%s)(objectClass=posixGroup))" % criteria
+        try:
+            m1 = _LDAPPool.getConn(self.host,self.port,self.bindca,self.bindcert,self.bindkey,dn)
+            results = m1.getList(self.basedn, self.scope, filter, sattrs)
+            _LDAPPool.releaseConn(m1)
+        except ipaerror.exception_for(ipaerror.LDAP_NOT_FOUND):
+            results = []
+
+        groups = []
+        for u in results:
+            groups.append(self.convert_entry(u))
+    
+        return groups
+
+    def add_user_to_group(self, user, group, opts=None):
+        """Add a user to an existing group.
+           user is a uid of the user to add
+           group is the cn of the group to be added to
+        """
+
+        if opts:
+            self.set_principal(opts['remoteuser'])
+
+        old_group = self.get_group_by_cn(group, None, opts)
+        if old_group is None:
+            raise ipaerror.gen_exception(ipaerror.LDAP_NOT_FOUND)
+        new_group = copy.deepcopy(old_group)
+
+        user_dn = self.get_user_by_uid(user, ['dn', 'uid', 'objectclass'], opts)
+        if user_dn is None:
+            raise ipaerror.gen_exception(ipaerror.LDAP_NOT_FOUND)
+
+        if new_group.get('uniquemember') is not None:
+            if ((isinstance(new_group.get('uniquemember'), str)) or (isinstance(new_group.get('uniquemember'), unicode))):
+                new_group['uniquemember'] = [new_group['uniquemember']]
+            new_group['uniquemember'].append(user_dn['dn'])
+        else:
+            new_group['uniquemember'] = user_dn['dn']
+
+        try:
+            ret = self.__update_entry(old_group, new_group, opts)
+        except ipaerror.exception_for(ipaerror.LDAP_EMPTY_MODLIST):
+            raise
+        return ret
+
+    def add_users_to_group(self, users, group, opts=None):
+        """Given a list of user uid's add them to the group cn denoted by group
+           Returns a list of the users were not added to the group.
+        """
+
+        failed = []
+
+        if (isinstance(users, str)):
+            users = [users]
+
+        for user in users:
+            try:
+                self.add_user_to_group(user, group, opts)
+            except ipaerror.exception_for(ipaerror.LDAP_EMPTY_MODLIST):
+                # User is already in the group
+                failed.append(user)
+            except ipaerror.gen_exception(ipaerror.LDAP_NOT_FOUND):
+                # User or the group does not exist
+                failed.append(user)
+
+        return failed
+
+    def remove_user_from_group(self, user, group, opts=None):
+        """Remove a user from an existing group.
+           user is a uid of the user to remove
+           group is the cn of the group to be removed from
+        """
+
+        if opts:
+            self.set_principal(opts['remoteuser'])
+
+        old_group = self.get_group_by_cn(group, None, opts)
+        if old_group is None:
+            raise ipaerror.gen_exception(ipaerror.LDAP_NOT_FOUND)
+        new_group = copy.deepcopy(old_group)
+
+        user_dn = self.get_user_by_uid(user, ['dn', 'uid', 'objectclass'], opts)
+        if user_dn is None:
+            raise ipaerror.gen_exception(ipaerror.LDAP_NOT_FOUND)
+
+        if new_group.get('uniquemember') is not None:
+            if ((isinstance(new_group.get('uniquemember'), str)) or (isinstance(new_group.get('uniquemember'), unicode))):
+                new_group['uniquemember'] = [new_group['uniquemember']]
+            try:
+                new_group['uniquemember'].remove(user_dn['dn'])
+            except ValueError:
+                # User is not in the group
+                # FIXME: raise more specific error?
+                raise ipaerror.gen_exception(ipaerror.LDAP_NOT_FOUND)
+        else:
+            # Nothing to do if the group has no members
+            # FIXME raise SOMETHING?
+            return "Success"
+
+        try:
+            ret = self.__update_entry(old_group, new_group, opts)
+        except ipaerror.exception_for(ipaerror.LDAP_EMPTY_MODLIST):
+            raise
+        return ret
+
+    def remove_users_from_group(self, users, group, opts=None):
+        """Given a list of user uid's remove them from the group cn denoted
+           by group
+           Returns a list of the users were not removed from the group.
+        """
+
+        failed = []
+
+        if (isinstance(users, str)):
+            users = [users]
+
+        for user in users:
+            try:
+                self.remove_user_from_group(user, group, opts)
+            except ipaerror.exception_for(ipaerror.LDAP_EMPTY_MODLIST):
+                # User is not in the group
+                failed.append(user)
+            except ipaerror.gen_exception(ipaerror.LDAP_NOT_FOUND):
+                # User or the group does not exist
+                failed.append(user)
+
+        return failed
+
+    def update_group (self, oldgroup, newgroup, opts=None):
+        """Update a group in LDAP"""
+        return self.__update_entry(oldgroup, newgroup, opts)
 
 def ldap_search_escape(match):
     """Escapes out nasty characters from the ldap search.
diff --git a/ipa-server/xmlrpc-server/ipaxmlrpc.py b/ipa-server/xmlrpc-server/ipaxmlrpc.py
index 7998dd7e0..3f30f8fbd 100644
--- a/ipa-server/xmlrpc-server/ipaxmlrpc.py
+++ b/ipa-server/xmlrpc-server/ipaxmlrpc.py
@@ -295,6 +295,15 @@ def handler(req, profiling=False):
             h.register_function(f.find_users)
             h.register_function(f.update_user)
             h.register_function(f.mark_user_deleted)
+            h.register_function(f.get_group_by_cn)
+            h.register_function(f.get_group_by_dn)
+            h.register_function(f.add_group)
+            h.register_function(f.find_groups)
+            h.register_function(f.add_user_to_group)
+            h.register_function(f.add_users_to_group)
+            h.register_function(f.remove_user_from_group)
+            h.register_function(f.remove_users_from_group)
+            h.register_function(f.update_group)
             h.handle_request(req)
         finally:
              pass