Merge branch 'master' of git://git.engineering.redhat.com/users/rcritten/freeipa2

10 files changed, 325 insertions, 212 deletions

diff --git a/ipalib/cli.py b/ipalib/cli.py
index 0bab952bf..518b71298 100644
--- a/ipalib/cli.py
+++ b/ipalib/cli.py
@@ -697,14 +697,25 @@ class CLI(object):
         if self.options.interactive:
             self.prompt_interactively(cmd, kw)
         self.prompt_for_passwords(cmd, kw)
+        self.set_defaults(cmd, kw)
         result = cmd(**kw)
         if callable(cmd.output_for_cli):
             for param in cmd.params():
                 if param.ispassword():
-                    del kw[param.name]
+                    try:
+                        del kw[param.name]
+                    except KeyError:
+                        pass
             (args, options) = cmd.params_2_args_options(kw)
             cmd.output_for_cli(self.api.Backend.textui, result, *args, **options)
 
+    def set_defaults(self, cmd, kw):
+        for param in cmd.params():
+            if not kw.get(param.name):
+                value = param.get_default(**kw)
+                if value:
+                    kw[param.name] = value
+
     def prompt_for_passwords(self, cmd, kw):
         for param in cmd.params():
             if 'password' not in param.flags:
diff --git a/ipalib/errors.py b/ipalib/errors.py
index bc4074d2d..6dd6eb01f 100644
--- a/ipalib/errors.py
+++ b/ipalib/errors.py
@@ -423,6 +423,14 @@ class HostService(ConfigurationError):
     """You must enroll a host in order to create a host service"""
     faultCode = 1026
 
+class InsufficientAccess(GenericError):
+    """You do not have permission to perform this task"""
+    faultCode = 1027
+
+class InvalidUserPrincipal(GenericError):
+    """Invalid user principal"""
+    faultCode = 1028
+
 class FunctionDeprecated(GenericError):
     """Raised by a deprecated function"""
     faultCode = 2000
diff --git a/ipalib/plugins/f_automount.py b/ipalib/plugins/f_automount.py
index 7a251572e..4c392438d 100644
--- a/ipalib/plugins/f_automount.py
+++ b/ipalib/plugins/f_automount.py
@@ -34,14 +34,14 @@ from ldap import explode_dn
 map_attributes = ['automountMapName', 'description', ]
 key_attributes = ['description', 'automountKey', 'automountInformation']
 
-def display_entry(entry):
+def display_entry(textui, entry):
     # FIXME: for now delete dn here. In the future pass in the kw to
     # output_for_cli()
     attr = sorted(entry.keys())
 
     for a in attr:
         if a != 'dn':
-            print "%s: %s" % (a, entry[a])
+            textui.print_plain("%s: %s" % (a, entry[a]))
 
 def make_automount_dn(mapname):
     """
@@ -96,12 +96,11 @@ class automount_addmap(crud.Add):
         kw['objectClass'] = ['automountMap']
 
         return ldap.create(**kw)
-    def output_for_cli(self, ret):
+    def output_for_cli(self, textui, result, map, **options):
         """
         Output result of this command to command line interface.
         """
-        if ret:
-            print "Automount map added"
+        textui.print_plain("Automount map %s added" % map)
 
 api.register(automount_addmap)
 
@@ -139,12 +138,11 @@ class automount_addkey(crud.Add):
         kw['objectClass'] = ['automount']
 
         return ldap.create(**kw)
-    def output_for_cli(self, ret):
+    def output_for_cli(self, textui, result, *args, **options):
         """
         Output result of this command to command line interface.
         """
-        if ret:
-            print "Automount key added"
+        textui.print_plain("Automount key added")
 
 api.register(automount_addkey)
 
@@ -161,18 +159,17 @@ class automount_delmap(crud.Del):
            :param kw: Not used.
         """
         ldap = self.api.Backend.ldap
-        dn = ldap.find_entry_dn("automountmapname", mapname, "automountmap")
+        dn = ldap.find_entry_dn("automountmapname", mapname, "automountmap", api.env.container_automount)
         keys = api.Command['automount_getkeys'](mapname)
         if keys:
             for k in keys:
                 ldap.delete(k.get('dn'))
         return ldap.delete(dn)
-    def output_for_cli(self, ret):
+    def output_for_cli(self, textui, result, *args, **options):
         """
         Output result of this command to command line interface.
         """
