Restore ACI when aci_mod fails

aci_mod command is composed of 2 ACI commands: aci_del which
deletes the old ACI and aci_add which adds the new modified ACI.
However, if aci_add command fails then both new and the old ACI
are lost. Old ACI must be restored in this case.

https://fedorahosted.org/freeipa/ticket/2013
https://fedorahosted.org/freeipa/ticket/2014

ipalib/plugins/aci.py

@@ -117,6 +117,7 @@ must include all existing attributes as well. When doing an aci-mod the
 targetattr REPLACES the current attributes, it does not add to them.
 
 """
+from copy import deepcopy
 
 from ipalib import api, crud, errors
 from ipalib import Object, Command
@@ -614,14 +615,18 @@ class aci_mod(crud.Update):
         # The strategy here is to convert the ACI we're updating back into
         # a series of keywords. Then we replace any keywords that have been
         # updated and convert that back into an ACI and write it out.
-        newkw =  _aci_to_kw(ldap, aci)
+        oldkw = _aci_to_kw(ldap, aci)
+        newkw = deepcopy(oldkw)
         if 'selfaci' in newkw and newkw['selfaci'] == True:
             # selfaci is set in aci_to_kw to True only if the target is self
             kw['selfaci'] = True
         for k in kw.keys():
             newkw[k] = kw[k]
-        if 'aciname' in newkw:
-            del newkw['aciname']
+        for acikw in (oldkw, newkw):
+            try:
+                del acikw['aciname']
+            except KeyError:
+                pass
 
         # _make_aci is what is run in aci_add and validates the input.
         # Do this before we delete the existing ACI.
@@ -631,7 +636,16 @@ class aci_mod(crud.Update):
 
         self.api.Command['aci_del'](aciname, **kw)
 
+        try:
             result = self.api.Command['aci_add'](aciname, **newkw)['result']
+        except Exception, e:
+            # ACI could not be added, try to restore the old deleted ACI and
+            # report the ADD error back to user
+            try:
+                self.api.Command['aci_add'](aciname, **oldkw)
+            except:
+                pass
+            raise e
 
         if kw.get('raw', False):
             result = dict(aci=unicode(newaci))

tests/test_xmlrpc/test_selfservice_plugin.py

@@ -172,6 +172,31 @@ class test_selfservice(Declarative):
         ),
 
 
+        dict(
+            desc='Try to update %r with empty permissions' % selfservice1,
+            command=(
+                'selfservice_mod', [selfservice1], dict(permissions=None)
+            ),
+            expected=errors.RequirementError(name='permissions'),
+        ),
+
+
+        dict(
+            desc='Retrieve %r to verify invalid update' % selfservice1,
+            command=('selfservice_show', [selfservice1], {}),
+            expected=dict(
+                value=selfservice1,
+                summary=None,
+                result={
+                        'attrs': [u'street', u'c', u'l', u'st', u'postalcode'],
+                        'permissions': [u'read'],
+                        'selfaci': True,
+                        'aciname': selfservice1,
+                },
+            ),
+        ),
+
+
         dict(
             desc='Delete %r' % selfservice1,
             command=('selfservice_del', [selfservice1], {}),