Hello,
A permission object was not removed in permission-add when adding the ACI failed. Here is a fix.

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


Earlier we agreed that patch authors should bug the reviewer. I guess now this means I should set Patch-review-by in the ticket, right? So: Martin, you reviewed the other ACI patches so I think you should continue. If you don't agree, unset the field in the ticket.

--
Petr³
From 5ad2066b71b09248d348a5c4c85ef2ace0c553a4 Mon Sep 17 00:00:00 2001
From: Petr Viktorin <[email protected]>
Date: Fri, 21 Feb 2014 13:58:15 +0100
Subject: [PATCH] permission_add: Remove permission entry if adding the ACI
 fails

https://fedorahosted.org/freeipa/ticket/4187
---
 ipalib/plugins/permission.py                   | 15 ++++++++++++++-
 ipatests/test_xmlrpc/test_permission_plugin.py | 25 +++++++++++++++++++++++++
 2 files changed, 39 insertions(+), 1 deletion(-)

diff --git a/ipalib/plugins/permission.py b/ipalib/plugins/permission.py
index 64deb99ef98583daf0419a240aa8852b0262874d..cb6f18b478735920bbf6cef4febc91481631c560 100644
--- a/ipalib/plugins/permission.py
+++ b/ipalib/plugins/permission.py
@@ -812,7 +812,20 @@ def pre_callback(self, ldap, dn, entry, attrs_list, *keys, **options):
         return dn
 
     def post_callback(self, ldap, dn, entry, *keys, **options):
-        self.obj.add_aci(entry)
+        try:
+            self.obj.add_aci(entry)
+        except Exception:
+            # Adding the ACI failed.
+            # We want to be 100% sure the ACI is not there, so try to
+            # remove it. (This is a no-op if the ACI was not added.)
+            self.obj.remove_aci(entry)
+            # Remove the entry
+            try:
+                self.api.Backend['ldap2'].delete_entry(entry)
+            except errors.NotFound:
+                pass
+            # Re-raise original exception
+            raise
         self.obj.postprocess_result(entry, options)
         return dn
 
diff --git a/ipatests/test_xmlrpc/test_permission_plugin.py b/ipatests/test_xmlrpc/test_permission_plugin.py
index 4903bfae340dd8955a170bbb2c8121468bc47a18..7693fe29b5de3fc4f2fb1c05b1d5c806e7786e70 100644
--- a/ipatests/test_xmlrpc/test_permission_plugin.py
+++ b/ipatests/test_xmlrpc/test_permission_plugin.py
@@ -220,6 +220,31 @@ class test_permission_negative(Declarative):
         verify_permission_aci_missing(permission1, users_dn),
 
         dict(
+            desc='Try creating %r with bad attribute name' % permission1,
+            command=(
+                'permission_add', [permission1], dict(
+                    type=u'user',
+                    ipapermright=u'write',
+                    attrs=u'bogusattr',
+                )
+            ),
+            expected=errors.InvalidSyntax(
+                attr=r'targetattr "bogusattr" does not exist in schema. '
+                     r'Please add attributeTypes "bogusattr" to '
+                     r'schema if necessary. '
+                     r'ACL Syntax Error(-5):'
+                     r'(targetattr = \22bogusattr\22)'
+                     r'(targetfilter = \22(objectclass=posixaccount)\22)'
+                     r'(version 3.0;acl \22permission:%(name)s\22;'
+                     r'allow (write) groupdn = \22ldap:///%(dn)s\22;)' % dict(
+                        name=permission1,
+                        dn=permission1_dn),
+            ),
+        ),
+
+        verify_permission_aci_missing(permission1, users_dn),
+
+        dict(
             desc='Create %r so we can try breaking it' % permission1,
             command=(
                 'permission_add', [permission1], dict(
-- 
1.8.5.3

_______________________________________________
Freeipa-devel mailing list
[email protected]
https://www.redhat.com/mailman/listinfo/freeipa-devel

Reply via email to