Re: [Freeipa-devel] [PATCH] Fixed permission lookup
Jan Zelený wrote: Rob Crittendenrcrit...@redhat.com wrote: Jan Zelený wrote: Martin Kosekmko...@redhat.com wrote: On Fri, 2011-01-28 at 09:21 +0100, Martin Kosek wrote: On Thu, 2011-01-27 at 15:41 +0100, Jan Zelený wrote: Rob Crittendenrcrit...@redhat.com wrote: Jan Zelený wrote: Martin Kosekmko...@redhat.comwrote: On Thu, 2011-01-27 at 11:15 +0100, Jan Zelený wrote: Lookup based on --filter wasn't implemented at all. It did't show until now, because of bug sitting on top of it which was resulting in internal error. This patch fixes the bug and adds the filtering functionality. https://fedorahosted.org/freeipa/ticket/818 NACK Did you build this patch on current master? Because in your patch, you removed changes in permission-find from my previous patch 017 ACI plugin supports prefixes. After your patch, permission-find fails: $ ipa permission-find ipa: ERROR: 'aciprefix' is required Martin Sorry, I accidentaly mixed the code with a part of the older one. Sending corrected patch. Jan I think the more stuff in baseldap.py:LDAPSearch() was there because adding entries in a post_callback wasn't working. It only let you reduce the number or modify what was already there IIRC. From what I know, lists should allow you to expand them without any problems (not sure how is the concept called in Python, Pavel told me about it). Also I didn't encounter any problems with this approach (and the post callback actually adds some entries), that's why I changed it the way I did. Jan ACK I think the concept of adding new items to list 'entries' is right. Martin Second-thought-NACK After some thoughts about permissions and ACIs I think the ACI filtering should be moved to ACI plugin - aci_find command. So that it is available to other commands built over ACI plugin that would need searching by filter. A good place to move the filtering by 'filter' would be instead of the following comment in aci.py: # TODO: searching by: filter, subtree Martin Good catch. I'm sending another version of the patch in attachment. Jan This only does filter exact matches, is that adequate or should we return any filter that has the query as a substring? rob I thought about that as well. If you think it is more appropriate, I'll update the patch. But IMO this behavior is what users will expect. Jan Ok, I pushed this to master. Can you open a ticket to do substring searches? I think it might be handy to have at some point, not enough of a priority to hold the rest of this up. rob ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCH] Fixed permission lookup
Rob Crittenden rcrit...@redhat.com wrote: Jan Zelený wrote: Rob Crittendenrcrit...@redhat.com wrote: Jan Zelený wrote: Martin Kosekmko...@redhat.com wrote: On Fri, 2011-01-28 at 09:21 +0100, Martin Kosek wrote: On Thu, 2011-01-27 at 15:41 +0100, Jan Zelený wrote: Rob Crittendenrcrit...@redhat.com wrote: Jan Zelený wrote: Martin Kosekmko...@redhat.comwrote: On Thu, 2011-01-27 at 11:15 +0100, Jan Zelený wrote: Lookup based on --filter wasn't implemented at all. It did't show until now, because of bug sitting on top of it which was resulting in internal error. This patch fixes the bug and adds the filtering functionality. https://fedorahosted.org/freeipa/ticket/818 NACK Did you build this patch on current master? Because in your patch, you removed changes in permission-find from my previous patch 017 ACI plugin supports prefixes. After your patch, permission-find fails: $ ipa permission-find ipa: ERROR: 'aciprefix' is required Martin Sorry, I accidentaly mixed the code with a part of the older one. Sending corrected patch. Jan I think the more stuff in baseldap.py:LDAPSearch() was there because adding entries in a post_callback wasn't working. It only let you reduce the number or modify what was already there IIRC. From what I know, lists should allow you to expand them without any problems (not sure how is the concept called in Python, Pavel told me about it). Also I didn't encounter any problems with this approach (and the post callback actually adds some entries), that's why I changed it the way I did. Jan ACK I think the concept of adding new items to list 'entries' is right. Martin Second-thought-NACK After some thoughts about permissions and ACIs I think the ACI filtering should be moved to ACI plugin - aci_find command. So that it is available to other commands built over ACI plugin that would need searching by filter. A good place to move the filtering by 'filter' would be instead of the following comment in aci.py: # TODO: searching by: filter, subtree Martin Good catch. I'm sending another version of the patch in attachment. Jan This only does filter exact matches, is that adequate or should we return any filter that has the query as a substring? rob I thought about that as well. If you think it is more appropriate, I'll update the patch. But IMO this behavior is what users will expect. Jan Ok, I pushed this to master. Can you open a ticket to do substring searches? I think it might be handy to have at some point, not enough of a priority to hold the rest of this up. rob Sure, will do. As we discussed this with Jakub and Martin, this feature would be handy not only here, but elsewhere as well. Hence it might be useful to implement it in baseldap (if possible). Jan ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCH] Fixed permission lookup
Jan Zeleny wrote: Rob Crittendenrcrit...@redhat.com wrote: Jan Zelený wrote: Rob Crittendenrcrit...@redhat.com wrote: Jan Zelený wrote: Martin Kosekmko...@redhat.comwrote: On Fri, 2011-01-28 at 09:21 +0100, Martin Kosek wrote: On Thu, 2011-01-27 at 15:41 +0100, Jan Zelený wrote: Rob Crittendenrcrit...@redhat.comwrote: Jan Zelený wrote: Martin Kosekmko...@redhat.com wrote: On Thu, 2011-01-27 at 11:15 +0100, Jan Zelený wrote: Lookup based on --filter wasn't implemented at all. It did't show until now, because of bug sitting on top of it which was resulting in internal error. This patch fixes the bug and adds the filtering functionality. https://fedorahosted.org/freeipa/ticket/818 NACK Did you build this patch on current master? Because in your patch, you removed changes in permission-find from my previous patch 017 ACI plugin supports prefixes. After your patch, permission-find fails: $ ipa permission-find ipa: ERROR: 'aciprefix' is required Martin Sorry, I accidentaly mixed the code with a part of the older one. Sending corrected patch. Jan I think the more stuff in baseldap.py:LDAPSearch() was there because adding entries in a post_callback wasn't working. It only let you reduce the number or modify what was already there IIRC. From what I know, lists should allow you to expand them without any problems (not sure how is the concept called in Python, Pavel told me about it). Also I didn't encounter any problems with this approach (and the post callback actually adds some entries), that's why I changed it the way I did. Jan ACK I think the concept of adding new items to list 'entries' is right. Martin Second-thought-NACK After some thoughts about permissions and ACIs I think the ACI filtering should be moved to ACI plugin - aci_find command. So that it is available to other commands built over ACI plugin that would need searching by filter. A good place to move the filtering by 'filter' would be instead of the following comment in aci.py: # TODO: searching by: filter, subtree Martin Good catch. I'm sending another version of the patch in attachment. Jan This only does filter exact matches, is that adequate or should we return any filter that has the query as a substring? rob I thought about that as well. If you think it is more appropriate, I'll update the patch. But IMO this behavior is what users will expect. Jan Ok, I pushed this to master. Can you open a ticket to do substring searches? I think it might be handy to have at some point, not enough of a priority to hold the rest of this up. rob Sure, will do. As we discussed this with Jakub and Martin, this feature would be handy not only here, but elsewhere as well. Hence it might be useful to implement it in baseldap (if possible). For LDAP-based entries this already happens, see ldap2.make_filter(). The permissions plugin does a lot of stuff difference since we do the search manually as opposed to over LDAP. rob ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCH] Fixed permission lookup
Rob Crittenden rcrit...@redhat.com wrote: Jan Zelený wrote: Martin Kosekmko...@redhat.com wrote: On Fri, 2011-01-28 at 09:21 +0100, Martin Kosek wrote: On Thu, 2011-01-27 at 15:41 +0100, Jan Zelený wrote: Rob Crittendenrcrit...@redhat.com wrote: Jan Zelený wrote: Martin Kosekmko...@redhat.com wrote: On Thu, 2011-01-27 at 11:15 +0100, Jan Zelený wrote: Lookup based on --filter wasn't implemented at all. It did't show until now, because of bug sitting on top of it which was resulting in internal error. This patch fixes the bug and adds the filtering functionality. https://fedorahosted.org/freeipa/ticket/818 NACK Did you build this patch on current master? Because in your patch, you removed changes in permission-find from my previous patch 017 ACI plugin supports prefixes. After your patch, permission-find fails: $ ipa permission-find ipa: ERROR: 'aciprefix' is required Martin Sorry, I accidentaly mixed the code with a part of the older one. Sending corrected patch. Jan I think the more stuff in baseldap.py:LDAPSearch() was there because adding entries in a post_callback wasn't working. It only let you reduce the number or modify what was already there IIRC. From what I know, lists should allow you to expand them without any problems (not sure how is the concept called in Python, Pavel told me about it). Also I didn't encounter any problems with this approach (and the post callback actually adds some entries), that's why I changed it the way I did. Jan ACK I think the concept of adding new items to list 'entries' is right. Martin Second-thought-NACK After some thoughts about permissions and ACIs I think the ACI filtering should be moved to ACI plugin - aci_find command. So that it is available to other commands built over ACI plugin that would need searching by filter. A good place to move the filtering by 'filter' would be instead of the following comment in aci.py: # TODO: searching by: filter, subtree Martin Good catch. I'm sending another version of the patch in attachment. Jan This only does filter exact matches, is that adequate or should we return any filter that has the query as a substring? rob I thought about that as well. If you think it is more appropriate, I'll update the patch. But IMO this behavior is what users will expect. Jan ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCH] Fixed permission lookup
On Thu, 2011-01-27 at 15:41 +0100, Jan Zelený wrote: Rob Crittenden rcrit...@redhat.com wrote: Jan Zelený wrote: Martin Kosekmko...@redhat.com wrote: On Thu, 2011-01-27 at 11:15 +0100, Jan Zelený wrote: Lookup based on --filter wasn't implemented at all. It did't show until now, because of bug sitting on top of it which was resulting in internal error. This patch fixes the bug and adds the filtering functionality. https://fedorahosted.org/freeipa/ticket/818 NACK Did you build this patch on current master? Because in your patch, you removed changes in permission-find from my previous patch 017 ACI plugin supports prefixes. After your patch, permission-find fails: $ ipa permission-find ipa: ERROR: 'aciprefix' is required Martin Sorry, I accidentaly mixed the code with a part of the older one. Sending corrected patch. Jan I think the more stuff in baseldap.py:LDAPSearch() was there because adding entries in a post_callback wasn't working. It only let you reduce the number or modify what was already there IIRC. From what I know, lists should allow you to expand them without any problems (not sure how is the concept called in Python, Pavel told me about it). Also I didn't encounter any problems with this approach (and the post callback actually adds some entries), that's why I changed it the way I did. Jan ACK I think the concept of adding new items to list 'entries' is right. Martin ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCH] Fixed permission lookup
On Fri, 2011-01-28 at 09:21 +0100, Martin Kosek wrote: On Thu, 2011-01-27 at 15:41 +0100, Jan Zelený wrote: Rob Crittenden rcrit...@redhat.com wrote: Jan Zelený wrote: Martin Kosekmko...@redhat.com wrote: On Thu, 2011-01-27 at 11:15 +0100, Jan Zelený wrote: Lookup based on --filter wasn't implemented at all. It did't show until now, because of bug sitting on top of it which was resulting in internal error. This patch fixes the bug and adds the filtering functionality. https://fedorahosted.org/freeipa/ticket/818 NACK Did you build this patch on current master? Because in your patch, you removed changes in permission-find from my previous patch 017 ACI plugin supports prefixes. After your patch, permission-find fails: $ ipa permission-find ipa: ERROR: 'aciprefix' is required Martin Sorry, I accidentaly mixed the code with a part of the older one. Sending corrected patch. Jan I think the more stuff in baseldap.py:LDAPSearch() was there because adding entries in a post_callback wasn't working. It only let you reduce the number or modify what was already there IIRC. From what I know, lists should allow you to expand them without any problems (not sure how is the concept called in Python, Pavel told me about it). Also I didn't encounter any problems with this approach (and the post callback actually adds some entries), that's why I changed it the way I did. Jan ACK I think the concept of adding new items to list 'entries' is right. Martin Second-thought-NACK After some thoughts about permissions and ACIs I think the ACI filtering should be moved to ACI plugin - aci_find command. So that it is available to other commands built over ACI plugin that would need searching by filter. A good place to move the filtering by 'filter' would be instead of the following comment in aci.py: # TODO: searching by: filter, subtree Martin ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCH] Fixed permission lookup
Martin Kosek mko...@redhat.com wrote:
On Fri, 2011-01-28 at 09:21 +0100, Martin Kosek wrote:
On Thu, 2011-01-27 at 15:41 +0100, Jan Zelený wrote:
Rob Crittenden rcrit...@redhat.com wrote:
Jan Zelený wrote:
Martin Kosekmko...@redhat.com wrote:
On Thu, 2011-01-27 at 11:15 +0100, Jan Zelený wrote:
Lookup based on --filter wasn't implemented at all. It did't show
until now, because of bug sitting on top of it which was
resulting in internal error. This patch fixes the bug and adds
the filtering functionality.
https://fedorahosted.org/freeipa/ticket/818
NACK
Did you build this patch on current master? Because in your patch,
you removed changes in permission-find from my previous patch
017 ACI plugin supports prefixes. After your patch,
permission-find fails:
$ ipa permission-find
ipa: ERROR: 'aciprefix' is required
Martin
Sorry, I accidentaly mixed the code with a part of the older one.
Sending corrected patch.
Jan
I think the more stuff in baseldap.py:LDAPSearch() was there because
adding entries in a post_callback wasn't working. It only let you
reduce the number or modify what was already there IIRC.
From what I know, lists should allow you to expand them without any
problems
(not sure how is the concept called in Python, Pavel told me about it).
Also I didn't encounter any problems with this approach (and the post
callback actually adds some entries), that's why I changed it the way
I did.
Jan
ACK
I think the concept of adding new items to list 'entries' is right.
Martin
Second-thought-NACK
After some thoughts about permissions and ACIs I think the ACI filtering
should be moved to ACI plugin - aci_find command. So that it is
available to other commands built over ACI plugin that would need
searching by filter.
A good place to move the filtering by 'filter' would be instead of the
following comment in aci.py:
# TODO: searching by: filter, subtree
Martin
Good catch. I'm sending another version of the patch in attachment.
Jan
From 1c4af3408b15fc933370de95940300920cad0260 Mon Sep 17 00:00:00 2001
From: Jan Zeleny jzel...@redhat.com
Date: Thu, 27 Jan 2011 05:11:28 -0500
Subject: [PATCH] Fixed permission lookup
Lookup based on --filter wasn't implemented at all. It did't show until
now, because of bug sitting on top of it which was resulting in internal
error. This patch fixes the bug and adds the filtering functionality.
https://fedorahosted.org/freeipa/ticket/818
---
ipalib/plugins/aci.py| 11 ++-
ipalib/plugins/baseldap.py | 12 +---
ipalib/plugins/permission.py |5 +
3 files changed, 16 insertions(+), 12 deletions(-)
diff --git a/ipalib/plugins/aci.py b/ipalib/plugins/aci.py
index 648f5111fcfd23975fe317d6b89b6163508a15a3..4ddaf98ab8f4ca3cb9235021912afbd7987fed13 100644
--- a/ipalib/plugins/aci.py
+++ b/ipalib/plugins/aci.py
@@ -780,7 +780,16 @@ class aci_find(crud.Search):
except ValueError:
pass
-# TODO: searching by: filter, subtree
+if 'filter' in kw:
+if not kw['filter'].startswith('('):
+kw['filter'] = unicode('('+kw['filter']+')')
+for a in acis:
+if 'targetfilter' not in a.target or\
+not a.target['targetfilter']['expression'] or\
+a.target['targetfilter']['expression'] != kw['filter']:
+results.remove(a)
+
+# TODO: searching by: subtree
acis = []
for result in results:
diff --git a/ipalib/plugins/baseldap.py b/ipalib/plugins/baseldap.py
index b20d96012e0dc7f91209a3623d8ad90cd023e006..d25deb5270ee2b79c2229e9265fa11c3ccca8b17 100644
--- a/ipalib/plugins/baseldap.py
+++ b/ipalib/plugins/baseldap.py
@@ -1372,11 +1372,9 @@ class LDAPSearch(CallbackInterface, crud.Search):
for callback in self.POST_CALLBACKS:
if hasattr(callback, 'im_self'):
-more = callback(ldap, entries, truncated, *args, **options)
+callback(ldap, entries, truncated, *args, **options)
else:
-more = callback(self, ldap, entries, truncated, *args, **options)
-if more:
-entries = entries + more
+callback(self, ldap, entries, truncated, *args, **options)
if not options.get('raw', False):
for e in entries:
@@ -1392,11 +1390,11 @@ class LDAPSearch(CallbackInterface, crud.Search):
truncated=truncated,
)
-def pre_callback(self, ldap, filter, attrs_list, base_dn, scope, *args, **options):
-return (filter, base_dn, scope)
+def pre_callback(self, ldap, filters, attrs_list, base_dn, scope, *args, **options):
+return (filters, base_dn, scope)
def post_callback(self, ldap,
Re: [Freeipa-devel] [PATCH] Fixed permission lookup
On Fri, 2011-01-28 at 13:01 +0100, Jan Zelený wrote: Martin Kosek mko...@redhat.com wrote: On Fri, 2011-01-28 at 09:21 +0100, Martin Kosek wrote: On Thu, 2011-01-27 at 15:41 +0100, Jan Zelený wrote: Rob Crittenden rcrit...@redhat.com wrote: Jan Zelený wrote: Martin Kosekmko...@redhat.com wrote: On Thu, 2011-01-27 at 11:15 +0100, Jan Zelený wrote: Lookup based on --filter wasn't implemented at all. It did't show until now, because of bug sitting on top of it which was resulting in internal error. This patch fixes the bug and adds the filtering functionality. https://fedorahosted.org/freeipa/ticket/818 NACK Did you build this patch on current master? Because in your patch, you removed changes in permission-find from my previous patch 017 ACI plugin supports prefixes. After your patch, permission-find fails: $ ipa permission-find ipa: ERROR: 'aciprefix' is required Martin Sorry, I accidentaly mixed the code with a part of the older one. Sending corrected patch. Jan I think the more stuff in baseldap.py:LDAPSearch() was there because adding entries in a post_callback wasn't working. It only let you reduce the number or modify what was already there IIRC. From what I know, lists should allow you to expand them without any problems (not sure how is the concept called in Python, Pavel told me about it). Also I didn't encounter any problems with this approach (and the post callback actually adds some entries), that's why I changed it the way I did. Jan ACK I think the concept of adding new items to list 'entries' is right. Martin Second-thought-NACK After some thoughts about permissions and ACIs I think the ACI filtering should be moved to ACI plugin - aci_find command. So that it is available to other commands built over ACI plugin that would need searching by filter. A good place to move the filtering by 'filter' would be instead of the following comment in aci.py: # TODO: searching by: filter, subtree Martin Good catch. I'm sending another version of the patch in attachment. Jan ACK Martin ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
[Freeipa-devel] [PATCH] Fixed permission lookup
Lookup based on --filter wasn't implemented at all. It did't show until
now, because of bug sitting on top of it which was resulting in internal
error. This patch fixes the bug and adds the filtering functionality.
https://fedorahosted.org/freeipa/ticket/818
--
Thank you
Jan Zeleny
Red Hat Software Engineer
Brno, Czech Republic
From 4e5cea38d40d75dea0042701a28d235ec60c878b Mon Sep 17 00:00:00 2001
From: Jan Zeleny jzel...@redhat.com
Date: Thu, 27 Jan 2011 05:11:28 -0500
Subject: [PATCH] Fixed permission lookup
Lookup based on --filter wasn't implemented at all. It did't show until
now, because of bug sitting on top of it which was resulting in internal
error. This patch fixes the bug and adds the filtering functionality.
https://fedorahosted.org/freeipa/ticket/818
---
ipalib/plugins/baseldap.py | 10 --
ipalib/plugins/permission.py | 22 ++
2 files changed, 14 insertions(+), 18 deletions(-)
diff --git a/ipalib/plugins/baseldap.py b/ipalib/plugins/baseldap.py
index b20d96012e0dc7f91209a3623d8ad90cd023e006..6d58eb97742a29a584d97913fc26c4ccf5d4c349 100644
--- a/ipalib/plugins/baseldap.py
+++ b/ipalib/plugins/baseldap.py
@@ -1372,11 +1372,9 @@ class LDAPSearch(CallbackInterface, crud.Search):
for callback in self.POST_CALLBACKS:
if hasattr(callback, 'im_self'):
-more = callback(ldap, entries, truncated, *args, **options)
+callback(ldap, entries, truncated, *args, **options)
else:
-more = callback(self, ldap, entries, truncated, *args, **options)
-if more:
-entries = entries + more
+callback(self, ldap, entries, truncated, *args, **options)
if not options.get('raw', False):
for e in entries:
@@ -1392,8 +1390,8 @@ class LDAPSearch(CallbackInterface, crud.Search):
truncated=truncated,
)
-def pre_callback(self, ldap, filter, attrs_list, base_dn, scope, *args, **options):
-return (filter, base_dn, scope)
+def pre_callback(self, ldap, filters, attrs_list, base_dn, scope, *args, **options):
+return (filters, base_dn, scope)
def post_callback(self, ldap, entries, truncated, *args, **options):
return []
diff --git a/ipalib/plugins/permission.py b/ipalib/plugins/permission.py
index 0c2855ff5c181a56455bb9b180b6f22472ce8fa4..1cbdd449763d8fafee0c5bd94669f5a7bb11f3bb 100644
--- a/ipalib/plugins/permission.py
+++ b/ipalib/plugins/permission.py
@@ -319,33 +319,33 @@ class permission_find(LDAPSearch):
)
def post_callback(self, ldap, entries, truncated, *args, **options):
-newentries = []
for entry in entries:
(dn, attrs) = entry
try:
-aci = self.api.Command.aci_show(attrs['cn'][0], aciprefix=ACI_PREFIX)['result']
-
-# copy information from respective ACI to permission entry
+aci = self.api.Command.aci_show(attrs['description'][0])['result']
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])
+self.debug('ACI not found for %s' % attrs['description'][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']
-
+if 'filter' in options and not options['filter'].startswith('('):
+options['filter'] = unicode('('+options['filter']+')')
for aci in results:
+if 'filter' in options:
+if 'filter' not in aci or not aci['filter'] or\
+aci['filter'] != options['filter']:
+continue
found = False
if 'permission' in aci:
for entry in entries:
(dn, attrs) = entry
-if aci['permission'] == attrs['cn'][0]:
+if aci['permission'] == attrs['cn']:
found = True
break
if not found:
@@ -357,9 +357,7 @@ class permission_find(LDAPSearch):
dn = attrs['dn']
del attrs['dn']
if (dn, attrs) not in entries:
-newentries.append((dn, attrs))
-
-return newentries
+entries.append((dn, attrs))
api.register(permission_find)
--
1.7.3.4
___
Freeipa-devel mailing list
Freeipa-devel@redhat.com
https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCH] Fixed permission lookup
On Thu, 2011-01-27 at 11:15 +0100, Jan Zelený wrote: Lookup based on --filter wasn't implemented at all. It did't show until now, because of bug sitting on top of it which was resulting in internal error. This patch fixes the bug and adds the filtering functionality. https://fedorahosted.org/freeipa/ticket/818 NACK Did you build this patch on current master? Because in your patch, you removed changes in permission-find from my previous patch 017 ACI plugin supports prefixes. After your patch, permission-find fails: $ ipa permission-find ipa: ERROR: 'aciprefix' is required Martin ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCH] Fixed permission lookup
Martin Kosek mko...@redhat.com wrote:
On Thu, 2011-01-27 at 11:15 +0100, Jan Zelený wrote:
Lookup based on --filter wasn't implemented at all. It did't show until
now, because of bug sitting on top of it which was resulting in internal
error. This patch fixes the bug and adds the filtering functionality.
https://fedorahosted.org/freeipa/ticket/818
NACK
Did you build this patch on current master? Because in your patch, you
removed changes in permission-find from my previous patch 017 ACI
plugin supports prefixes. After your patch, permission-find fails:
$ ipa permission-find
ipa: ERROR: 'aciprefix' is required
Martin
Sorry, I accidentaly mixed the code with a part of the older one. Sending
corrected patch.
Jan
From 717e995250193667cc98b5f16d347dbbeff2802c Mon Sep 17 00:00:00 2001
From: Jan Zeleny jzel...@redhat.com
Date: Thu, 27 Jan 2011 05:11:28 -0500
Subject: [PATCH] Fixed permission lookup
Lookup based on --filter wasn't implemented at all. It did't show until
now, because of bug sitting on top of it which was resulting in internal
error. This patch fixes the bug and adds the filtering functionality.
https://fedorahosted.org/freeipa/ticket/818
---
ipalib/plugins/baseldap.py | 12 +---
ipalib/plugins/permission.py | 11 +++
2 files changed, 12 insertions(+), 11 deletions(-)
diff --git a/ipalib/plugins/baseldap.py b/ipalib/plugins/baseldap.py
index b20d96012e0dc7f91209a3623d8ad90cd023e006..d25deb5270ee2b79c2229e9265fa11c3ccca8b17 100644
--- a/ipalib/plugins/baseldap.py
+++ b/ipalib/plugins/baseldap.py
@@ -1372,11 +1372,9 @@ class LDAPSearch(CallbackInterface, crud.Search):
for callback in self.POST_CALLBACKS:
if hasattr(callback, 'im_self'):
-more = callback(ldap, entries, truncated, *args, **options)
+callback(ldap, entries, truncated, *args, **options)
else:
-more = callback(self, ldap, entries, truncated, *args, **options)
-if more:
-entries = entries + more
+callback(self, ldap, entries, truncated, *args, **options)
if not options.get('raw', False):
for e in entries:
@@ -1392,11 +1390,11 @@ class LDAPSearch(CallbackInterface, crud.Search):
truncated=truncated,
)
-def pre_callback(self, ldap, filter, attrs_list, base_dn, scope, *args, **options):
-return (filter, base_dn, scope)
+def pre_callback(self, ldap, filters, attrs_list, base_dn, scope, *args, **options):
+return (filters, base_dn, scope)
def post_callback(self, ldap, entries, truncated, *args, **options):
-return []
+pass
def exc_callback(self, args, options, exc, call_func, *call_args, **call_kwargs):
raise exc
diff --git a/ipalib/plugins/permission.py b/ipalib/plugins/permission.py
index 0c2855ff5c181a56455bb9b180b6f22472ce8fa4..212a0469b55d19d76030f6384458943d5b8a19a6 100644
--- a/ipalib/plugins/permission.py
+++ b/ipalib/plugins/permission.py
@@ -319,7 +319,6 @@ class permission_find(LDAPSearch):
)
def post_callback(self, ldap, entries, truncated, *args, **options):
-newentries = []
for entry in entries:
(dn, attrs) = entry
try:
@@ -340,7 +339,13 @@ class permission_find(LDAPSearch):
truncated = truncated or aciresults['truncated']
results = aciresults['result']
+if 'filter' in options and not options['filter'].startswith('('):
+options['filter'] = unicode('('+options['filter']+')')
for aci in results:
+if 'filter' in options:
+if 'filter' not in aci or not aci['filter'] or\
+aci['filter'] != options['filter']:
+continue
found = False
if 'permission' in aci:
for entry in entries:
@@ -357,9 +362,7 @@ class permission_find(LDAPSearch):
dn = attrs['dn']
del attrs['dn']
if (dn, attrs) not in entries:
-newentries.append((dn, attrs))
-
-return newentries
+entries.append((dn, attrs))
api.register(permission_find)
--
1.7.3.4
___
Freeipa-devel mailing list
Freeipa-devel@redhat.com
https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCH] Fixed permission lookup
Rob Crittenden rcrit...@redhat.com wrote: Jan Zelený wrote: Martin Kosekmko...@redhat.com wrote: On Thu, 2011-01-27 at 11:15 +0100, Jan Zelený wrote: Lookup based on --filter wasn't implemented at all. It did't show until now, because of bug sitting on top of it which was resulting in internal error. This patch fixes the bug and adds the filtering functionality. https://fedorahosted.org/freeipa/ticket/818 NACK Did you build this patch on current master? Because in your patch, you removed changes in permission-find from my previous patch 017 ACI plugin supports prefixes. After your patch, permission-find fails: $ ipa permission-find ipa: ERROR: 'aciprefix' is required Martin Sorry, I accidentaly mixed the code with a part of the older one. Sending corrected patch. Jan I think the more stuff in baseldap.py:LDAPSearch() was there because adding entries in a post_callback wasn't working. It only let you reduce the number or modify what was already there IIRC. From what I know, lists should allow you to expand them without any problems (not sure how is the concept called in Python, Pavel told me about it). Also I didn't encounter any problems with this approach (and the post callback actually adds some entries), that's why I changed it the way I did. Jan ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
