Re: [Freeipa-devel] [PATCHES] 0455-0459 Add support for managed permissions
On 02/10/2014 04:53 PM, Petr Viktorin wrote:
On 01/31/2014 01:43 PM, Martin Kosek wrote:
On 01/24/2014 04:48 PM, Petr Viktorin wrote:
On 01/23/2014 02:42 PM, Simo Sorce wrote:
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
Re: [Freeipa-devel] [PATCHES] 0455-0459 Add support for managed permissions
On 02/12/2014 04:57 PM, Martin Kosek wrote:
On 02/10/2014 04:53 PM, Petr Viktorin wrote:
On 01/31/2014 01:43 PM, Martin Kosek wrote:
On 01/24/2014 04:48 PM, Petr Viktorin wrote:
On 01/23/2014 02:42 PM, Simo Sorce wrote:
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
Re: [Freeipa-devel] [PATCHES] 0455-0459 Add support for managed permissions
On 01/24/2014 04:48 PM, Petr Viktorin wrote:
On 01/23/2014 02:42 PM, Simo Sorce wrote:
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
thinking I shouldn't
Re: [Freeipa-devel] [PATCHES] 0455-0459 Add support for managed permissions
On 01/24/2014 04:48 PM, Petr Viktorin wrote:
On 01/23/2014 02:42 PM, Simo Sorce wrote:
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
thinking I shouldn't have rejected it.
I would take a consistent approach indeed.
I do not much care for the virtual
Re: [Freeipa-devel] [PATCHES] 0455-0459 Add support for managed permissions
On 01/27/2014 09:17 AM, Petr Spacek wrote: On 27.1.2014 08:07, Martin Kosek wrote: On 01/24/2014 05:23 PM, Simo Sorce wrote: On Fri, 2014-01-24 at 17:17 +0100, Petr Viktorin wrote: On 01/24/2014 04:57 PM, Simo Sorce wrote: On Fri, 2014-01-24 at 16:48 +0100, Petr Viktorin wrote: ... Technically we could alias the name so the attribute can be called either way, but that is not necessarily a good option either. If breaking master is unacceptable, we can use the old name instead. ipaPermIncludedAttr is more consistent but ipaPermAllowedAttr isn't downright wrong. Ok, let's hear other opinions, I see a lot f value in consistent naming, and not breaking a developer build is not that strong of a reason to have substandard naming I guess. What do others think ? Simo. Hmm, I obviously see things differently here. I would rather break the master and let developers running on the git version to reinstall the servers (including myself) than to have to live with suboptimal attribute name for ever or by adding unnecessary cruft to the code... I think you got lost in the nots. You and Simo agree. (Speaking as lab admin:) Please, break it! It will force people to finally reinstall years old VMs! :-) -- Petr³ ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCHES] 0455-0459 Add support for managed permissions
On 01/27/2014 09:50 AM, Petr Viktorin wrote: On 01/27/2014 09:17 AM, Petr Spacek wrote: On 27.1.2014 08:07, Martin Kosek wrote: On 01/24/2014 05:23 PM, Simo Sorce wrote: On Fri, 2014-01-24 at 17:17 +0100, Petr Viktorin wrote: On 01/24/2014 04:57 PM, Simo Sorce wrote: On Fri, 2014-01-24 at 16:48 +0100, Petr Viktorin wrote: ... Technically we could alias the name so the attribute can be called either way, but that is not necessarily a good option either. If breaking master is unacceptable, we can use the old name instead. ipaPermIncludedAttr is more consistent but ipaPermAllowedAttr isn't downright wrong. Ok, let's hear other opinions, I see a lot f value in consistent naming, and not breaking a developer build is not that strong of a reason to have substandard naming I guess. What do others think ? Simo. Hmm, I obviously see things differently here. I would rather break the master and let developers running on the git version to reinstall the servers (including myself) than to have to live with suboptimal attribute name for ever or by adding unnecessary cruft to the code... I think you got lost in the nots. You and Simo agree. Ah, I see - you are right (indeed to many nots). Martin ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCHES] 0455-0459 Add support for managed permissions
On 01/24/2014 05:23 PM, Simo Sorce wrote: On Fri, 2014-01-24 at 17:17 +0100, Petr Viktorin wrote: On 01/24/2014 04:57 PM, Simo Sorce wrote: On Fri, 2014-01-24 at 16:48 +0100, Petr Viktorin wrote: ... Technically we could alias the name so the attribute can be called either way, but that is not necessarily a good option either. If breaking master is unacceptable, we can use the old name instead. ipaPermIncludedAttr is more consistent but ipaPermAllowedAttr isn't downright wrong. Ok, let's hear other opinions, I see a lot f value in consistent naming, and not breaking a developer build is not that strong of a reason to have substandard naming I guess. What do others think ? Simo. Hmm, I obviously see things differently here. I would rather break the master and let developers running on the git version to reinstall the servers (including myself) than to have to live with suboptimal attribute name for ever or by adding unnecessary cruft to the code... Martin ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCHES] 0455-0459 Add support for managed permissions
On Fri, 2014-01-24 at 16:48 +0100, Petr Viktorin wrote: All right. Here are patches; I'll start updating the design page. **NOTE** This renames the 'ipaPermAllowedAttr' attribute to 'ipaPermIncludedAttr'. Upgrades from master will fail, so please install a new server. Of course no released versions of FreeIPA are affected. I assume there's no clean way to rename an attribute without changing the OID? You do not need to change the OID in this case, these attributes have not been released in any production version so I think it is ok to just rename. Is it OK to break master this way? I would prefer you didn't, what breaks master exactly ? The schema update ? Technically we could alias the name so the attribute can be called either way, but that is not necessarily a good option either. Simo. -- Simo Sorce * Red Hat, Inc * New York ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCHES] 0455-0459 Add support for managed permissions
On 01/24/2014 04:57 PM, Simo Sorce wrote: On Fri, 2014-01-24 at 16:48 +0100, Petr Viktorin wrote: All right. Here are patches; I'll start updating the design page. **NOTE** This renames the 'ipaPermAllowedAttr' attribute to 'ipaPermIncludedAttr'. Upgrades from master will fail, so please install a new server. Of course no released versions of FreeIPA are affected. I assume there's no clean way to rename an attribute without changing the OID? You do not need to change the OID in this case, these attributes have not been released in any production version so I think it is ok to just rename. Is it OK to break master this way? I would prefer you didn't, what breaks master exactly ? The schema update ? Yes. The schema update fails when it tries to add a differently named attribute with the same OID. Technically we could alias the name so the attribute can be called either way, but that is not necessarily a good option either. If breaking master is unacceptable, we can use the old name instead. ipaPermIncludedAttr is more consistent but ipaPermAllowedAttr isn't downright wrong. -- Petr³ ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