-        if ret:
-            print "Automount map and associated keys deleted"
+        print "Automount map and associated keys deleted"
 
 api.register(automount_delmap)
 
@@ -205,12 +202,11 @@ class automount_delkey(crud.Del):
         if not keydn:
             raise errors.NotFound
         return ldap.delete(keydn)
-    def output_for_cli(self, ret):
+    def output_for_cli(self, textui, result, *args, **options):
         """
         Output result of this command to command line interface.
         """
-        if ret:
-            print "Automount key deleted"
+        print "Automount key deleted"
 
 api.register(automount_delkey)
 
@@ -238,12 +234,11 @@ class automount_modmap(crud.Mod):
         dn = ldap.find_entry_dn("automountmapname", mapname, "automountmap", api.env.container_automount)
         return ldap.update(dn, **kw)
 
-    def output_for_cli(self, ret):
+    def output_for_cli(self, textui, result, *args, **options):
         """
         Output result of this command to command line interface.
         """
-        if ret:
-            print "Automount map updated"
+        print "Automount map updated"
 
 api.register(automount_modmap)
 
@@ -286,12 +281,12 @@ class automount_modkey(crud.Mod):
             raise errors.NotFound
         return ldap.update(keydn, **kw)
 
-    def output_for_cli(self, ret):
+    def output_for_cli(self, textui, result, *args, **options):
         """
         Output result of this command to command line interface.
         """
-        if ret:
-            print "Automount key updated"
+        print "Automount key updated"
+
 api.register(automount_modkey)
 
 
@@ -309,26 +304,27 @@ class automount_findmap(crud.Find):
             kw[s] = term
 
         kw['objectclass'] = 'automountMap'
+        kw['base'] = api.env.container_automount
         if kw.get('all', False):
             kw['attributes'] = ['*']
         else:
             kw['attributes'] = map_attributes
         return ldap.search(**kw)
-    def output_for_cli(self, entries):
-        if not entries:
-            return
-        counter = entries[0]
-        entries = entries[1:]
+
+    def output_for_cli(self, textui, result, *args, **options):
+        counter = result[0]
+        entries = result[1:]
         if counter == 0:
-            print "No entries found"
+            textui.print_plain("No entries found")
             return
         elif counter == -1:
-            print "These results are truncated."
-            print "Please refine your search and try again."
+            textui.print_plain("These results are truncated.")
+            textui.print_plain("Please refine your search and try again.")
 
         for e in entries:
-            display_entry(e)
-            print ""
+            display_entry(textui, e)
+            textui.print_plain("")
+
 api.register(automount_findmap)
 
 
@@ -350,26 +346,26 @@ class automount_findkey(crud.Find):
             kw[s] = term
 
         kw['objectclass'] = 'automount'
+        kw['base'] = api.env.container_automount
         if kw.get('all', False):
             kw['attributes'] = ['*']
         else:
             kw['attributes'] = key_attributes
         return ldap.search(**kw)
-    def output_for_cli(self, entries):
-        if not entries:
-            return
-        counter = entries[0]
-        entries = entries[1:]
+    def output_for_cli(self, textui, result, *args, **options):
+        counter = result[0]
+        entries = result[1:]
         if counter == 0:
-            print "No entries found"
+            textui.print_plain("No entries found")
             return
         elif counter == -1:
-            print "These results are truncated."
-            print "Please refine your search and try again."
+            textui.print_plain("These results are truncated.")
+            textui.print_plain("Please refine your search and try again.")
 
         for e in entries:
-            display_entry(e)
-            print ""
+            display_entry(textui, e)
+            textui.print_plain("")
+
 api.register(automount_findkey)
 
 
@@ -394,9 +390,9 @@ class automount_showmap(crud.Get):
             return ldap.retrieve(dn)
         else:
             return ldap.retrieve(dn, map_attributes)
-    def output_for_cli(self, entry):
-        if entry:
-            display_entry(entry)
+    def output_for_cli(self, textui, result, *args, **options):
+        if result:
+            display_entry(textui, result)
 
 api.register(automount_showmap)
 
@@ -436,16 +432,16 @@ class automount_showkey(crud.Get):
             return ldap.retrieve(keydn)
         else:
             return ldap.retrieve(keydn, key_attributes)
-    def output_for_cli(self, entry):
+    def output_for_cli(self, textui, result, *args, **options):
         # The automount map name associated with this key is available only
         # in the dn. Add it as an attribute to display instead.
