ACI plugin supports prefixes

When more than one plugin produce ACIs, they share common namespace
of ACI name. This may lead to name collisions between the ACIs
from different plugins.

This patch introduces a mandatory "prefix" attribute for non-find
ACI operations which allow plugins to use their own prefixes
(i.e. namespaces) which is then used when a name of the ACI is
generated.

Permission, Delegation and Selfservice plugins has been updated
to use their own prefixes thus avoiding name collisions by using
their own namespaces. Default ACIs in LDIFs has been updated to
follow this new policy.

Permission plugin now uses its CN (=primary key) instead of
description in ACI names as Description may not be unique.

This change requires an IPA server reinstall since the default ACI
set has been changed.

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

4 files changed, 131 insertions, 44 deletions

diff --git a/ipalib/plugins/aci.py b/ipalib/plugins/aci.py
index 7f9b0f216..648f5111f 100644
--- a/ipalib/plugins/aci.py
+++ b/ipalib/plugins/aci.py
@@ -89,22 +89,22 @@ command-line now (see last example).
 
  Add an ACI so that the group "secretaries" can update the address on any user:
    ipa group-add --desc="Office secretaries" secretaries
-   ipa aci-add --attrs=streetAddress --memberof=ipausers --group=secretaries --permissions=write "Secretaries write addresses"
+   ipa aci-add --attrs=streetAddress --memberof=ipausers --group=secretaries --permissions=write --prefix=none "Secretaries write addresses"
 
  Show the new ACI:
-   ipa aci-show "Secretaries write addresses"
+   ipa aci-show --prefix=none "Secretaries write addresses"
 
  Add an ACI that allows members of the "addusers" permission to add new users:
-   ipa aci-add --type=user --permission=addusers --permissions=add "Add new users"
+   ipa aci-add --type=user --permission=addusers --permissions=add --prefix=none "Add new users"
 
  Add an ACI that allows members of the editors manage members of the admins group:
-   ipa aci-add --permissions=write --attrs=member --targetgroup=admins --group=editors "Editors manage admins"
+   ipa aci-add --permissions=write --attrs=member --targetgroup=admins --group=editors --prefix=none "Editors manage admins"
 
  Add an ACI that allows members of the admin group to manage the street and zip code of those in the editors group:
-   ipa aci-add --permissions=write --memberof=editors --group=admins --attrs=street,postalcode "admins edit the address of editors"
+   ipa aci-add --permissions=write --memberof=editors --group=admins --attrs=street,postalcode --prefix=none "admins edit the address of editors"
 
  Add an ACI that allows the admins group manage the street and zipcode of those who work for the boss:
-   ipa aci-add --permissions=write --group=admins --attrs=street,postalcode --filter="(manager=uid=boss,cn=users,cn=accounts,dc=example,dc=com)" "Edit the address of those who work for the boss"
+   ipa aci-add --permissions=write --group=admins --attrs=street,postalcode --filter="(manager=uid=boss,cn=users,cn=accounts,dc=example,dc=com)" --prefix=none "Edit the address of those who work for the boss"
 
  Add an entirely new kind of record to IPA that isn't covered by any of the --type options, creating a permission:
    ipa permission-add  --permissions=add --subtree="cn=*,cn=orange,cn=accounts,dc=example,dc=com" --desc="Add Orange Entries" add_orange
@@ -128,6 +128,8 @@ if api.env.in_server and api.env.context in ['lite', 'server']:
     from ldap import explode_dn
 import logging
 
