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

[Petr Viktorin]

On 02/20/2014 01:06 PM, Martin Kosek wrote:
> 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
>

Thanks, pushed to master: 0f1e13761993cdc77a573a68686723fa816ce5a9

-- 
Petr³