ipa permission-mod --rename does not work

This patch fixes nonfunctional rename operation in permission
plugin. Also makes sure, that no change is made to the underlying
ACI in pre_callback() when the target permission already exists.

Several tests for the rename operation have been created to ensure
that the it won't break again unnoticed.

https://fedorahosted.org/freeipa/ticket/814

ipalib/plugins/permission.py

@@ -242,12 +242,26 @@ class permission_mod(LDAPUpdate):
     msg_summary = _('Modified permission "%(value)s"')
 
     def pre_callback(self, ldap, dn, entry_attrs, attrs_list, *keys, **options):
+        # check if permission is in LDAP
         try:
             (dn, attrs) = ldap.get_entry(
                 dn, attrs_list, normalize=self.obj.normalize_dn
             )
         except errors.NotFound:
             self.obj.handle_not_found(*keys)
+
+        # when renaming permission, check if the target permission does not
+        # exists already. Then, make changes to underlying ACI
+        if 'rename' in options:
+            try:
+                new_dn = dn.replace(keys[-1], options['rename'], 1)
+                (new_dn, attrs) = ldap.get_entry(
+                    new_dn, attrs_list, normalize=self.obj.normalize_dn
+                )
+                raise errors.DuplicateEntry()
+            except errors.NotFound:
+                pass    # permission may be renamed, continue
+
         opts = copy.copy(options)
         for o in ['all', 'raw', 'rights', 'description', 'rename']:
             if o in opts:
@@ -292,15 +306,18 @@ class permission_mod(LDAPUpdate):
 
     def post_callback(self, ldap, dn, entry_attrs, *keys, **options):
         # rename the underlying ACI after the change to permission
+        cn = keys[-1]
+
         if 'rename' in options:
-            aciname = keys[-1]   # ACI still refers to the old permission CN
-            self.api.Command.aci_mod(aciname,aciprefix=ACI_PREFIX,
+            self.api.Command.aci_mod(cn,aciprefix=ACI_PREFIX,
                         permission=options['rename'])
 
-            self.api.Command.aci_rename(aciname, aciprefix=ACI_PREFIX,
-                        newname=keys[-1], newprefix=ACI_PREFIX)
+            self.api.Command.aci_rename(cn, aciprefix=ACI_PREFIX,
+                        newname=options['rename'], newprefix=ACI_PREFIX)
 
-        result = self.api.Command.permission_show(keys[-1])['result']
+            cn = options['rename']     # rename finished
+
+        result = self.api.Command.permission_show(cn)['result']
         for r in result:
             if not r.startswith('member'):
                 entry_attrs[r] = result[r]

tests/test_xmlrpc/test_permission_plugin.py

@@ -32,6 +32,12 @@ permission1_dn = u'cn=%s,%s,%s' % (permission1,
 )
 
 
+permission1_renamed = u'testperm1_rn'
+permission1_renamed_dn = u'cn=%s,%s,%s' % (permission1_renamed,
+    api.env.container_permission,api.env.basedn,
+)
+
+
 permission2 = u'testperm2'
 permission2_dn = u'cn=%s,%s,%s' % (permission2,
     api.env.container_permission,api.env.basedn,
@@ -332,13 +338,67 @@ class test_permission(Declarative):
         ),
 
 
+
         dict(
-            desc='Delete %r' % permission1,
-            command=('permission_del', [permission1], {}),
+            desc='Try to rename %r to existing permission %r' % (permission1,
+                                                                 permission2),
+            command=(
+                'permission_mod', [permission1], dict(rename=permission2,
+                                                      description=u"Renamed Desc",
+                                                      permissions=u'read',)
+            ),
+            expected=errors.DuplicateEntry(),
+        ),
+
+
+        dict(
+            desc='Check integrity of original permission %r' % permission1,
+            command=('permission_show', [permission1], {}),
+            expected=dict(
+                value=permission1,
+                summary=None,
+                result={
+                    'dn': permission1_dn,
+                    'cn': [permission1],
+                    'description': [u'New desc 1'],
+                    'member_privilege': [privilege1],
+                    'type': u'user',
+                    'permissions': [u'write'],
+                },
+            ),
+        ),
+
+
+        dict(
+            desc='Rename %r to permission %r' % (permission1,
+                                                 permission1_renamed),
+            command=(
+                'permission_mod', [permission1], dict(rename=permission1_renamed,
+                                                      description=u"Renamed Desc",
+                                                      permissions= u'read',)
+            ),
+            expected=dict(
+                value=permission1,
+                summary=u'Modified permission "%s"' % permission1,
+                result={
+                    'dn': permission1_renamed_dn,
+                    'cn': [permission1_renamed],
+                    'description': [u'Renamed Desc'],
+                    'member_privilege': [privilege1],
+                    'type': u'user',
+                    'permissions': [u'read'],
+                },
+            ),
+        ),
+
+
+        dict(
+            desc='Delete %r' % permission1_renamed,
+            command=('permission_del', [permission1_renamed], {}),
             expected=dict(
                 result=dict(failed=u''),
-                value=permission1,
-                summary=u'Deleted permission "%s"' % permission1,
+                value=permission1_renamed,
+                summary=u'Deleted permission "%s"' % permission1_renamed,
             )
         ),