Re: [Freeipa-devel] [PATCHES] 0464-0466 Multivalued targetfilter
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³ ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCHES] 0464-0466 Multivalued targetfilter
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 ti
Re: [Freeipa-devel] [PATCHES] 0464-0466 Multivalued targetfilter
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³ ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCHES] 0464-0466 Multivalued targetfilter
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
Re: [Freeipa-devel] [PATCHES] 0464-0466 Multivalued targetfilter
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 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... 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: # 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. 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? Otherwise the patches worked fine for me. Martin ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
