[Freeipa-devel] [PATCH] Fixed permission lookup

[Rob Crittenden]

Jan Zeleny wrote:
> Rob Crittenden<rcritten at redhat.com>  wrote:
>> Jan Zelený wrote:
>>> Rob Crittenden<rcritten at redhat.com>   wrote:
>>>> 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
>>>>
>>>> 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