-        if entry and not entry.get('automountmapname'):
-            elements = explode_dn(entry.get('dn').lower())
+        if result and not result.get('automountmapname'):
+            elements = explode_dn(result.get('dn').lower())
             for e in elements:
                 (attr, value) = e.split('=',1)
                 if attr == 'automountmapname':
-                    entry['automountmapname'] = value
-            display_entry(entry)
+                    result['automountmapname'] = value
+            display_entry(textui, result)
 
 api.register(automount_showkey)
 
@@ -475,9 +471,89 @@ class automount_getkeys(frontend.Command):
             keys = []
 
         return keys
-    def output_for_cli(self, keys):
-        if keys:
-            for k in keys:
-                print k.get('automountkey')
+    def output_for_cli(self, textui, result, *args, **options):
+        for k in result:
+            textui.print_plain('%s' % k.get('automountkey'))
 
 api.register(automount_getkeys)
+
+
+class automount_getmaps(frontend.Command):
+    'Retrieve all automount maps'
+    takes_args = (
+        Param('automountmapname?',
+            cli_name='mapname',
+            primary_key=True,
+            doc='A group of related automount objects',
+        ),
+    )
+    def execute(self, mapname, **kw):
+        """
+        Execute the automount-getmaps operation.
+
+        Return a list of all automount maps.
+        """
+
+        ldap = self.api.Backend.ldap
+        base = api.env.container_automount + "," + api.env.basedn
+
+        if not mapname:
+            mapname = "auto.master"
+        search_base = "automountmapname=%s,%s" % (mapname, base)
+        maps = ldap.get_one_entry(search_base, "objectClass=*", ["*"])
+
+        return maps
+    def output_for_cli(self, textui, result, *args, **options):
+        for k in result:
+            textui.print_plain('%s: %s' % (k.get('automountinformation'), k.get('automountkey')))
+
+api.register(automount_getmaps)
+
+class automount_addindirectmap(crud.Add):
+    'Add a new automap indirect mount point.'
+    takes_options = (
+        Param('parentmap?',
+            cli_name='parentmap',
+            default='auto.master',
+            doc='The parent map to connect this to. Default: auto.master'),
+        Param('automountkey',
+            cli_name='key',
+            doc='An entry in an automount map'),
+        Param('description?',
+            doc='A description of the automount map'),
+    )
+
+    def execute(self, mapname, **kw):
+        """
+        Execute the automount-addindirectmap operation.
+
+        Returns the key entry as it will be created in LDAP.
+
+        This function creates 2 LDAP entries. It creates an
+        automountmapname entry and an automountkey entry.
+
+        :param mapname: The map name being added.
+        :param kw['parentmap'] is the top-level map to add this to.
+           defaulting to auto.master
+        :param kw['automountkey'] is the mount point
+        :param kw['description'] is a textual description of this map
+        """
+        mapkw = {}
+        if kw.get('description'):
+            mapkw['description'] = kw.get('description')
+        newmap = api.Command['automount_addmap'](mapname, **mapkw)
+
+        keykw = {'automountkey': kw['automountkey'], 'automountinformation': mapname}
+        if kw.get('description'):
+            keykw['description'] = kw.get('description')
+        newkey = api.Command['automount_addkey'](kw['parentmap'], **keykw)
+
+        return newkey
+    def output_for_cli(self, textui, result, map, **options):
+        """
+        Output result of this command to command line interface.
+        """
+        textui.print_plain("Indirect automount map %s added" % map)
+
+api.register(automount_addindirectmap)
+
diff --git a/ipalib/plugins/f_group.py b/ipalib/plugins/f_group.py
index 9df83a299..803e5d000 100644
--- a/ipalib/plugins/f_group.py
+++ b/ipalib/plugins/f_group.py
@@ -29,6 +29,19 @@ from ipalib import errors
 from ipalib import ipa_types
 
 
