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