permission plugin: Do not change extra target filters by "views"

Previously, setting/deleting the "--type" virtual attribute removed
all (objectclass=...) target filters.
Change so that only the filter associated with --type is removed.

The same change applies to --memberof: only filters associated
with the option are removed when --memberof is (un-)set.

Follow-up to https://fedorahosted.org/freeipa/ticket/4216

Reviewed-By: Martin Kosek <mkosek@redhat.com>

ipalib/plugins/permission.py

@@ -689,10 +689,10 @@ class permission(baseldap.LDAPObject):
             If true, a dictionary of operations on ipapermtargetfilter is
             returned.
             These operations must be performed after the existing entry
-            is retreived.
+            is retrieved.
             The dict has the following keys:
-                - remove: list of regular expression objects; values that match
+                - remove: list of regular expression objects;
-                    any of them sould be removed
+                    implicit values that match any of them should be removed
                 - add: list of values to be added, after any removals
         :merge_targetfilter:
             If true, the extratargetfilter is copied into ipapermtargetfilter.
@@ -1042,10 +1042,13 @@ class permission_mod(baseldap.LDAPUpdate):
                 list(filter_attr_info['implicit_targetfilters']))
 
         filter_ops = context.filter_ops
+        old_filter_attr_info = self.obj._get_filter_attr_info(old_entry)
+        old_implicit_filters = old_filter_attr_info['implicit_targetfilters']
         removes = filter_ops.get('remove', [])
         new_filters = set(
             filt for filt in (entry.get('ipapermtargetfilter') or [])
-            if not any(rem.match(filt) for rem in removes))
+            if filt not in old_implicit_filters or
+                not any(rem.match(filt) for rem in removes))
         new_filters.update(filter_ops.get('add', []))
         new_filters.update(options.get('ipapermtargetfilter') or [])
         entry['ipapermtargetfilter'] = list(new_filters)

ipatests/test_xmlrpc/test_permission_plugin.py

