[Freeipa-devel] [PATCHES] 0455-0459 Add support for managed permissions
Petr Viktorin
pviktori at redhat.com
Fri Jan 24 15:48:52 UTC 2014
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
--
Petr³
-------------- next part --------------
A non-text attachment was scrubbed...
Name: freeipa-pviktori-0455-Permission-plugin-fixes.patch
Type: text/x-patch
Size: 3959 bytes
Desc: not available
URL: <http://listman.redhat.com/archives/freeipa-devel/attachments/20140124/a15621ed/attachment.bin>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: freeipa-pviktori-0456-permission-plugin-Convert-options-in-execute-not-arg.patch
Type: text/x-patch
Size: 3198 bytes
Desc: not available
URL: <http://listman.redhat.com/archives/freeipa-devel/attachments/20140124/a15621ed/attachment-0001.bin>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: freeipa-pviktori-0457-permission-plugin-Generate-ACIs-in-the-plugin.patch
Type: text/x-patch
Size: 3125 bytes
Desc: not available
URL: <http://listman.redhat.com/archives/freeipa-devel/attachments/20140124/a15621ed/attachment-0002.bin>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: freeipa-pviktori-0458-Make-it-possible-to-call-custom-functions-in-Declara.patch
Type: text/x-patch
Size: 1798 bytes
Desc: not available
URL: <http://listman.redhat.com/archives/freeipa-devel/attachments/20140124/a15621ed/attachment-0003.bin>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: freeipa-pviktori-0459-Add-support-for-managed-permissions.patch
Type: text/x-patch
Size: 75372 bytes
Desc: not available
URL: <http://listman.redhat.com/archives/freeipa-devel/attachments/20140124/a15621ed/attachment-0004.bin>
More information about the Freeipa-devel
mailing list