[Freeipa-devel] [PATCHES] 0464-0466 Multivalued targetfilter

[Martin Kosek]

On 02/20/2014 12:57 PM, Petr Viktorin wrote:
> On 02/19/2014 05:32 PM, Martin Kosek wrote:
>> On 02/19/2014 10:44 AM, Petr Viktorin wrote:
>>> On 02/18/2014 08:02 PM, Petr Viktorin wrote:
>>>> On 02/18/2014 09:42 AM, Martin Kosek wrote:
>>>>> On 02/13/2014 01:12 PM, Petr Viktorin wrote:
>>>>>> Hello,
>>>>>> These patches fix https://fedorahosted.org/freeipa/ticket/4074
>>>>>> Design:
>>>>>> http://www.freeipa.org/page/V3/Multivalued_target_filters_in_permissions
>>>>>>
>>>>>>
>>>>>> Since --type, affects only targetfilter values in the form
>>>>>> "(objectclass=...)"
>>>>>> and leaves others alone, and in the -mod command we don't fetch the
>>>>>> existing
>>>>>> entry until the pre_callback, I had to put the adds/removes in the
>>>>>> context. I
>>>>>> don't think the approach is too terrible given the limitations.
>>>>>
>>>>> 464:
>>>>>
>>>>> 1) Internal Error:
>>>>>
>>>>> # ipa permission-mod can_write2 --filter='foo=bar'
>>>>> ipa: ERROR: an internal error has occurred
>>>>
>>>> Thanks for the catch! Fixed & added to tests.
>>>>
>>>>> 2) ACI target filter
>>>>>
>>>>> I would relabel targetfilter from "ACI target filter" to "Target
>>>>> filter" to
>>>>> make it consistent with other ACI attributes. We are sort of hiding
>>>>> there are
>>>>> any underlying ACIs anyway...
>>>>
>>>> Fixed.
>>>>
>>>>> 3) I am now thinking about the behavior of --memberof and --filter
>>>>> options and
>>>>> how they interact. It looks OK except for the case when I set filter
>>>>> to None:
>>>>
>>>> The same would happen when setting --filter to any other value(s) that
>>>> don't include existing memberOf filters.
>>>>
>>>>> # ipa permission-mod can_write2 --memberof=bar
>>>>> --------------------------------
>>>>> Modified permission "can_write2"
>>>>> --------------------------------
>>>>>     Permission name: can_write2
>>>>>     Permissions: write
>>>>>     Effective attributes: description
>>>>>     Bind rule type: permission
>>>>>     Subtree: dc=example,dc=com
>>>>>     ACI target filter: (foo=bar),
>>>>>
>>>>> (memberOf=cn=bar,cn=groups,cn=accounts,dc=example,dc=com)
>>>>>     Member of group: bar
>>>>> # ipa permission-mod can_write2 --filter=
>>>>> --------------------------------
>>>>> Modified permission "can_write2"
>>>>> --------------------------------
>>>>>     Permission name: can_write2
>>>>>     Permissions: write
>>>>>     Effective attributes: description
>>>>>     Bind rule type: permission
>>>>>     Subtree: dc=example,dc=com
>>>>>
>>>>> Then both memberOf and filter is erased. Are we ok with that?
>>>>> Shouldn't we keep
>>>>> memberOf part until that is set to None? I am not insisting, just
>>>>> trying to
>>>>> discuss the best behavior.
>>>>
>>>> Memberof affects the filter; this is even pointed out in --help output.
>>>> The alternative would be to make --filter= exclude filters affected by
>>>> other options, which I think would be even more confusing.
>>>> Keep in mind --type sets (objectclass=...) filters in exactly the same
>>>> way as --memberof works for (memberof=...).
>>>> The --memberof, --targetgroup, --type options are just shortcuts for
>>>> setting other permission attributes. I'm hoping we can get this message
>>>> across, in --help, and in the docs, well enough to reduce the confusion.
>>>>
>>>>> 465: I know this was already discussed previously, but I am now having
>>>>> second
>>>>> thoughts - should we use posixAccount as THE objectclass for user
>>>>> targetfilter?
>>>>>
>>>>> # ipa permission-add can_write --permissions write --attrs=description
>>>>> --type=user
>>>>> ----------------------------
>>>>> Added permission "can_write"
>>>>> ----------------------------
>>>>>     Permission name: can_write
>>>>>     Permissions: write
>>>>>     Effective attributes: description
>>>>>     Bind rule type: permission
>>>>>     Subtree: cn=users,cn=accounts,dc=example,dc=com
>>>>>     ACI target filter: (objectclass=posixaccount)
>>>>>     Type: user
>>>>>
>>>>> What if we add system users at some point which would miss the
>>>>> posixaccount
>>>>> objectclass? Wouldn't it be better to use the inetOrgPerson structural
>>>>> objectclass instead of posixAccount? Simo or Ludwig, any opinion on this?
>>>>
>>>> I'm not opposed.
>>>> (I don't think it should block this patchset though. We'll have to add
>>>> canonical objectclasses to all the types that don't currently have them.
>>>> Deciding it for `user` can be a part of that effort.)
>>>
>>> Apologies, I've sent a slightly wrong version of the tests. Here's the fixed
>>> patchset.
>>>
>>
>> Thanks. New check works fine, but the attribute name in the error message seems
>> inconsistent:
>>
>> # ipa permission-mod can_write --filter=foo=bar
>> ipa: ERROR: invalid 'filter': must be enclosed in parentheses
>>
>> # ipa permission-mod can_write --filter="(foo=bar))"
>> ipa: ERROR: invalid 'ipapermtargetfilter': Bad search filter
>>
>> If you fix that, it's ACK for all.
>>
>> Martin
>>
> 
> It turns out this is a deeper issue; for the time being I'd rather defer it for
> more discussion & refactoring than hack around it.
> 
> https://fedorahosted.org/freeipa/ticket/4183
> 

Ok, agreed. ACK to the original patches then.

Martin