+def get_members(members):
+    """
+    Return a list of members.
+
+    It is possible that the value passed in is None.
+    """
+    if members:
+        members = members.split(',')
+    else:
+        members = []
+
+    return members
+
 class group(frontend.Object):
     """
     Group object.
@@ -83,12 +96,13 @@ class group_add(crud.Add):
 
         return ldap.create(**kw)
 
-    def output_for_cli(self, ret):
+    def output_for_cli(self, textui, result, *args, **options):
         """
         Output result of this command to command line interface.
         """
-        if ret:
-            print "Group added"
+        textui.print_name(self.name)
+        textui.print_entry(result)
+        textui.print_dashed('Added group "%s"' % result['cn'])
 
 api.register(group_add)
 
@@ -121,12 +135,11 @@ class group_del(crud.Del):
 
         return ldap.delete(dn)
 
-    def output_for_cli(self, ret):
+    def output_for_cli(self, textui, result, cn):
         """
         Output result of this command to command line interface.
         """
-        if ret:
-            print "Group deleted"
+        textui.print_plain("Deleted group %s" % cn)
 
 api.register(group_del)
 
@@ -151,12 +164,12 @@ class group_mod(crud.Mod):
         dn = ldap.find_entry_dn("cn", cn, "posixGroup")
         return ldap.update(dn, **kw)
 
-    def output_for_cli(self, ret):
+    def output_for_cli(self, textui, result, cn, **options):
         """
         Output result of this command to command line interface.
         """
-        if ret:
-            print "Group updated"
+        if result:
+            textui.print_plain("Group updated")
 
 api.register(group_mod)
 
@@ -171,30 +184,33 @@ class group_find(crud.Find):
         search_fields_conf_str = config.get('ipagroupsearchfields')
         search_fields = search_fields_conf_str.split(",")
 
+        search_kw = {}
         for s in search_fields:
-            kw[s] = term
+            search_kw[s] = term
 
         object_type = ldap.get_object_type("cn")
         if object_type and not kw.get('objectclass'):
-            kw['objectclass'] = object_type
-        return ldap.search(**kw)
-
-    def output_for_cli(self, groups):
-        if not groups:
+            search_kw['objectclass'] = object_type
+        return ldap.search(**search_kw)
+
+    def output_for_cli(self, textui, result, uid, **options):
+        counter = result[0]
+        groups = result[1:]
+        if counter == 0 or len(groups) == 0:
+            textui.print_plain("No entries found")
             return
-
-        counter = groups[0]
-        groups = groups[1:]
-        if counter == 0:
-            print "No entries found"
+        if len(groups) == 1:
+            textui.print_entry(groups[0])
             return
-        elif counter == -1:
-            print "These results are truncated."
-            print "Please refine your search and try again."
+        textui.print_name(self.name)
 
         for g in groups:
-            for a in g.keys():
-                print "%s: %s" % (a, g[a])
+            textui.print_entry(g)
+            textui.print_plain('')
+        if counter == -1:
+            textui.print_plain("These results are truncated.")
+            textui.print_plain("Please refine your search and try again.")
+        textui.print_count(groups, '%d groups matched')
 
 api.register(group_find)
 
@@ -218,12 +234,24 @@ class group_show(crud.Get):
         # FIXME: should kw contain the list of attributes to display?
         return ldap.retrieve(dn)
 
-    def output_for_cli(self, group):
-        if not group:
+    def output_for_cli(self, textui, result, *args, **options):
+        counter = result[0]
+        groups = result[1:]
+        if counter == 0 or len(groups) == 0:
+            textui.print_plain("No entries found")
             return
-
-        for a in group.keys():
-            print "%s: %s" % (a, group[a])
+        if len(groups) == 1:
+            textui.print_entry(groups[0])
+            return
+        textui.print_name(self.name)
+        for u in groups:
+            textui.print_plain('%(givenname)s %(sn)s:' % u)
+            textui.print_entry(u)
+            textui.print_plain('')
+        if counter == -1:
+            textui.print_plain('These results are truncated.')
+            textui.print_plain('Please refine your search and try again.')
+        textui.print_count(groups, '%d groups matched')
 
 api.register(group_show)
 
@@ -253,7 +281,7 @@ class group_add_member(frontend.Command):
         to_add = []
         completed = 0
 
-        members = kw.get('groups', '').split(',')
+        members = get_members(kw.get('groups', ''))
         for m in members:
             if not m: continue
             try:
@@ -263,7 +291,7 @@ class group_add_member(frontend.Command):
                 add_failed.append(m)
                 continue
 
-        members = kw.get('users', '').split(',')
+        members = get_members(kw.get('users', ''))
         for m in members:
             if not m: continue
             try:
@@ -282,11 +310,11 @@ class group_add_member(frontend.Command):
 
         return add_failed
 
-    def output_for_cli(self, add_failed):
+    def output_for_cli(self, textui, result, *args, **options):
         """
         Output result of this command to command line interface.
         """
-        if add_failed:
+        if result:
             print "These entries failed to add to the group:"
             for a in add_failed:
                 print "\t'%s'" % a
@@ -320,7 +348,7 @@ class group_remove_member(frontend.Command):
         remove_failed = []
         completed = 0
 
-        members = kw.get('groups', '').split(',')
+        members = get_members(kw.get('groups', ''))
         for m in members:
             if not m: continue
             try:
@@ -330,7 +358,7 @@ class group_remove_member(frontend.Command):
                 remove_failed.append(m)
                 continue
 
-        members = kw.get('users', '').split(',')
+        members = get_members(kw.get('users', ''))
         for m in members:
             try:
                 member_dn = ldap.find_entry_dn("uid", m,)
@@ -348,11 +376,11 @@ class group_remove_member(frontend.Command):
 
         return remove_failed
 
-    def output_for_cli(self, remove_failed):
+    def output_for_cli(self, textui, result, *args, **options):
         """
         Output result of this command to command line interface.
         """
-        if remove_failed:
+        if result:
             print "These entries failed to be removed from the group:"
             for a in remove_failed:
                 print "\t'%s'" % a
diff --git a/ipalib/plugins/f_host.py b/ipalib/plugins/f_host.py
index e842230fe..7903ff90f 100644
--- a/ipalib/plugins/f_host.py
+++ b/ipalib/plugins/f_host.py
@@ -149,12 +149,11 @@ class host_add(crud.Add):
                 kw['objectclass'].remove('krbprincipalaux')
 
         return ldap.create(**kw)
-    def output_for_cli(self, ret):
+    def output_for_cli(self, textui, result, *args, **options):
         """
         Output result of this command to command line interface.
         """
-        if ret:
-            print "Host added"
+        textui.print_plain("Host added")
 
 api.register(host_add)
 
@@ -172,12 +171,11 @@ class host_del(crud.Del):
         ldap = self.api.Backend.ldap
         dn = get_host(hostname)
         return ldap.delete(dn)
-    def output_for_cli(self, ret):
+    def output_for_cli(self, textui, result, *args, **options):
         """
         Output result of this command to command line interface.
         """
-        if ret:
-            print "Host deleted"
+        textui.print_plain("Host deleted")
 
 api.register(host_del)
 
@@ -202,12 +200,11 @@ class host_mod(crud.Mod):
         dn = get_host(hostname)
         return ldap.update(dn, **kw)
 
-    def output_for_cli(self, ret):
+    def output_for_cli(self, textui, result, *args, **options):
         """
         Output result of this command to command line interface.
         """
-        if ret:
-            print "Host updated"
+        textui.print_plain("Host updated")
 
 api.register(host_mod)
 
@@ -232,31 +229,29 @@ class host_find(crud.Find):
         #search_fields = search_fields_conf_str.split(",")
         search_fields = ['cn','serverhostname','description','localityname','nshostlocation','nshardwareplatform','nsosversion']
 
+        search_kw = {}
         for s in search_fields:
-            kw[s] = term
+            search_kw[s] = term
 
         # Can't use ldap.get_object_type() since cn is also used for group dns
-        kw['objectclass'] = "ipaHost"
+        search_kw['objectclass'] = "ipaHost"
         if kw.get('all', False):
-            kw['attributes'] = ['*']
+            search_kw['attributes'] = ['*']
         else:
-            kw['attributes'] = default_attributes
-        return ldap.search(**kw)
-    def output_for_cli(self, hosts):
-        if not hosts:
-            return
-        counter = hosts[0]
-        hosts = hosts[1:]
+            search_kw['attributes'] = default_attributes
+        return ldap.search(**search_kw)
+    def output_for_cli(self, textui, result, *args, **options):
+        counter = result[0]
+        hosts = result[1:]
         if counter == 0:
-            print "No entries found"
+            textui.print_plain("No entries found")
             return
-        elif counter == -1:
-            print "These results are truncated."
-            print "Please refine your search and try again."
 
         for h in hosts:
-            for a in h.keys():
-                print "%s: %s" % (a, h[a])
+            textui.print_entry(h)
+        if counter == -1:
+            textui.print_plain("These results are truncated.")
+            textui.print_plain("Please refine your search and try again.")
 api.register(host_find)
 
 
@@ -286,9 +281,7 @@ class host_show(crud.Get):
             value = ldap.retrieve(dn, default_attributes)
             del value['dn']
             return value
-    def output_for_cli(self, host):
-        if host:
-            for a in host.keys():
-                print "%s: %s" % (a, host[a])
+    def output_for_cli(self, textui, result, *args, **options):
+        textui.print_entry(result)
 
 api.register(host_show)
diff --git a/ipalib/plugins/f_hostgroup.py b/ipalib/plugins/f_hostgroup.py
index 8e4c37407..3e14b09a2 100644
--- a/ipalib/plugins/f_hostgroup.py
+++ b/ipalib/plugins/f_hostgroup.py
@@ -30,6 +30,19 @@ from ipalib import ipa_types
 
 hostgroup_filter = "groupofnames)(!(objectclass=posixGroup)"
 
+def get_members(members):
+    """
+    Return a list of members.
+
+    It is possible that the value passed in is None.
+    """
+    if members:
+        members = members.split(',')
+    else:
+        members = []
+
+    return members
+
 class hostgroup(frontend.Object):
     """
     Host Group object.
