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

[Petr Viktorin]

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

-- 
Petr³