Fix changing membergroup in a delegation.

This is mostly due to inconsistent option name usage but also due
to the aci plugin not always treating memberof as a special kind
of filter.

ticket 869

3 files changed, 29 insertions, 19 deletions

diff --git a/ipalib/plugins/aci.py b/ipalib/plugins/aci.py
index 4ddaf98a..903c600b 100644
--- a/ipalib/plugins/aci.py
+++ b/ipalib/plugins/aci.py
@@ -189,6 +189,17 @@ def _parse_aci_name(aciname):
 
     return (aciparts[0], aciparts[2])
 
+def _group_from_memberof(memberof):
+    """
+    Pull the group name out of a memberOf filter
+    """
+    st = memberof.find('memberOf=')
+    if st == -1:
+        # We have a raw group name, use that
+        return api.Object['group'].get_dn(memberof)
+    en = memberof.find(')', st)
+    return memberof[st+9:en]
+
 def _make_aci(ldap, current, aciname, kw):
     """
     Given a name and a set of keywords construct an ACI.
@@ -209,6 +220,9 @@ def _make_aci(ldap, current, aciname, kw):
     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'))
 
+    if t2 + t6 > 1:
+        raise errors.ValidationError(name='target', error=_('filter and memberof are mutually exclusive'))
+
     group = 'group' in kw
     permission = 'permission' in kw
     selfaci = 'selfaci' in kw and kw['selfaci'] == True
@@ -248,8 +262,8 @@ def _make_aci(ldap, current, aciname, kw):
         if 'attrs' in kw:
             a.set_target_attr(kw['attrs'])
         if 'memberof' in kw:
-            entry_attrs = api.Command['group_show'](kw['memberof'])['result']
-            a.set_target_filter('memberOf=%s' % entry_attrs['dn'])
+            groupdn = _group_from_memberof(kw['memberof'])
+            a.set_target_filter('memberOf=%s' % groupdn)
         if 'filter' in kw:
             # Test the filter by performing a simple search on it. The
             # filter is considered valid if either it returns some entries
@@ -298,7 +312,7 @@ def _aci_to_kw(ldap, a, test=False):
         kw['attrs'] = tuple(kw['attrs'])
     if 'targetfilter' in a.target:
         target = a.target['targetfilter']['expression']
-        if target.startswith('memberOf'):
+        if target.startswith('(memberOf') or target.startswith('memberOf'):
             kw['memberof'] = unicode(target)
         else:
             kw['filter'] = unicode(target)
@@ -707,10 +721,7 @@ class aci_find(crud.Search):
 
         if 'memberof' in kw:
             try:
-                result = self.api.Command['group_show'](
-                    kw['memberof']
-                )['result']
-                dn = result['dn']
+                dn = _group_from_memberof(kw['memberof'])
             except errors.NotFound:
                 pass
             else:
diff --git a/ipalib/plugins/delegation.py b/ipalib/plugins/delegation.py
index 6173b4b4..bd6f6571 100644
--- a/ipalib/plugins/delegation.py
+++ b/ipalib/plugins/delegation.py
@@ -57,7 +57,7 @@ def convert_delegation(ldap, aci):
     memberOf is in filter but we want to pull out the group for easier
     displaying.
     """
-    filter = aci['filter']
+    filter = aci['memberof']
     st = filter.find('memberOf=')
     if st == -1:
         raise errors.NotFound(reason=_('Delegation \'%(permission)s\' not found') % dict(permission=aci['aciname']))
@@ -69,9 +69,8 @@ def convert_delegation(ldap, aci):
         # Uh oh, the group we're granting access to has an error
         msg = _('Error retrieving member group %(group)s: %(error)s') % (membergroup, str(e))
         raise errors.NonFatalError(reason=msg)
-    aci['membergroup'] = entry_attrs['cn']
+    aci['memberof'] = entry_attrs['cn'][0]
 
-    del aci['filter']
     del aci['aciprefix']     # do not include prefix in result
 
     return aci
@@ -85,7 +84,7 @@ def is_delegation(ldap, aciname):
     membergroup.
     """
     result = api.Command['aci_show'](aciname, aciprefix=ACI_PREFIX)['result']
-    if 'filter' in result:
+    if 'memberof' in result:
         result = convert_delegation(ldap, result)
     else:
         raise errors.NotFound(reason=_('Delegation \'%(permission)s\' not found') % dict(permission=aciname))
@@ -162,7 +161,7 @@ class delegation_add(crud.Create):
             kw['permissions'] = (u'write',)
         kw['aciprefix'] = ACI_PREFIX
         result = api.Command['aci_add'](aciname, **kw)['result']
-        if 'filter' in result:
+        if 'memberof' in result:
             result = convert_delegation(ldap, result)
 
         return dict(
@@ -206,7 +205,7 @@ class delegation_mod(crud.Update):
         is_delegation(ldap, aciname)
         kw['aciprefix'] = ACI_PREFIX
         result = api.Command['aci_mod'](aciname, **kw)['result']
-        if 'filter' in result:
+        if 'memberof' in result:
             result = convert_delegation(ldap, result)
         return dict(
             result=result,
@@ -232,7 +231,7 @@ class delegation_find(crud.Search):
         results = []
         for aci in acis:
             try:
-                if 'filter' in aci:
+                if 'memberof' in aci:
                     aci = convert_delegation(ldap, aci)
                     results.append(aci)
             except errors.NotFound:
diff --git a/tests/test_xmlrpc/test_delegation_plugin.py b/tests/test_xmlrpc/test_delegation_plugin.py
index 2b5f6f60..2131c5ad 100644
--- a/tests/test_xmlrpc/test_delegation_plugin.py
+++ b/tests/test_xmlrpc/test_delegation_plugin.py
@@ -89,7 +89,7 @@ class test_delegation(Declarative):
                     permissions=[u'write'],
                     aciname=delegation1,
                     group=u'editors',
-                    membergroup=[member1],
+                    memberof=member1,
                 ),
             ),
         ),
@@ -120,7 +120,7 @@ class test_delegation(Declarative):
                     'permissions': [u'write'],
                     'aciname': delegation1,
                     'group': u'editors',
-                    'membergroup': [member1],
+                    'memberof': member1,
                 },
             ),
         ),
@@ -139,7 +139,7 @@ class test_delegation(Declarative):
                     'permissions': [u'write'],
                     'aciname': delegation1,
                     'group': u'editors',
-                    'membergroup': [member1],
+                    'memberof': member1,
                     },
                 ],
             ),
@@ -159,7 +159,7 @@ class test_delegation(Declarative):
                     permissions=[u'read'],
                     aciname=delegation1,
                     group=u'editors',
-                    membergroup=[member1],
+                    memberof=member1,
                 ),
             ),
         ),
@@ -176,7 +176,7 @@ class test_delegation(Declarative):
                     'permissions': [u'read'],
                     'aciname': delegation1,
                     'group': u'editors',
-                    'membergroup': [member1],
+                    'memberof': member1,
                 },
             ),
         ),