@@ -80,12 +93,11 @@ class hostgroup_add(crud.Add):
 
         return ldap.create(**kw)
 
-    def output_for_cli(self, ret):
+    def output_for_cli(self, textui, result, *args, **options):
         """
         Output result of this command to command line interface.
         """
-        if ret:
-            print "Group added"
+        textui.print_plain("Group added")
 
 api.register(hostgroup_add)
 
@@ -107,12 +119,11 @@ class hostgroup_del(crud.Del):
 
         return ldap.delete(dn)
 
-    def output_for_cli(self, ret):
+    def output_for_cli(self, textui, result, *args, **options):
         """
         Output result of this command to command line interface.
         """
-        if ret:
-            print "Group deleted"
+        textui.print_plain("Group deleted")
 
 api.register(hostgroup_del)
 
@@ -137,12 +148,11 @@ class hostgroup_mod(crud.Mod):
         dn = ldap.find_entry_dn("cn", cn, hostgroup_filter)
         return ldap.update(dn, **kw)
 
-    def output_for_cli(self, ret):
+    def output_for_cli(self, textui, result, *args, **options):
         """
         Output result of this command to command line interface.
         """
-        if ret:
-            print "Group updated"
+        texui.print_plain("Group updated")
 
 api.register(hostgroup_mod)
 
@@ -159,28 +169,26 @@ class hostgroup_find(crud.Find):
         search_fields_conf_str = config.get('ipagroupsearchfields')
         search_fields = search_fields_conf_str.split(",")
 
+        search_kw = {}
         for s in search_fields:
-            kw[s] = term
+            search_kw[s] = term
 
-        kw['objectclass'] = hostgroup_filter
-        return ldap.search(**kw)
+        search_kw['objectclass'] = hostgroup_filter
+        return ldap.search(**search_kw)
 
-    def output_for_cli(self, groups):
-        if not groups:
-            return
-
-        counter = groups[0]
-        groups = groups[1:]
+    def output_for_cli(self, textui, result, *args, **options):
+        counter = result[0]
+        groups = result[1:]
         if counter == 0:
-            print "No entries found"
+            textui.print_plain("No entries found")
             return
-        elif counter == -1:
-            print "These results are truncated."
-            print "Please refine your search and try again."
 
         for g in groups:
-            for a in g.keys():
-                print "%s: %s" % (a, g[a])
+            textui.print_entry(g)
+
+        if counter == -1:
+            textui.print_plain("These results are truncated.")
+            textui.print_plain("Please refine your search and try again.")
 
 api.register(hostgroup_find)
 
@@ -206,12 +214,8 @@ class hostgroup_show(crud.Get):
         # FIXME: should kw contain the list of attributes to display?
         return ldap.retrieve(dn)
 
-    def output_for_cli(self, group):
-        if not group:
-            return
-
-        for a in group.keys():
-            print "%s: %s" % (a, group[a])
+    def output_for_cli(self, textui, result, *args, **options):
+        textui.print_entry(result)
 
 api.register(hostgroup_show)
 
@@ -241,7 +245,7 @@ class hostgroup_add_member(frontend.Command):
         to_add = []
         completed = 0
 
-        members = kw.get('groups', '').split(',')
+        members = get_members(kw.get('groups', ''))
         for m in members:
             if not m: continue
             try:
@@ -251,7 +255,7 @@ class hostgroup_add_member(frontend.Command):
                 add_failed.append(m)
                 continue
 
-        members = kw.get('hosts', '').split(',')
+        members = get_members(kw.get('hosts', ''))
         for m in members:
             if not m: continue
             try:
