[Freeipa-devel] [PATCH] Fixed permission lookup

[Martin Kosek]

On Fri, 2011-01-28 at 13:01 +0100, Jan Zelený wrote:
> Martin Kosek <mkosek at 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 <rcritten at redhat.com> wrote:
> > > > > Jan Zelený wrote:
> > > > > > Martin Kosek<mkosek at 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