Raise DuplicateEntry Error when adding a duplicate sudo option

https://fedorahosted.org/freeipa/ticket/1276
https://fedorahosted.org/freeipa/ticket/1277
https://fedorahosted.org/freeipa/ticket/1308

Added new Exception: AttrValueNotFound
Fixed XML Test for Sudorule remove_option
1276 (Raise AttrValueNotFound when trying to remove a non-existent option from Sudo rule)
1277 (Raise DuplicateEntry Error when adding a duplicate sudo option)
1308 (Make sudooption a required option for sudorule_remove_option)

5 files changed, 62 insertions, 31 deletions

diff --git a/API.txt b/API.txt
index 2f8c1a991..87a6e79ce 100644
--- a/API.txt
+++ b/API.txt
@@ -2495,7 +2495,7 @@ output: Output('completed', <type 'int'>, Gettext('Number of members removed', d
 command: sudorule_remove_option
 args: 1,1,1
 arg: Str('cn', attribute=True, cli_name='sudorule_name', label=Gettext('Rule name', domain='ipa', localedir=None), multivalue=False, primary_key=True, query=True, required=True)
-option: Str('ipasudoopt?', cli_name='sudooption', label=Gettext('Sudo Option', domain='ipa', localedir=None))
+option: Str('ipasudoopt', cli_name='sudooption', label=Gettext('Sudo Option', domain='ipa', localedir=None))
 output: Output('result', None, None)
 command: sudorule_remove_runasgroup
 args: 1,4,3
diff --git a/VERSION b/VERSION
index 6cbf732ea..e31f0d03e 100644
--- a/VERSION
+++ b/VERSION
@@ -79,4 +79,4 @@ IPA_DATA_VERSION=20100614120000
 #                                                      #
 ########################################################
 IPA_API_VERSION_MAJOR=2
-IPA_API_VERSION_MINOR=5
+IPA_API_VERSION_MINOR=6
diff --git a/ipalib/errors.py b/ipalib/errors.py
index 55bb8a38b..cad6e37b4 100644
--- a/ipalib/errors.py
+++ b/ipalib/errors.py
@@ -1192,6 +1192,24 @@ class ReverseMemberError(ExecutionError):
     format = _('A problem was encountered when verifying that all members were %(verb)s: %(exc)s')
 
 
+class AttrValueNotFound(ExecutionError):
+    """
+    **4026** Raised when an Attribute/Value pair is not found.
+
+    For example:
+
+    >>> raise NotFound(attr='ipasudoopt', value='authenticate')
+    Traceback (most recent call last):
+      ...
+    AttrValueNotFound: ipasudoopt does not contain 'authenticate'.
+
+    """
+
+    errno = 4026
+    rval = 1
+    format = _('%(attr)s does not contain \'%(value)s\'')
+
+
 class BuiltinError(ExecutionError):
     """
     **4100** Base class for builtin execution errors (*4100 - 4199*).
diff --git a/ipalib/plugins/sudorule.py b/ipalib/plugins/sudorule.py
index 2759534e0..a7fd82775 100644
--- a/ipalib/plugins/sudorule.py
+++ b/ipalib/plugins/sudorule.py
@@ -52,7 +52,7 @@ class sudorule(LDAPObject):
         'cn', 'ipaenabledflag',
         'description', 'usercategory', 'hostcategory',
         'cmdcategory', 'memberuser', 'memberhost',
-        'memberallowcmd', 'memberdenycmd',
+        'memberallowcmd', 'memberdenycmd', 'ipasudoopt',
     ]
     uuid_attribute = 'ipauniqueid'
     rdn_attribute = 'ipauniqueid'
@@ -611,11 +611,19 @@ class sudorule_add_option(LDAPQuery):
 
         dn = self.obj.get_dn(cn)
 
+        if not options['ipasudoopt'].strip():
+            raise errors.EmptyModlist()
         (dn, entry_attrs) = ldap.get_entry(dn, ['ipasudoopt'])
 
-        entry_attrs.setdefault('ipasudoopt', []).append(
-            options['ipasudoopt']
-        )
+        try:
+            if options['ipasudoopt'] not in entry_attrs['ipasudoopt']:
+                entry_attrs.setdefault('ipasudoopt', []).append(
+                    options['ipasudoopt'])
+            else:
+                raise errors.DuplicateEntry
+        except KeyError:
+            entry_attrs.setdefault('ipasudoopt', []).append(
+                options['ipasudoopt'])
         try:
             ldap.update_entry(dn, entry_attrs)
         except errors.EmptyModlist:
@@ -623,15 +631,12 @@ class sudorule_add_option(LDAPQuery):
         except errors.NotFound:
             self.obj.handle_not_found(cn)
 
-        return dict(result=entry_attrs)
-
-    def output_for_cli(self, textui, result, cn, **options):
-        textui.print_name(self.name)
-        textui.print_dashed(
-            'Added option "%s" to Sudo rule "%s"' % (
-                options['ipasudoopt'], cn
+        attrs_list = self.obj.default_attributes
+        (dn, entry_attrs) = ldap.get_entry(
+            dn, attrs_list, normalize=self.obj.normalize_dn
             )
-        )
+
+        return dict(result=entry_attrs)
 
 api.register(sudorule_add_option)
 
@@ -641,7 +646,7 @@ class sudorule_remove_option(LDAPQuery):
     Remove an option from Sudo rule.
     """
     takes_options = (
-        Str('ipasudoopt?',
+        Str('ipasudoopt',
             cli_name='sudooption',
             label=_('Sudo Option'),
         ),
@@ -652,25 +657,34 @@ class sudorule_remove_option(LDAPQuery):
 
         dn = self.obj.get_dn(cn)
 
+        if not options['ipasudoopt'].strip():
+            raise errors.EmptyModlist()
         (dn, entry_attrs) = ldap.get_entry(dn, ['ipasudoopt'])
         try:
-            entry_attrs.setdefault('ipasudoopt', []).remove(
-                options['ipasudoopt']
-            )
-            ldap.update_entry(dn, entry_attrs)
-        except (ValueError, errors.EmptyModlist):
+            if options['ipasudoopt'] in entry_attrs['ipasudoopt']:
+                entry_attrs.setdefault('ipasudoopt', []).remove(
+                    options['ipasudoopt'])
+                ldap.update_entry(dn, entry_attrs)
+            else:
+                raise errors.AttrValueNotFound(
+                    attr='ipasudoopt',
+                    value=options['ipasudoopt']
+                    )
+        except ValueError, e:
             pass
+        except KeyError:
+            raise errors.AttrValueNotFound(
+                    attr='ipasudoopt',
+                    value=options['ipasudoopt']
+                    )
         except errors.NotFound:
             self.obj.handle_not_found(cn)
 
-        return dict(result=True)
-
-    def output_for_cli(self, textui, result, cn, **options):
-        textui.print_name(self.name)
-        textui.print_dashed(
-            'Removed option "%s" from Sudo rule "%s"' % (
-                options['ipasudoopt'], cn
+        attrs_list = self.obj.default_attributes
+        (dn, entry_attrs) = ldap.get_entry(
+            dn, attrs_list, normalize=self.obj.normalize_dn
             )
-        )
+
+        return dict(result=entry_attrs)
 
 api.register(sudorule_remove_option)
diff --git a/tests/test_xmlrpc/test_sudorule_plugin.py b/tests/test_xmlrpc/test_sudorule_plugin.py
index 3de1adce0..88c608d61 100644
--- a/tests/test_xmlrpc/test_sudorule_plugin.py
+++ b/tests/test_xmlrpc/test_sudorule_plugin.py
@@ -316,7 +316,6 @@ class test_sudorule(XMLRPC_test):
         ret = api.Command['sudorule_add_runasgroup'](
             self.rule_name, group=self.test_external_group
         )
-        print ret
         assert ret['completed'] == 1
         failed = ret['failed']
         entry = ret['result']
@@ -330,7 +329,6 @@ class test_sudorule(XMLRPC_test):
         ret = api.Command['sudorule_remove_runasgroup'](
             self.rule_name, group=self.test_external_group
         )
-        print ret
         assert ret['completed'] == 1
         failed = ret['failed']
         entry = ret['result']
@@ -355,7 +353,8 @@ class test_sudorule(XMLRPC_test):
         ret = api.Command['sudorule_remove_option'](
             self.rule_name, ipasudoopt=self.test_option
         )
-        assert ret['result'] is True
+        entry = ret['result']
+        assert 'ipasudoopt' not in entry
 
     def test_a_sudorule_add_host(self):
         """