Re: [Freeipa-devel] [PATCHES] 0447-0449 Add support for managed permissions
On 01/22/2014 10:27 AM, Petr Viktorin wrote: On 01/08/2014 04:49 PM, Petr Viktorin wrote: Hello, This adds managed permissions, the framework that will make our default permissions merge IPA updates and user changes sanely. There is no updater yet, nor does this add any actual managed permissions, so there's no user-visible change (beyond help text and a disabled option). To test the patch you might need to touch LDAP directly. Ticket: https://fedorahosted.org/freeipa/ticket/4033 Design (no updater plugin changes yet): http://www.freeipa.org/page/V3/Managed_Read_permissions 0447 - Minor fixes. 0448 - Since you can't create managed permissions through the API, I needed to get creative with the declarative tests. The tests will need a custom function that adds a managed perm. 0449 - The change itself. ping; any thoughts on this one? 1) 449, the comment: +Deleting or renaming a managed permission, as well as changing its target, +is not supported. +) + _( I am not sure that the phrase not supported is the right one. It sounds to me like this is something we want to allow, just not implemented yet. IMO is not allowed would be better. 2) Can you add allow_mod_for_managed flag description to parameters.py? +flags={'no_create', 'allow_mod_for_managed'}, So far we try to add all flag descriptions there. 3) When I updated the test to not delete the testperm, I tried to show the managed permission and it is not entirely clear, see: # ipa permission-show testperm Permission name: testperm Permissions: write * Attributes: cn, o, sn * Excluded attributes: cn, sn Bind rule type: all Subtree: cn=users,cn=accounts,dc=example,dc=com ACI target DN: uid=*,cn=users,cn=accounts,dc=example,dc=com Type: user * Default attributes: l, o, cn * Effective attributes: l, o The Attributes mean actually attributes explicitly allowed by user, but it is not obvious from the output. Maybe it would be better to return only Effective attributes by default and return the 3 source lists only when --all is passed. But this would require us to let Command override LDAPObject's default_attributes, which framework cannot do. Alternatively, we may choose to use the attributes differently with managed permissions: - we add the new attributeType ipaPermIncludedAttr. It would be used for the user-specified whitelist of attributes instead of ipaPermAllowedAttr - we do not use the ipaPermAllowedAttr with managed attributes at all or use it for the Effective attributes list My point is that the semantics of ipaPermAllowedAttr is different for managed and non-managed permission, so it may confuse people. For example, you may want to search for all permissions that allow attribute sn: # ipa permission-find --attrs sn - 4 permissions matched - Permission name: anon Permissions: read Attributes: sn Bind rule type: anonymous Subtree: cn=users,cn=accounts,dc=example,dc=com ACI target DN: uid=*,cn=users,cn=accounts,dc=example,dc=com Type: user ... Permission name: testperm Permissions: write Attributes: cn, o, sn Excluded attributes: cn, sn Bind rule type: anonymous Subtree: cn=users,cn=accounts,dc=example,dc=com ACI target DN: uid=*,cn=users,cn=accounts,dc=example,dc=com Type: user Default attributes: l, o, cn Effective attributes: l, o ... As you see, it matched both testperm and anon even though testperm does not really allow sn as it excluded. Thoughts? Martin ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCHES] 0447-0449 Add support for managed permissions
On 01/23/2014 12:24 PM, Martin Kosek wrote: On 01/22/2014 10:27 AM, Petr Viktorin wrote: On 01/08/2014 04:49 PM, Petr Viktorin wrote: Hello, This adds managed permissions, the framework that will make our default permissions merge IPA updates and user changes sanely. There is no updater yet, nor does this add any actual managed permissions, so there's no user-visible change (beyond help text and a disabled option). To test the patch you might need to touch LDAP directly. Ticket: https://fedorahosted.org/freeipa/ticket/4033 Design (no updater plugin changes yet): http://www.freeipa.org/page/V3/Managed_Read_permissions 0447 - Minor fixes. 0448 - Since you can't create managed permissions through the API, I needed to get creative with the declarative tests. The tests will need a custom function that adds a managed perm. 0449 - The change itself. ping; any thoughts on this one? 1) 449, the comment: +Deleting or renaming a managed permission, as well as changing its target, +is not supported. +) + _( I am not sure that the phrase not supported is the right one. It sounds to me like this is something we want to allow, just not implemented yet. IMO is not allowed would be better. Makes sense. 2) Can you add allow_mod_for_managed flag description to parameters.py? +flags={'no_create', 'allow_mod_for_managed'}, So far we try to add all flag descriptions there. OK 3) When I updated the test to not delete the testperm, I tried to show the managed permission and it is not entirely clear, see: # ipa permission-show testperm Permission name: testperm Permissions: write * Attributes: cn, o, sn * Excluded attributes: cn, sn Bind rule type: all Subtree: cn=users,cn=accounts,dc=example,dc=com ACI target DN: uid=*,cn=users,cn=accounts,dc=example,dc=com Type: user * Default attributes: l, o, cn * Effective attributes: l, o Well, this is a tradeoff between presenting what's stored in LDAP and what's in the ACI. The Attributes mean actually attributes explicitly allowed by user, but it is not obvious from the output. Maybe it would be better to return only Effective attributes by default and return the 3 source lists only when --all is passed. But this would require us to let Command override LDAPObject's default_attributes, which framework cannot do. Modifying default_attributes would not work because the 3 lists need to be loaded from LDAP to determine the effective attributes. It's possible to remove the extra attributes in the post_callback, postprocess_result already does similar output manipulation. Alternatively, we may choose to use the attributes differently with managed permissions: - we add the new attributeType ipaPermIncludedAttr. It would be used for the user-specified whitelist of attributes instead of ipaPermAllowedAttr - we do not use the ipaPermAllowedAttr with managed attributes at all or use it for the Effective attributes list My point is that the semantics of ipaPermAllowedAttr is different for managed and non-managed permission, so it may confuse people. Well, the semantics are always the same (effective = (default | allowed) - excluded). I agree that it can be confusing; perhaps I'm in too deep to judge how it looks from the outside. For example, you may want to search for all permissions that allow attribute sn: # ipa permission-find --attrs sn - 4 permissions matched - Permission name: anon Permissions: read Attributes: sn Bind rule type: anonymous Subtree: cn=users,cn=accounts,dc=example,dc=com ACI target DN: uid=*,cn=users,cn=accounts,dc=example,dc=com Type: user ... Permission name: testperm Permissions: write Attributes: cn, o, sn Excluded attributes: cn, sn Bind rule type: anonymous Subtree: cn=users,cn=accounts,dc=example,dc=com ACI target DN: uid=*,cn=users,cn=accounts,dc=example,dc=com Type: user Default attributes: l, o, cn Effective attributes: l, o ... As you see, it matched both testperm and anon even though testperm does not really allow sn as it excluded. Thoughts? Well, we could have default, included, excluded attributes stored in LDAP as now (using the name included instead of allowed), and make effective attributes (--attrs) into an updatable virtual attribute: when setting it, IPA would consult the default attributes and update included/excluded accordingly. (With non-managed permissions default is empty, so only included would be updated.) And searching on --attrs would construct an appropriate filter. I thought about this approach earlier but thought that it obscured what's actually stored in LDAP. Given recent discussions I'm now thinking I shouldn't have rejected it. -- Petr³ ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCHES] 0447-0449 Add support for managed permissions
On Thu, 2014-01-23 at 13:23 +0100, Petr Viktorin wrote: On 01/23/2014 12:24 PM, Martin Kosek wrote: On 01/22/2014 10:27 AM, Petr Viktorin wrote: On 01/08/2014 04:49 PM, Petr Viktorin wrote: Hello, This adds managed permissions, the framework that will make our default permissions merge IPA updates and user changes sanely. There is no updater yet, nor does this add any actual managed permissions, so there's no user-visible change (beyond help text and a disabled option). To test the patch you might need to touch LDAP directly. Ticket: https://fedorahosted.org/freeipa/ticket/4033 Design (no updater plugin changes yet): http://www.freeipa.org/page/V3/Managed_Read_permissions 0447 - Minor fixes. 0448 - Since you can't create managed permissions through the API, I needed to get creative with the declarative tests. The tests will need a custom function that adds a managed perm. 0449 - The change itself. ping; any thoughts on this one? 1) 449, the comment: +Deleting or renaming a managed permission, as well as changing its target, +is not supported. +) + _( I am not sure that the phrase not supported is the right one. It sounds to me like this is something we want to allow, just not implemented yet. IMO is not allowed would be better. Makes sense. 2) Can you add allow_mod_for_managed flag description to parameters.py? +flags={'no_create', 'allow_mod_for_managed'}, So far we try to add all flag descriptions there. OK 3) When I updated the test to not delete the testperm, I tried to show the managed permission and it is not entirely clear, see: # ipa permission-show testperm Permission name: testperm Permissions: write * Attributes: cn, o, sn * Excluded attributes: cn, sn Bind rule type: all Subtree: cn=users,cn=accounts,dc=example,dc=com ACI target DN: uid=*,cn=users,cn=accounts,dc=example,dc=com Type: user * Default attributes: l, o, cn * Effective attributes: l, o Well, this is a tradeoff between presenting what's stored in LDAP and what's in the ACI. The Attributes mean actually attributes explicitly allowed by user, but it is not obvious from the output. Maybe it would be better to return only Effective attributes by default and return the 3 source lists only when --all is passed. But this would require us to let Command override LDAPObject's default_attributes, which framework cannot do. Modifying default_attributes would not work because the 3 lists need to be loaded from LDAP to determine the effective attributes. It's possible to remove the extra attributes in the post_callback, postprocess_result already does similar output manipulation. Alternatively, we may choose to use the attributes differently with managed permissions: - we add the new attributeType ipaPermIncludedAttr. It would be used for the user-specified whitelist of attributes instead of ipaPermAllowedAttr - we do not use the ipaPermAllowedAttr with managed attributes at all or use it for the Effective attributes list My point is that the semantics of ipaPermAllowedAttr is different for managed and non-managed permission, so it may confuse people. Well, the semantics are always the same (effective = (default | allowed) - excluded). I agree that it can be confusing; perhaps I'm in too deep to judge how it looks from the outside. For example, you may want to search for all permissions that allow attribute sn: # ipa permission-find --attrs sn - 4 permissions matched - Permission name: anon Permissions: read Attributes: sn Bind rule type: anonymous Subtree: cn=users,cn=accounts,dc=example,dc=com ACI target DN: uid=*,cn=users,cn=accounts,dc=example,dc=com Type: user ... Permission name: testperm Permissions: write Attributes: cn, o, sn Excluded attributes: cn, sn Bind rule type: anonymous Subtree: cn=users,cn=accounts,dc=example,dc=com ACI target DN: uid=*,cn=users,cn=accounts,dc=example,dc=com Type: user Default attributes: l, o, cn Effective attributes: l, o ... As you see, it matched both testperm and anon even though testperm does not really allow sn as it excluded. Thoughts? Well, we could have default, included, excluded attributes stored in LDAP as now (using the name included instead of allowed), and make effective attributes (--attrs) into an updatable virtual attribute: when setting it, IPA would consult the default attributes and update included/excluded accordingly. (With non-managed permissions default is empty, so only included would be updated.) And searching on --attrs would construct an appropriate filter. I thought about this approach earlier but thought that it obscured what's actually stored in LDAP. Given recent discussions I'm now
Re: [Freeipa-devel] [PATCHES] 0447-0449 Add support for managed permissions
On 01/08/2014 04:49 PM, Petr Viktorin wrote: Hello, This adds managed permissions, the framework that will make our default permissions merge IPA updates and user changes sanely. There is no updater yet, nor does this add any actual managed permissions, so there's no user-visible change (beyond help text and a disabled option). To test the patch you might need to touch LDAP directly. Ticket: https://fedorahosted.org/freeipa/ticket/4033 Design (no updater plugin changes yet): http://www.freeipa.org/page/V3/Managed_Read_permissions 0447 - Minor fixes. 0448 - Since you can't create managed permissions through the API, I needed to get creative with the declarative tests. The tests will need a custom function that adds a managed perm. 0449 - The change itself. ping; any thoughts on this one? -- Petr³ ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCHES] 0447-0449 Add support for managed permissions
On 01/08/2014 04:49 PM, Petr Viktorin wrote: Hello, This adds managed permissions, the framework that will make our default permissions merge IPA updates and user changes sanely. There is no updater yet, nor does this add any actual managed permissions, so there's no user-visible change (beyond help text and a disabled option). To test the patch you might need to touch LDAP directly. Ticket: https://fedorahosted.org/freeipa/ticket/4033 Design (no updater plugin changes yet): http://www.freeipa.org/page/V3/Managed_Read_permissions 0447 - Minor fixes. 0448 - Since you can't create managed permissions through the API, I needed to get creative with the declarative tests. The tests will need a custom function that adds a managed perm. 0449 - The change itself. One thing I meant to write in this mail but forgot: I added a comment to the end of VERSION. There was a patch/thread about this, but it hasn't really reached a conclusion: https://www.redhat.com/archives/freeipa-devel/2013-December/msg00084.html If anyone is still opposed to the comment speak up and I can remove it. FYI, my roadmap: I'll now concentrate on #4074 (Use targetfilter targetattrfilter in permissions) since it looks like we'll want most of the default permissions to use location + objectclass targetfilter. (The ticket turned out to be trickier than it seems, stay tuned). http://www.redhat.com/archives/freeipa-devel/2013-December/msg00063.html I'm planning to add the updater, and design how plugins will advertise their default permissions, after we decide how most of the default permissions will look. -- Petr³ ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCHES] 0447-0449 Add support for managed permissions
On Thu, 09 Jan 2014, Martin Kosek wrote: On 01/08/2014 06:46 PM, Petr Viktorin wrote: On 01/08/2014 04:49 PM, Petr Viktorin wrote: Hello, This adds managed permissions, the framework that will make our default permissions merge IPA updates and user changes sanely. There is no updater yet, nor does this add any actual managed permissions, so there's no user-visible change (beyond help text and a disabled option). To test the patch you might need to touch LDAP directly. Ticket: https://fedorahosted.org/freeipa/ticket/4033 Design (no updater plugin changes yet): http://www.freeipa.org/page/V3/Managed_Read_permissions 0447 - Minor fixes. 0448 - Since you can't create managed permissions through the API, I needed to get creative with the declarative tests. The tests will need a custom function that adds a managed perm. 0449 - The change itself. One thing I meant to write in this mail but forgot: I added a comment to the end of VERSION. There was a patch/thread about this, but it hasn't really reached a conclusion: https://www.redhat.com/archives/freeipa-devel/2013-December/msg00084.html If anyone is still opposed to the comment speak up and I can remove it. +1 for using the comment from me, if some server side setting is not an option. The format you used -IPA_API_VERSION_MINOR=72 +IPA_API_VERSION_MINOR=73 +# Last change: pviktori - Managed permissions is quite lightweight and not a burden to write. I agree, it is simple and clear. -- / Alexander Bokovoy ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel