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 _______________________________________________ Freeipa-devel mailing list [email protected] https://www.redhat.com/mailman/listinfo/freeipa-devel
