On Wed, 2012-05-30 at 14:43 +0200, Ondrej Hamada wrote:
> On 05/30/2012 07:45 AM, Martin Kosek wrote: 
> > When permission-find post callback detected a --pkey-only option,
> > it just terminated. However, this way the results that could have
> > been added from aci_find matches were not included.
> > 
> > Fix the post callback to go through the entire matching process.
> > Also make sure that DNS permissions have a correct objectclass
> > (ipapermission), otherwise such objects are not matched by the
> > permission LDAP search.
> > 
> > https://fedorahosted.org/freeipa/ticket/2658
> > 
> > 
> > 
> > _______________________________________________
> > Freeipa-devel mailing list
> > Freeipa-devel@redhat.com
> > https://www.redhat.com/mailman/listinfo/freeipa-devel
> Patch needs rebase
> 
> It does not apply because of changes made to
> ipalib/plugins/permission.py (by Rob's patch #1018)
> 

Rebased version attached.

Martin
>From d06be5a311ffe6fbc531f5387022d1e2ad1e67b0 Mon Sep 17 00:00:00 2001
From: Martin Kosek <mko...@redhat.com>
Date: Thu, 31 May 2012 12:39:24 +0200
Subject: [PATCH] permission-find missed some results with --pkey-only option

When permission-find post callback detected a --pkey-only option,
it just terminated. However, this way the results that could have
been added from aci_find matches were not included.

Fix the post callback to go through the entire matching process.
Also make sure that DNS permissions have a correct objectclass
(ipapermission), otherwise such objects are not matched by the
permission LDAP search.

https://fedorahosted.org/freeipa/ticket/2658
---
 install/share/dns.ldif                      |    4 +++
 install/updates/40-dns.update               |    6 ++++
 ipalib/plugins/permission.py                |   38 +++++++++++++++-----------
 tests/test_xmlrpc/test_permission_plugin.py |   19 +++++++++++++
 4 files changed, 51 insertions(+), 16 deletions(-)

diff --git a/install/share/dns.ldif b/install/share/dns.ldif
index cd77fe22cafed438b3549b19d7b125ca466e66f8..81ba21009ea5583437022344a6e72f7b26419cd9 100644
--- a/install/share/dns.ldif
+++ b/install/share/dns.ldif
@@ -34,6 +34,7 @@ dn: cn=add dns entries,cn=permissions,cn=pbac,$SUFFIX
 changetype: add
 objectClass: groupofnames
 objectClass: top
+objectClass: ipapermission
 cn: add dns entries
 description: Add DNS entries
 member: cn=DNS Administrators,cn=privileges,cn=pbac,$SUFFIX
@@ -43,6 +44,7 @@ dn: cn=remove dns entries,cn=permissions,cn=pbac,$SUFFIX
 changetype: add
 objectClass: groupofnames
 objectClass: top
+objectClass: ipapermission
 cn: remove dns entries
 description: Remove DNS entries
 member: cn=DNS Administrators,cn=privileges,cn=pbac,$SUFFIX
@@ -52,6 +54,7 @@ dn: cn=update dns entries,cn=permissions,cn=pbac,$SUFFIX
 changetype: add
 objectClass: groupofnames
 objectClass: top
+objectClass: ipapermission
 cn: update dns entries
 description: Update DNS entries
 member: cn=DNS Administrators,cn=privileges,cn=pbac,$SUFFIX
@@ -72,6 +75,7 @@ dn: cn=Write DNS Configuration,cn=permissions,cn=pbac,$SUFFIX
 changetype: add
 objectClass: groupofnames
 objectClass: top
+objectClass: ipapermission
 cn: Write DNS Configuration
 description: Write DNS Configuration
 member: cn=DNS Administrators,cn=privileges,cn=pbac,$SUFFIX
diff --git a/install/updates/40-dns.update b/install/updates/40-dns.update
index 02af8e467c99f905232b785b3677f44020a69c40..3dacb248f06626431e4eef9a65394008a5c71acb 100644
--- a/install/updates/40-dns.update
+++ b/install/updates/40-dns.update
@@ -1,17 +1,23 @@
 # Add missing member values to attach permissions to their respective
 # privileges and run a memberOf task.
 dn: cn=add dns entries,cn=permissions,cn=pbac,$SUFFIX
+addifexist:objectclass: ipapermission
 addifexist:member: 'cn=DNS Administrators,cn=privileges,cn=pbac,$SUFFIX'
 addifexist:member: 'cn=DNS Servers,cn=privileges,cn=pbac,$SUFFIX'
 
 dn: cn=remove dns entries,cn=permissions,cn=pbac,$SUFFIX
+addifexist:objectclass: ipapermission
 addifexist:member: 'cn=DNS Administrators,cn=privileges,cn=pbac,$SUFFIX'
 addifexist:member: 'cn=DNS Servers,cn=privileges,cn=pbac,$SUFFIX'
 
 dn: cn=update dns entries,cn=permissions,cn=pbac,$SUFFIX
+addifexist:objectclass: ipapermission
 addifexist:member: 'cn=DNS Administrators,cn=privileges,cn=pbac,$SUFFIX'
 addifexist:member: 'cn=DNS Servers,cn=privileges,cn=pbac,$SUFFIX'
 
+dn: cn=Write DNS Configuration,cn=permissions,cn=pbac,$SUFFIX
+addifexist:objectclass: ipapermission
+
 dn: cn=Update PBAC memberOf $TIME, cn=memberof task, cn=tasks, cn=config
 add: objectClass: top
 add: objectClass: extensibleObject
diff --git a/ipalib/plugins/permission.py b/ipalib/plugins/permission.py
index a484ff640820c2ba93d21b8dbfcbd2c66698e513..cfa2e5f9eb790995ff0a4bfd417904a4626c6b0c 100644
--- a/ipalib/plugins/permission.py
+++ b/ipalib/plugins/permission.py
@@ -350,22 +350,22 @@ class permission_find(LDAPSearch):
     has_output_params = LDAPSearch.has_output_params + output_params
 
     def post_callback(self, ldap, entries, truncated, *args, **options):
-        if options.pop('pkey_only', False):
-            return truncated
-        for entry in entries:
-            (dn, attrs) = entry
-            try:
-                aci = self.api.Command.aci_show(attrs['cn'][0], aciprefix=ACI_PREFIX, **options)['result']
+        pkey_only = options.pop('pkey_only', False)
+        if not pkey_only:
+            for entry in entries:
+                (dn, attrs) = entry
+                try:
+                    aci = self.api.Command.aci_show(attrs['cn'][0], aciprefix=ACI_PREFIX, **options)['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['cn'][0])
-        if truncated:
-            # size/time limit met, no need to search acis
-            return truncated
+                    # 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['cn'][0])
+            if truncated:
+                # size/time limit met, no need to search acis
+                return truncated
 
         if 'sizelimit' in options:
             max_entries = options['sizelimit']
@@ -406,9 +406,15 @@ class permission_find(LDAPSearch):
                     permission = self.api.Command.permission_show(aci['permission'], **options)['result']
                     dn = permission['dn']
                     del permission['dn']
+                    if pkey_only:
+                        new_entry = (dn, {self.obj.primary_key.name: \
+                                          permission[self.obj.primary_key.name]})
+                    else:
+                        new_entry = (dn, permission)
+
                     if (dn, permission) not in entries:
                        if len(entries) < max_entries:
-                           entries.append((dn, permission))
+                           entries.append(new_entry)
                        else:
                            truncated = True
                            break
diff --git a/tests/test_xmlrpc/test_permission_plugin.py b/tests/test_xmlrpc/test_permission_plugin.py
index d8ff14903d582a7c880d348ae28f0c0061437542..6613c9bbac985dd5732e14413cbe46135a789d55 100644
--- a/tests/test_xmlrpc/test_permission_plugin.py
+++ b/tests/test_xmlrpc/test_permission_plugin.py
@@ -368,6 +368,25 @@ class test_permission(Declarative):
 
 
         dict(
+            desc='Search by ACI attribute with --pkey-only',
+            command=('permission_find', [], {'pkey_only': True,
+                                             'attrs': [u'krbminpwdlife']}),
+            expected=dict(
+                count=1,
+                truncated=False,
+                summary=u'1 permission matched',
+                result=[
+                    {
+                        'dn': lambda x: DN(x) == DN(('cn','Modify Group Password Policy'),
+                                                     api.env.container_permission,api.env.basedn),
+                        'cn': [u'Modify Group Password Policy'],
+                    },
+                ],
+            ),
+        ),
+
+
+        dict(
             desc='Search for %r' % privilege1,
             command=('privilege_find', [privilege1], {}),
             expected=dict(
-- 
1.7.7.6

_______________________________________________
Freeipa-devel mailing list
Freeipa-devel@redhat.com
https://www.redhat.com/mailman/listinfo/freeipa-devel

Reply via email to