[Freeipa-devel] [PATCHES] 0455-0459 Add support for managed permissions

Tue Jan 28 11:02:34 UTC 2014

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 attribute part in LDAP, as long as
>> our tool show prominently the effective list.
>> And also as long as all permissions behave the same way in the general
>> mechanism in LDAP.
>>
>> Simo.
>>
>
> 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? Is it OK to break master this way?
>
> As before three source lists are stored in LDAP:
> - ipaPermDefaultAttr
> - ipaPermIncludedAttr (--includedattrs)
> - ipaPermExcludedAttr (--excludedattrs)
>
> Setting --attrs ("Effective Attributes") will set the included/excluded
> attributes based on the default set.
> For normal permissions, default & excluded will be empty, and in this
> case only effective attributes will be displayed on output (unless --all
> or --raw is given).
>
>
>
> I've added some preparatory patches for #4074 to this batch, mostly to
> prevent rebase conflicts with that work. Re-numbering for a sane order.
>
> Apply on top of my patch 0452
>
> 0455 - minor fixes, unchanged from 0447
>
> 0456 - converting options on the server it will allow us to consult the
> entry's existing state. Arguably it's also cleaner to use execute than
> args_options_2_params for this.
>
> 0457 - generate ACIs in the plugin. This is the next step in obsoleting
> the ACI class, which in the end should only be necessary for updating
> old-style ACIs. The one-way function should be easier to maintain and
> extend.
>
> 0458 - allow callables in declarative tests, unchanged from 0448
>
> 0459 - managed permissions
>
>
> Or: git pull https://github.com/encukou/freeipa 3566-managed-perms
> commit 51bb7f27516202a062ffa25fedae18d0e9f302b6

I've updated the design pages.

http://www.freeipa.org/page/V3/Managed_Read_permissions
http://www.freeipa.org/page/V3/Permissions_V2
http://www.freeipa.org/page/V3/Managed_Read_permissions/tests

-- 
Petr³