@@ -2424,6 +2424,171 @@ class test_permission_targetfilter(Declarative):
         )
     ] + [
 
+        dict(
+            desc='Set extra objectclass filter on %r' % permission1,
+            command=(
+                'permission_mod', [permission1], dict(
+                    extratargetfilter=[u'(cn=*)', u'(objectclass=top)'],
+                    all=True,
+                )
+            ),
+            expected=dict(
+                value=permission1,
+                summary=u'Modified permission "%s"' % permission1,
+                result=dict(
+                    dn=permission1_dn,
+                    cn=[permission1],
+                    objectclass=objectclasses.permission,
+                    type=[u'user'],
+                    ipapermright=[u'write'],
+                    attrs=[u'sn'],
+                    ipapermincludedattr=[u'sn'],
+                    ipapermbindruletype=[u'permission'],
+                    ipapermissiontype=[u'SYSTEM', u'V2'],
+                    ipapermlocation=[users_dn],
+                    memberof=[u'admins'],
+                    extratargetfilter=[u'(cn=*)', u'(objectclass=top)'],
+                    ipapermtargetfilter=[
+                        u'(cn=*)',
+                        u'(memberOf=%s)' % DN(('cn', 'admins'), groups_dn),
+                        u'(objectclass=posixaccount)',
+                        u'(objectclass=top)'],
+                ),
+            ),
+        ),
+
+        verify_permission_aci(
+            permission1, users_dn,
+            '(targetattr = "sn")' +
+            '(targetfilter = "(&' +
+                '(cn=*)' +
+                '(memberOf=%s)' % DN('cn=admins', groups_dn) +
+                '(objectclass=posixaccount)' +
+                '(objectclass=top)' +
+            ')")' +
+            '(version 3.0;acl "permission:%s";' % permission1 +
+            'allow (write) groupdn = "ldap:///%s";)' % permission1_dn
+        ),
+
+        dict(
+            desc='Unset type on %r to verify extra objectclass filter stays' % permission1,
+            command=(
+                'permission_mod', [permission1], dict(
+                    type=None,
+                    all=True,
+                )
+            ),
+            expected=dict(
+                value=permission1,
+                summary=u'Modified permission "%s"' % permission1,
+                result=dict(
+                    dn=permission1_dn,
+                    cn=[permission1],
+                    objectclass=objectclasses.permission,
+                    ipapermright=[u'write'],
+                    attrs=[u'sn'],
+                    ipapermincludedattr=[u'sn'],
+                    ipapermbindruletype=[u'permission'],
+                    ipapermissiontype=[u'SYSTEM', u'V2'],
+                    ipapermlocation=[api.env.basedn],
+                    memberof=[u'admins'],
+                    extratargetfilter=[u'(cn=*)', u'(objectclass=top)'],
+                    ipapermtargetfilter=[
+                        u'(cn=*)',
+                        u'(memberOf=%s)' % DN(('cn', 'admins'), groups_dn),
+                        u'(objectclass=top)'],
+                ),
+            ),
+        ),
+
+        verify_permission_aci(
+            permission1, api.env.basedn,
+            '(targetattr = "sn")' +
+            '(targetfilter = "(&' +
+                '(cn=*)' +
+                '(memberOf=%s)' % DN('cn=admins', groups_dn) +
+                '(objectclass=top)' +
+            ')")' +
+            '(version 3.0;acl "permission:%s";' % permission1 +
+            'allow (write) groupdn = "ldap:///%s";)' % permission1_dn
+        ),
+
+        dict(
+            desc='Set wildcard memberof filter on %r' % permission1,
+            command=(
+                'permission_mod', [permission1], dict(
+                    extratargetfilter=u'(memberof=*)',
+                    all=True,
+                )
+            ),
+            expected=dict(
+                value=permission1,
+                summary=u'Modified permission "%s"' % permission1,
+                result=dict(
+                    dn=permission1_dn,
+                    cn=[permission1],
+                    objectclass=objectclasses.permission,
+                    ipapermright=[u'write'],
+                    attrs=[u'sn'],
+                    ipapermincludedattr=[u'sn'],
+                    ipapermbindruletype=[u'permission'],
+                    ipapermissiontype=[u'SYSTEM', u'V2'],
+                    ipapermlocation=[api.env.basedn],
+                    memberof=[u'admins'],
+                    extratargetfilter=[u'(memberof=*)'],
+                    ipapermtargetfilter=[
+                        u'(memberOf=%s)' % DN(('cn', 'admins'), groups_dn),
+                        u'(memberof=*)'],
+                ),
+            ),
+        ),
+
+        verify_permission_aci(
+            permission1, api.env.basedn,
+            '(targetattr = "sn")' +
+            '(targetfilter = "(&' +
+                '(memberOf=%s)' % DN('cn=admins', groups_dn) +
+                '(memberof=*)' +
+            ')")' +
+            '(version 3.0;acl "permission:%s";' % permission1 +
+            'allow (write) groupdn = "ldap:///%s";)' % permission1_dn
+        ),
+
+        dict(
+            desc='Remove --memberof on %r to verify wildcard is still there' % permission1,
+            command=(
+                'permission_mod', [permission1], dict(
+                    memberof=[],
+                    all=True,
+                )
+            ),
+            expected=dict(
+                value=permission1,
+                summary=u'Modified permission "%s"' % permission1,
+                result=dict(
+                    dn=permission1_dn,
+                    cn=[permission1],
+                    objectclass=objectclasses.permission,
+                    ipapermright=[u'write'],
+                    attrs=[u'sn'],
+                    ipapermincludedattr=[u'sn'],
+                    ipapermbindruletype=[u'permission'],
+                    ipapermissiontype=[u'SYSTEM', u'V2'],
+                    ipapermlocation=[api.env.basedn],
+                    extratargetfilter=[u'(memberof=*)'],
+                    ipapermtargetfilter=[u'(memberof=*)'],
+                ),
+            ),
+        ),
+
+        verify_permission_aci(
+            permission1, api.env.basedn,
+            '(targetattr = "sn")' +
+            '(targetfilter = "(memberof=*)")' +
+            '(version 3.0;acl "permission:%s";' % permission1 +
+            'allow (write) groupdn = "ldap:///%s";)' % permission1_dn
+        ),
+
     ]