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

Reply via email to