+ACI_NAME_PREFIX_SEP = ":"
+
 _type_map = {
     'user': 'ldap:///uid=*,%s,%s' % (api.env.container_user, api.env.basedn),
     'group': 'ldap:///cn=*,%s,%s' % (api.env.container_group, api.env.basedn),
@@ -142,6 +144,10 @@ _valid_permissions_values = [
     u'read', u'write', u'add', u'delete', u'all'
 ]
 
+_valid_prefix_values = (
+    u'permission', u'delegation', u'selfservice', u'none'
+)
+
 class ListOfACI(output.Output):
     type = (list, tuple)
     doc = _('A list of ACI values')
@@ -162,6 +168,26 @@ aci_output = (
 )
 
 
+def _make_aci_name(aciprefix, aciname):
+    """
+    Given a name and a prefix construct an ACI name.
+    """
+    if aciprefix == u"none":
+        return aciname
+
+    return aciprefix + ACI_NAME_PREFIX_SEP + aciname
+
+def _parse_aci_name(aciname):
+    """
+    Parse the raw ACI name and return a tuple containing the ACI prefix
+    and the actual ACI name.
+    """
+    aciparts = aciname.partition(ACI_NAME_PREFIX_SEP)
+
+    if not aciparts[2]: # no prefix/name separator found
+        return (u"none",aciparts[0])
+
+    return (aciparts[0], aciparts[2])
 
 def _make_aci(ldap, current, aciname, kw):
     """
@@ -177,6 +203,9 @@ def _make_aci(ldap, current, aciname, kw):
     if t1 + t2 + t3 + t4 > 1:
         raise errors.ValidationError(name='target', error=_('type, filter, subtree and targetgroup are mutually exclusive'))
 
+    if 'aciprefix' not in kw:
+        raise errors.ValidationError(name='aciprefix', error=_('ACI prefix is required'))
+
     if t1 + t2 + t3 + t4 + t5 + t6 == 0:
         raise errors.ValidationError(name='target', error=_('at least one of: type, filter, subtree, targetgroup, attrs or memberof are required'))
 
@@ -209,7 +238,7 @@ def _make_aci(ldap, current, aciname, kw):
 
     try:
         a = ACI(current)
-        a.name = aciname
+        a.name = _make_aci_name(kw['aciprefix'], aciname)
         a.permissions = kw['permissions']
         if 'selfaci' in kw and kw['selfaci']:
             a.set_bindrule('userdn = "ldap:///self"')
@@ -260,7 +289,7 @@ def _aci_to_kw(ldap, a, test=False):
        _make_aci().
     """
     kw = {}
-    kw['aciname'] = a.name
+    kw['aciprefix'], kw['aciname'] = _parse_aci_name(a.name)
     kw['permissions'] = tuple(a.permissions)
     if 'targetattr' in a.target:
         kw['attrs'] = list(a.target['targetattr']['expression'])
@@ -328,9 +357,10 @@ def _convert_strings_to_acis(acistrs):
             logging.warn("Failed to parse: %s" % a)
     return acis
 
-def _find_aci_by_name(acis, aciname):
+def _find_aci_by_name(acis, aciprefix, aciname):
+    name = _make_aci_name(aciprefix, aciname).lower()
     for a in acis:
-        if a.name.lower() == aciname.lower():
+        if a.name.lower() == name:
             return a
     raise errors.NotFound(reason=_('ACI with name "%s" not found') % aciname)
 
@@ -351,6 +381,13 @@ def _normalize_permissions(permissions):
             valid_permissions.append(p)
     return ','.join(valid_permissions)
 
+_prefix_option = StrEnum('aciprefix',
+                cli_name='prefix',
+                label=_('ACI prefix'),
+                doc=_('Prefix used to distinguish ACI types ' \
+                    '(permission, delegation, selfservice, none)'),
+                values=_valid_prefix_values,
+                )
 
 class aci(Object):
     """
@@ -423,7 +460,6 @@ class aci(Object):
 
 api.register(aci)
 
-
 class aci_add(crud.Create):
     """
     Create new ACI.
@@ -432,6 +468,7 @@ class aci_add(crud.Create):
     msg_summary = _('Created ACI "%(value)s"')
 
     takes_options = (
+        _prefix_option,
         Flag('test?',
              doc=_('Test the ACI syntax but don\'t write anything'),
              default=False,
@@ -486,6 +523,8 @@ class aci_del(crud.Delete):
     has_output = output.standard_boolean
     msg_summary = _('Deleted ACI "%(value)s"')
 
+    takes_options = (_prefix_option,)
+
     def execute(self, aciname, **kw):
         """
         Execute the aci-delete operation.
@@ -500,7 +539,7 @@ class aci_del(crud.Delete):
 
         acistrs = entry_attrs.get('aci', [])
         acis = _convert_strings_to_acis(acistrs)
-        aci = _find_aci_by_name(acis, aciname)
+        aci = _find_aci_by_name(acis, kw['aciprefix'], aciname)
         for a in acistrs:
             candidate = ACI(a)
             if aci.isequal(candidate):
@@ -530,6 +569,8 @@ class aci_mod(crud.Update):
         ),
     )
 
+    takes_options = (_prefix_option,)
+
     msg_summary = _('Modified ACI "%(value)s"')
 
     def execute(self, aciname, **kw):
@@ -538,7 +579,7 @@ class aci_mod(crud.Update):
         (dn, entry_attrs) = ldap.get_entry(self.api.env.basedn, ['aci'])
 
         acis = _convert_strings_to_acis(entry_attrs.get('aci', []))
-        aci = _find_aci_by_name(acis, aciname)
+        aci = _find_aci_by_name(acis, kw['aciprefix'], aciname)
 
         # 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
@@ -558,7 +599,7 @@ class aci_mod(crud.Update):
         if aci.isequal(newaci):
             raise errors.EmptyModlist()
 
-        self.api.Command['aci_del'](aciname)
+        self.api.Command['aci_del'](aciname, **kw)
 
         result = self.api.Command['aci_add'](aciname, **newkw)['result']
 
@@ -597,6 +638,8 @@ class aci_find(crud.Search):
     NO_CLI = True
     msg_summary = ngettext('%(count)d ACI matched', '%(count)d ACIs matched', 0)
 
+    takes_options = (_prefix_option.clone_rename("aciprefix?", required=False),)
+
     def execute(self, term, **kw):
         ldap = self.api.Backend.ldap2
 
@@ -616,7 +659,15 @@ class aci_find(crud.Search):
 
         if 'aciname' in kw:
             for a in acis:
-                if a.name != kw['aciname']:
+                prefix, name = _parse_aci_name(a.name)
+                if name != kw['aciname']:
+                    results.remove(a)
+            acis = list(results)
+
+        if 'aciprefix' in kw:
+            for a in acis:
+                prefix, name = _parse_aci_name(a.name)
+                if prefix != kw['aciprefix']:
                     results.remove(a)
             acis = list(results)
 
@@ -760,6 +811,8 @@ class aci_show(crud.Retrieve):
         ),
     )
 
+    takes_options = (_prefix_option,)
+
     def execute(self, aciname, **kw):
         """
         Execute the aci-show operation.
@@ -775,7 +828,7 @@ class aci_show(crud.Retrieve):
 
         acis = _convert_strings_to_acis(entry_attrs.get('aci', []))
 
-        aci = _find_aci_by_name(acis, aciname)
+        aci = _find_aci_by_name(acis, kw['aciprefix'], aciname)
         if kw.get('raw', False):
             result = dict(aci=unicode(aci))
         else:
@@ -800,12 +853,13 @@ class aci_rename(crud.Update):
     )
 
     takes_options = (
+        _prefix_option,
         Str('newname',
              doc=_('New ACI name'),
         ),
     )
 
-    msg_summary = _('Renameed ACI to "%(value)s"')
+    msg_summary = _('Renamed ACI to "%(value)s"')
 
     def execute(self, aciname, **kw):
         ldap = self.api.Backend.ldap2
@@ -813,10 +867,11 @@ class aci_rename(crud.Update):
         (dn, entry_attrs) = ldap.get_entry(self.api.env.basedn, ['aci'])
 
         acis = _convert_strings_to_acis(entry_attrs.get('aci', []))
-        aci = _find_aci_by_name(acis, aciname)
+        aci = _find_aci_by_name(acis, kw['aciprefix'], aciname)
 
         for a in acis:
-            if kw['newname'] == a.name:
+            prefix, name = _parse_aci_name(a.name)
+            if _make_aci_name(prefix, kw['newname']) == a.name:
                 raise errors.DuplicateEntry()
 
         # The strategy here is to convert the ACI we're updating back into
@@ -833,7 +888,7 @@ class aci_rename(crud.Update):
         # Do this before we delete the existing ACI.
         newaci = _make_aci(ldap, None, kw['newname'], newkw)
 
-        self.api.Command['aci_del'](aciname)
+        self.api.Command['aci_del'](aciname, **kw)
 
         result = self.api.Command['aci_add'](kw['newname'], **newkw)['result']
 
diff --git a/ipalib/plugins/delegation.py b/ipalib/plugins/delegation.py
index 19d4c6da6..d5ff08a82 100644
--- a/ipalib/plugins/delegation.py
+++ b/ipalib/plugins/delegation.py
@@ -50,6 +50,8 @@ from ipalib import api, crud, errors
 from ipalib import output
 from ipalib import Object, Command
 
+ACI_PREFIX=u"delegation"
+
 def convert_delegation(ldap, aci):
     """
     memberOf is in filter but we want to pull out the group for easier
@@ -70,6 +72,7 @@ def convert_delegation(ldap, aci):
     aci['membergroup'] = entry_attrs['cn']
 
     del aci['filter']
+    del aci['aciprefix']     # do not include prefix in result
 
     return aci
 
@@ -81,7 +84,7 @@ def is_delegation(ldap, aciname):
     Return the result if it is a delegation ACI, adding a new attribute
     membergroup.
     """
-    result = api.Command['aci_show'](aciname)['result']
+    result = api.Command['aci_show'](aciname, aciprefix=ACI_PREFIX)['result']
     if 'filter' in result:
         result = convert_delegation(ldap, result)
     else:
@@ -157,6 +160,7 @@ class delegation_add(crud.Create):
         ldap = self.api.Backend.ldap2
         if not 'permissions' in kw:
             kw['permissions'] = (u'write',)
+        kw['aciprefix'] = ACI_PREFIX
         result = api.Command['aci_add'](aciname, **kw)['result']
         if 'filter' in result:
             result = convert_delegation(ldap, result)
@@ -180,6 +184,7 @@ class delegation_del(crud.Delete):
     def execute(self, aciname, **kw):
         ldap = self.api.Backend.ldap2
         is_delegation(ldap, aciname)
+        kw['aciprefix'] = ACI_PREFIX
         result = api.Command['aci_del'](aciname, **kw)
         return dict(
             result=True,
@@ -199,6 +204,7 @@ class delegation_mod(crud.Update):
     def execute(self, aciname, **kw):
         ldap = self.api.Backend.ldap2
         is_delegation(ldap, aciname)
+        kw['aciprefix'] = ACI_PREFIX
         result = api.Command['aci_mod'](aciname, **kw)['result']
         if 'filter' in result:
             result = convert_delegation(ldap, result)
@@ -221,6 +227,7 @@ class delegation_find(crud.Search):
 
     def execute(self, term, **kw):
         ldap = self.api.Backend.ldap2
+        kw['aciprefix'] = ACI_PREFIX
         acis = api.Command['aci_find'](term, **kw)['result']
         results = []
         for aci in acis:
diff --git a/ipalib/plugins/permission.py b/ipalib/plugins/permission.py
index 14d7b9656..0c2855ff5 100644
--- a/ipalib/plugins/permission.py
+++ b/ipalib/plugins/permission.py
@@ -61,7 +61,7 @@ EXAMPLES:
    ipa permission-add --desc="Add a User" --type=user --permissions=add adduser
 
  Add a permission that grants the ability to manage group membership:
-   ipa permission-add --desc='Manage group members' --attrs=member --permissions=-write --type=group manage_group_members
+   ipa permission-add --desc='Manage group members' --attrs=member --permissions=write --type=group manage_group_members
 """
 
 import copy
@@ -70,6 +70,7 @@ from ipalib import api, _, ngettext
 from ipalib import Flag, Str, StrEnum
 from ipalib.request import context
 
+ACI_PREFIX=u"permission"
 
 class permission(LDAPObject):
     """
@@ -167,8 +168,9 @@ class permission_add(LDAPCreate):
         del opts['description']
         opts['test'] = True
         opts['permission'] = keys[-1]
+        opts['aciprefix'] = ACI_PREFIX
         try:
-            self.api.Command.aci_add(options['description'], **opts)
+            self.api.Command.aci_add(keys[-1], **opts)
         except Exception, e:
             raise e
 
@@ -187,8 +189,9 @@ class permission_add(LDAPCreate):
         del opts['description']
         opts['test'] = False
         opts['permission'] = keys[-1]
+        opts['aciprefix'] = ACI_PREFIX
         try:
-            result = self.api.Command.aci_add(options['description'], **opts)['result']
+            result = self.api.Command.aci_add(keys[-1], **opts)['result']
             for attr in self.obj.aci_attributes:
                 if attr in result:
                     entry_attrs[attr] = result[attr]
@@ -204,7 +207,7 @@ class permission_add(LDAPCreate):
             except Exception, ignore:
                 pass
             try:
-                self.api.Command.aci_del(keys[-1])
+                self.api.Command.aci_del(keys[-1], aciprefix=ACI_PREFIX)
             except Exception, ignore:
                 pass
             raise e
@@ -221,12 +224,11 @@ class permission_del(LDAPDelete):
     msg_summary = _('Deleted permission "%(value)s"')
 
     def pre_callback(self, ldap, dn, *keys, **options):
-        (dn, entry_attrs) = ldap.get_entry(dn, ['*'])
-        if 'description' in entry_attrs:
-            try:
-                self.api.Command.aci_del(entry_attrs['description'][0])
-            except errors.NotFound:
-                pass
+        # remove permission even when the underlying ACI is missing
+        try:
+            self.api.Command.aci_del(keys[-1], aciprefix=ACI_PREFIX)
+        except errors.NotFound:
+            pass
         return dn
 
 api.register(permission_del)
@@ -247,9 +249,7 @@ class permission_mod(LDAPUpdate):
         except errors.NotFound:
             self.obj.handle_not_found(*keys)
         opts = copy.copy(options)
-        if 'description' in opts:
-            del opts['description']
-        for o in ['all', 'raw', 'rights', 'description']:
+        for o in ['all', 'raw', 'rights', 'description', 'rename']:
             if o in opts:
                 del opts[o]
         setattr(context, 'aciupdate', False)
@@ -258,8 +258,9 @@ class permission_mod(LDAPUpdate):
         if len(opts) > 0:
             opts['test'] = False
             opts['permission'] = keys[-1]
+            opts['aciprefix'] = ACI_PREFIX
             try:
-                self.api.Command.aci_mod(attrs['description'][0], **opts)
+                self.api.Command.aci_mod(keys[-1], **opts)
                 setattr(context, 'aciupdate', True)
             except Exception, e:
                 raise e
@@ -271,10 +272,6 @@ class permission_mod(LDAPUpdate):
             except:
                 pass
 
-        if 'description' in options:
-            if attrs['description'][0] != options['description']:
-                self.api.Command.aci_rename(attrs['description'][0], newname=options['description'])
-
         return dn
 
     def exc_callback(self, keys, options, exc, call_func, *call_args, **call_kwargs):
@@ -294,6 +291,15 @@ class permission_mod(LDAPUpdate):
             raise exc
 
     def post_callback(self, ldap, dn, entry_attrs, *keys, **options):
+        # rename the underlying ACI after the change to permission
+        if 'rename' in options:
+            aciname = keys[-1]   # ACI still refers to the old permission CN
+            self.api.Command.aci_mod(aciname,aciprefix=ACI_PREFIX,
+                        permission=options['rename'])
+
+            self.api.Command.aci_rename(aciname, aciprefix=ACI_PREFIX,
+                        newname=keys[-1], newprefix=ACI_PREFIX)
+
         result = self.api.Command.permission_show(keys[-1])['result']
         for r in result:
             if not r.startswith('member'):
@@ -317,24 +323,29 @@ class permission_find(LDAPSearch):
         for entry in entries:
             (dn, attrs) = entry
             try:
-                aci = self.api.Command.aci_show(attrs['description'][0])['result']
+                aci = self.api.Command.aci_show(attrs['cn'][0], aciprefix=ACI_PREFIX)['result']
+
+                # copy information from respective ACI to permission entry
                 for attr in self.obj.aci_attributes:
                     if attr in aci:
                         attrs[attr] = aci[attr]
             except errors.NotFound:
-                self.debug('ACI not found for %s' % attrs['description'][0])
+                self.debug('ACI not found for %s' % attrs['cn'][0])
 
         # Now find all the ACIs that match. Once we find them, add any that
         # aren't already in the list along with their permission info.
+        options['aciprefix'] = ACI_PREFIX
+
         aciresults = self.api.Command.aci_find(*args, **options)
         truncated = truncated or aciresults['truncated']
         results = aciresults['result']
+
         for aci in results:
             found = False
             if 'permission' in aci:
                 for entry in entries:
                     (dn, attrs) = entry
-                    if aci['permission'] == attrs['cn']:
+                    if aci['permission'] == attrs['cn'][0]:
                         found = True
                         break
                 if not found:
@@ -359,7 +370,7 @@ class permission_show(LDAPRetrieve):
     """
     def post_callback(self, ldap, dn, entry_attrs, *keys, **options):
         try:
-            aci = self.api.Command.aci_show(entry_attrs['description'][0])['result']
+            aci = self.api.Command.aci_show(keys[-1], aciprefix=ACI_PREFIX)['result']
             for attr in self.obj.aci_attributes:
                 if attr in aci:
                     entry_attrs[attr] = aci[attr]
diff --git a/ipalib/plugins/selfservice.py b/ipalib/plugins/selfservice.py
index adf6acb79..557304241 100644
--- a/ipalib/plugins/selfservice.py
+++ b/ipalib/plugins/selfservice.py
@@ -50,6 +50,8 @@ from ipalib import api, crud, errors
 from ipalib import output
 from ipalib import Object, Command
 
+ACI_PREFIX=u"selfservice"
+
 def is_selfservice(aciname):
     """
     Determine if the ACI is a Self-Service ACI and raise an exception if it
@@ -57,7 +59,7 @@ def is_selfservice(aciname):
 
     Return the result if it is a self-service ACI.
     """
-    result = api.Command['aci_show'](aciname)['result']
+    result = api.Command['aci_show'](aciname, aciprefix=ACI_PREFIX)['result']
     if 'selfaci' not in result or result['selfaci'] == False:
         raise errors.NotFound(reason=_('Self-service permission \'%(permission)s\' not found') % dict(permission=aciname))
     return result
@@ -119,7 +121,9 @@ class selfservice_add(crud.Create):
         if not 'permissions' in kw:
             kw['permissions'] = (u'write',)
         kw['selfaci'] = True
+        kw['aciprefix'] = ACI_PREFIX
         result = api.Command['aci_add'](aciname, **kw)['result']
+        del result['aciprefix']     # do not include prefix in result
 
         return dict(
             result=result,
@@ -139,7 +143,9 @@ class selfservice_del(crud.Delete):
 
     def execute(self, aciname, **kw):
         is_selfservice(aciname)
+        kw['aciprefix'] = ACI_PREFIX
         result = api.Command['aci_del'](aciname, **kw)
+
         return dict(
             result=True,
             value=aciname,
@@ -159,7 +165,10 @@ class selfservice_mod(crud.Update):
         is_selfservice(aciname)
         if 'attrs' in kw and kw['attrs'] is None:
             raise errors.RequirementError(name='attrs')
+
+        kw['aciprefix'] = ACI_PREFIX
         result = api.Command['aci_mod'](aciname, **kw)['result']
+        del result['aciprefix']     # do not include prefix in result
         return dict(
             result=result,
             value=aciname,
@@ -179,8 +188,12 @@ class selfservice_find(crud.Search):
 
     def execute(self, term, **kw):
         kw['selfaci'] = True
+        kw['aciprefix'] = ACI_PREFIX
         result = api.Command['aci_find'](term, **kw)['result']
 
+        for aci in result:
+            del aci['aciprefix']     # do not include prefix in result
+
         return dict(
             result=result,
             count=len(result),
@@ -202,6 +215,7 @@ class selfservice_show(crud.Retrieve):
 
     def execute(self, aciname, **kw):
         result = is_selfservice(aciname)
+        del result['aciprefix']     # do not include prefix in result
         return dict(
             result=result,
             value=aciname,