[Freeipa-devel] [PATCH] Fixed permission lookup

[Rob Crittenden]

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