@@ -270,16 +274,16 @@ class hostgroup_add_member(frontend.Command):
 
         return add_failed
 
-    def output_for_cli(self, add_failed):
+    def output_for_cli(self, textui, result, *args, **options):
         """
         Output result of this command to command line interface.
         """
-        if add_failed:
-            print "These entries failed to add to the group:"
-            for a in add_failed:
+        if result:
+            textui.print_plain("These entries failed to add to the group:")
+            for a in result:
                 print "\t'%s'" % a
         else:
-            print "Group membership updated."
+            textui.print_entry("Group membership updated.")
 
 api.register(hostgroup_add_member)
 
@@ -309,7 +313,7 @@ class hostgroup_remove_member(frontend.Command):
         remove_failed = []
         completed = 0
 
-        members = kw.get('groups', '').split(',')
+        members = get_members(kw.get('groups', ''))
         for m in members:
             if not m: continue
             try:
@@ -319,7 +323,7 @@ class hostgroup_remove_member(frontend.Command):
                 remove_failed.append(m)
                 continue
 
-        members = kw.get('hosts', '').split(',')
+        members = get_members(kw.get('hosts', ''))
         for m in members:
             if not m: continue
             try:
@@ -338,15 +342,15 @@ class hostgroup_remove_member(frontend.Command):
 
         return remove_failed
 
-    def output_for_cli(self, remove_failed):
+    def output_for_cli(self, textui, result, *args, **options):
         """
         Output result of this command to command line interface.
         """
-        if remove_failed:
-            print "These entries failed to be removed from the group:"
-            for a in remove_failed:
+        if result:
+            textui.print_plain("These entries failed to be removed from the group:")
+            for a in result:
                 print "\t'%s'" % a
         else:
-            print "Group membership updated."
+            textui.print_plain("Group membership updated.")
 
 api.register(hostgroup_remove_member)
