Memberof attribute control and update

Checking of parameters used by _make_aci funcion was rewritten.
Additional attributes of ACI(type, attribute, memberof, targetgroup,
subtree, filter) could be unset.

Permission plugin now allows to unset memberof value.
https://fedorahosted.org/freeipa/ticket/2255

Added checking of existence of groups that are specified in permission
and delegation module.

https://fedorahosted.org/freeipa/ticket/2286
https://fedorahosted.org/freeipa/ticket/2305

3 files changed, 90 insertions, 15 deletions

diff --git a/ipalib/plugins/aci.py b/ipalib/plugins/aci.py
index e87ac9bf..f0b81f48 100644
--- a/ipalib/plugins/aci.py
+++ b/ipalib/plugins/aci.py
@@ -208,22 +208,24 @@ def _make_aci(ldap, current, aciname, kw):
     Given a name and a set of keywords construct an ACI.
     """
     # Do some quick and dirty validation.
-    t1 = 'type' in kw
-    t2 = 'filter' in kw
-    t3 = 'subtree' in kw
-    t4 = 'targetgroup' in kw
-    t5 = 'attrs' in kw
-    t6 = 'memberof' in kw
-    if t1 + t2 + t3 + t4 > 1:
+    checked_args=['type','filter','subtree','targetgroup','attrs','memberof']
+    valid={}
+    for arg in checked_args:
+        if arg in kw:
+            valid[arg]=kw[arg] is not None
+        else:
+            valid[arg]=False
+
+    if valid['type'] + valid['filter'] + valid['subtree'] + valid['targetgroup'] > 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:
+    if sum(valid.itervalues()) == 0:
         raise errors.ValidationError(name='target', error=_('at least one of: type, filter, subtree, targetgroup, attrs or memberof are required'))
 
-    if t2 + t6 > 1:
+    if valid['filter'] + valid['memberof'] > 1:
         raise errors.ValidationError(name='target', error=_('filter and memberof are mutually exclusive'))
 
     group = 'group' in kw
@@ -262,12 +264,16 @@ def _make_aci(ldap, current, aciname, kw):
         else:
             dn = entry_attrs['dn']
             a.set_bindrule('groupdn = "ldap:///%s"' % dn)
-        if 'attrs' in kw:
+        if valid['attrs']:
             a.set_target_attr(kw['attrs'])
-        if 'memberof' in kw:
+        if valid['memberof']:
+            try:
+                api.Command['group_show'](kw['memberof'])
+            except errors.NotFound:
+                api.Object['group'].handle_not_found(kw['memberof'])
             groupdn = _group_from_memberof(kw['memberof'])
             a.set_target_filter('memberOf=%s' % groupdn)
-        if 'filter' in kw:
+        if valid['filter']:
             # Test the filter by performing a simple search on it. The
             # filter is considered valid if either it returns some entries
             # or it returns no entries, otherwise we let whatever exception
@@ -279,15 +285,15 @@ def _make_aci(ldap, current, aciname, kw):
             except errors.NotFound:
                 pass
             a.set_target_filter(kw['filter'])
-        if 'type' in kw:
+        if valid['type']:
             target = _type_map[kw['type']]
             a.set_target(target)
-        if 'targetgroup' in kw:
+        if valid['targetgroup']:
             # Purposely no try here so we'll raise a NotFound
             entry_attrs = api.Command['group_show'](kw['targetgroup'])['result']
             target = 'ldap:///%s' % entry_attrs['dn']
             a.set_target(target)
-        if 'subtree' in kw:
+        if valid['subtree']:
             # See if the subtree is a full URI
             target = kw['subtree']
             if not target.startswith('ldap:///'):
diff --git a/tests/test_xmlrpc/test_delegation_plugin.py b/tests/test_xmlrpc/test_delegation_plugin.py
index 1a9c3674..db5f7186 100644
--- a/tests/test_xmlrpc/test_delegation_plugin.py
+++ b/tests/test_xmlrpc/test_delegation_plugin.py
@@ -68,6 +68,18 @@ class test_delegation(Declarative):
             ),
         ),
 
+        dict(
+            desc='Try to create %r for non-existing member group' % delegation1,
+            command=(
+                'delegation_add', [delegation1], dict(
+                     attrs=u'street,c,l,st,postalCode',
+                     permissions=u'write',
+                     group=u'editors',
+                     memberof=u'nonexisting',
+                ),
+            ),
+            expected=errors.NotFound(reason='group not found'),
+        ),
 
         # Note that we add postalCode but expect postalcode. This tests
         # the attrs normalizer.
diff --git a/tests/test_xmlrpc/test_permission_plugin.py b/tests/test_xmlrpc/test_permission_plugin.py
index 50d36819..e8e6bebc 100644
--- a/tests/test_xmlrpc/test_permission_plugin.py
+++ b/tests/test_xmlrpc/test_permission_plugin.py
@@ -500,6 +500,16 @@ class test_permission(Declarative):
             )
         ),
 
+        dict(
+            desc='Try to create permission %r with non-existing memberof' % permission1,
+            command=(
+                'permission_add', [permission1], dict(
+                     memberof=u'nonexisting',
+                     permissions=u'write',
+                )
+            ),
+            expected=errors.NotFound(reason='group not found'),
+        ),
 
         dict(
             desc='Create memberof permission %r' % permission1,
@@ -507,6 +517,7 @@ class test_permission(Declarative):
                 'permission_add', [permission1], dict(
                      memberof=u'editors',
                      permissions=u'write',
+                     type=u'user',
                 )
             ),
             expected=dict(
@@ -518,6 +529,52 @@ class test_permission(Declarative):
                     objectclass=objectclasses.permission,
                     memberof=u'editors',
                     permissions=[u'write'],
+                    type=u'user',
+                ),
+            ),
+        ),
+
+        dict(
+            desc='Try to update non-existent memberof of %r' % permission1,
+            command=('permission_mod', [permission1], dict(memberof=u'nonexisting')),
+            expected=errors.NotFound(reason='group not found'),
+        ),
+
+        dict(
+            desc='Update memberof permission %r' % permission1,
+            command=(
+                'permission_mod', [permission1], dict(
+                     memberof=u'admins',
+                )
+            ),
+            expected=dict(
+                value=permission1,
+                summary=u'Modified permission "%s"' % permission1,
+                result=dict(
+                    dn=lambda x: DN(x) == permission1_dn,
+                    cn=[permission1],
+                    memberof=u'admins',
+                    permissions=[u'write'],
+                    type=u'user',
+                ),
+            ),
+        ),
+
+        dict(
+            desc='Unset memberof of permission %r' % permission1,
+            command=(
+                'permission_mod', [permission1], dict(
+                     memberof=None,
+                )
+            ),
+            expected=dict(
+                summary=u'Modified permission "%s"' % permission1,
+                value=permission1,
+                result=dict(
+                    dn=lambda x: DN(x) == permission1_dn,
+                    cn=[permission1],
+                    permissions=[u'write'],
+                    type=u'user',
                 ),
             ),
         ),