diff --git a/ipalib/plugins/f_passwd.py b/ipalib/plugins/f_passwd.py
index edc13b633..1e0dfc1cb 100644
--- a/ipalib/plugins/f_passwd.py
+++ b/ipalib/plugins/f_passwd.py
@@ -52,14 +52,12 @@ class passwd(frontend.Command):
         :param param uid: The login name of the user being updated.
         :param kw: Not used.
         """
-        if principal.find('@') < 0:
+        if principal.find('@') > 0:
             u = principal.split('@')
-            if len(u) > 2 or len(u) == 0:
-                print "Invalid user name (%s)" % principal
-            if len(u) == 1:
-                principal = principal+"@"+self.api.env.realm
-            else:
-                principal = principal
+            if len(u) > 2:
+                raise errors.InvalidUserPrincipal, principal
+        else:
+            principal = principal+"@"+self.api.env.realm
         dn = self.Backend.ldap.find_entry_dn(
             "krbprincipalname",
             principal,
diff --git a/ipalib/plugins/f_pwpolicy.py b/ipalib/plugins/f_pwpolicy.py
index ce52e4678..87a7d8fa6 100644
--- a/ipalib/plugins/f_pwpolicy.py
+++ b/ipalib/plugins/f_pwpolicy.py
@@ -88,9 +88,8 @@ class pwpolicy_mod(frontend.Command):
 
         return ldap.update(dn, **kw)
 
-    def output_for_cli(self, ret):
-        if ret:
-            print "Policy modified"
+    def output_for_cli(self, textui, result, *args, **options):
+        textui.print_plain("Policy modified")
 
 api.register(pwpolicy_mod)
 
@@ -120,14 +119,12 @@ class pwpolicy_show(frontend.Command):
 
         return policy
 
-    def output_for_cli(self, policy):
-        if not policy: return
-
-        print "Password Policy"
-        print "Min. Password Lifetime (hours): %s" % policy.get('krbminpwdlife')
-        print "Max. Password Lifetime (days): %s" % policy.get('krbmaxpwdlife')
-        print "Min. Number of Character Classes: %s" % policy.get('krbpwdmindiffchars')
-        print "Min. Length of Password: %s" % policy.get('krbpwdminlength')
-        print "Password History Size: %s" % policy.get('krbpwdhistorylength')
+    def output_for_cli(self, textui, result, *args, **options):
+        textui.print_plain("Password Policy")
+        textui.print_plain("Min. Password Lifetime (hours): %s" % result.get('krbminpwdlife'))
+        textui.print_plain("Max. Password Lifetime (days): %s" % result.get('krbmaxpwdlife'))
+        textui.print_plain("Min. Number of Character Classes: %s" % result.get('krbpwdmindiffchars'))
+        textui.print_plain("Min. Length of Password: %s" % result.get('krbpwdminlength'))
+        textui.print_plain("Password History Size: %s" % result.get('krbpwdhistorylength'))
 
 api.register(pwpolicy_show)
diff --git a/ipalib/plugins/f_service.py b/ipalib/plugins/f_service.py
index 04187a863..a353d52e9 100644
--- a/ipalib/plugins/f_service.py
+++ b/ipalib/plugins/f_service.py
@@ -149,31 +149,30 @@ class service_find(crud.Find):
     def execute(self, principal, **kw):
         ldap = self.api.Backend.ldap
 
-        kw['filter'] = "&(objectclass=krbPrincipalAux)(!(objectClass=posixAccount))(!(|(krbprincipalname=kadmin/*)(krbprincipalname=K/M@*)(krbprincipalname=krbtgt/*)))"
-        kw['krbprincipalname'] = principal
+        search_kw = {}
+        search_kw['filter'] = "&(objectclass=krbPrincipalAux)(!(objectClass=posixAccount))(!(|(krbprincipalname=kadmin/*)(krbprincipalname=K/M@*)(krbprincipalname=krbtgt/*)))"
+        search_kw['krbprincipalname'] = principal
 
         object_type = ldap.get_object_type("krbprincipalname")
         if object_type and not kw.get('objectclass'):
-            kw['objectclass'] = object_type
+            search_kw['objectclass'] = object_type
 
-        return ldap.search(**kw)
+        return ldap.search(**search_kw)
 
-    def output_for_cli(self, services):
-        if not services:
-            return
-
-        counter = services[0]
-        services = services[1:]
+    def output_for_cli(self, textui, result, *args, **options):
+        counter = result[0]
+        services = result[1:]
         if counter == 0:
-            print "No entries found"
+            textui.print_plain("No entries found")
             return
-        elif counter == -1:
-            print "These results are truncated."
-            print "Please refine your search and try again."
 
         for s in services:
-            for a in s.keys():
-                print "%s: %s" % (a, s[a])
+            textui.print_entry(s)
+
+        if counter == -1:
+            textui.print_plain("These results are truncated.")
+            textui.print_plain("Please refine your search and try again.")
+        textui.print_count(services, '%d services matched')
 
 api.register(service_find)
 
@@ -196,11 +195,7 @@ class service_show(crud.Get):
         dn = ldap.find_entry_dn("krbprincipalname", principal)
         # FIXME: should kw contain the list of attributes to display?
         return ldap.retrieve(dn)
-    def output_for_cli(self, service):
-        if not service:
-            return
-
-        for a in service.keys():
-            print "%s: %s" % (a, service[a])
+    def output_for_cli(self, textui, result, *args, **options):
+        textui.print_entry(result)
 
 api.register(service_show)
diff --git a/ipalib/plugins/f_user.py b/ipalib/plugins/f_user.py
index 45ee59f4f..04d7c930a 100644
--- a/ipalib/plugins/f_user.py
+++ b/ipalib/plugins/f_user.py
@@ -264,17 +264,18 @@ class user_find(crud.Find):
         search_fields_conf_str = config.get('ipausersearchfields')
         search_fields = search_fields_conf_str.split(",")
 
+        search_kw = {}
         for s in search_fields:
-            kw[s] = term
+            search_kw[s] = term
 
         object_type = ldap.get_object_type("uid")
         if object_type and not kw.get('objectclass'):
-            kw['objectclass'] = object_type
+            search_kw['objectclass'] = object_type
         if kw.get('all', False):
-            kw['attributes'] = ['*']
+            search_kw['attributes'] = ['*']
         else:
-            kw['attributes'] = default_attributes
-        return ldap.search(**kw)
+            search_kw['attributes'] = default_attributes
+        return ldap.search(**search_kw)
 
     def output_for_cli(self, textui, result, uid, **options):
         counter = result[0]
@@ -287,7 +288,9 @@ class user_find(crud.Find):
             return
         textui.print_name(self.name)
         for u in users:
-            textui.print_plain('%(givenname)s %(sn)s:' % u)
+            gn = u.get('givenname', '')
+            sn= u.get('sn', '')
+            textui.print_plain('%s %s:' % (gn, sn))
             textui.print_entry(u)
             textui.print_plain('')
         if counter == -1: