[Freeipa-devel] [PATCHES] 0473-0477 Managed permission updater, part 1

[Petr Viktorin]

On 02/28/2014 02:47 PM, Petr Viktorin wrote:
> On 02/28/2014 02:12 PM, Martin Kosek wrote:
>> On 02/26/2014 10:44 AM, Petr Viktorin wrote:
>>> Hello,
>>> Here are a few fixes/improvements, and the first part of a managed
>>> permission
>>> updater.
>>>
>>> The patches should go in this order but don't need to be ACKed/pushed
>>> all at once.
>>>
>>>
>>> Design:
>>> http://www.freeipa.org/page/V3/Managed_Read_permissions#Default_Permission_Updater
>>>
>>> Part of the work for: https://fedorahosted.org/freeipa/ticket/3566
>>>
>>>
>>> This part is a "preview" of sorts, to get the basic mechanism and the
>>> metadata
>>> format reviewed before I add all of the default read permissions.
>>> It implements the first section of "Default Permission Updater" in
>>> the design;
>>> "Replacing legacy default permissions" and "Removing the global
>>> anonymous read
>>> ACI" is left for later.
>>> Metadata is added for the netgroup plugin* for starters, so
>>> installing this
>>> will give you two shiny new read permissions:
>>>
>>> $ ipa permission-find ipa: --all
>>> ---------------------
>>> 2 permissions matched
>>> ---------------------
>>>    dn: cn=ipa:Read Netgroup Membership,cn=permissions,cn=pbac,$SUFFIX
>>>    Permission name: ipa:Read Netgroup Membership
>>>    Permissions: read, compare, search
>>>    Effective attributes: externalhost, member, memberof, memberuser
>>>    Default attributes: member, memberof, memberuser, externalhost
>>>    Bind rule type: all
>>>    Subtree: cn=ng,cn=alt,$SUFFIX
>>>    Target filter: (objectclass=ipanisnetgroup)
>>>    Type: netgroup
>>>    ipapermissiontype: V2, MANAGED, SYSTEM
>>>    objectclass: ipapermission, groupofnames, top, ipapermissionv2
>>>
>>>    dn: cn=ipa:Read Netgroups,cn=permissions,cn=pbac,$SUFFIX
>>>    Permission name: ipa:Read Netgroups
>>>    Permissions: read, compare, search
>>>    Effective attributes: cn, description, hostcategory, ipaenabledflag,
>>> ipauniqueid, nisdomainname, usercategory
>>>    Default attributes: cn, usercategory, hostcategory, ipauniqueid,
>>> ipaenabledflag, nisdomainname, description
>>>    Bind rule type: all
>>>    Subtree: cn=ng,cn=alt,$SUFFIX
>>>    Target filter: (objectclass=ipanisnetgroup)
>>>    Type: netgroup
>>>    ipapermissiontype: V2, MANAGED, SYSTEM
>>>    objectclass: ipapermission, groupofnames, top, ipapermissionv2
>>> ----------------------------
>>> Number of entries returned 2
>>> ----------------------------
>>>
>>> with corresponding ACIs at cn=ng,cn=alt,$SUFFIX:
>>>
>>> (targetattr = "externalhost || member || memberof ||
>>> memberuser")(targetfilter
>>> = "(objectclass=ipanisnetgroup)")(version 3.0;acl
>>> "permission:ipa:Read Netgroup
>>> Membership";allow (read,compare,search) userdn = "ldap:///all";)
>>> (targetattr = "cn || description || hostcategory || ipaenabledflag ||
>>> ipauniqueid || nisdomainname || usercategory")(targetfilter =
>>> "(objectclass=ipanisnetgroup)")(version 3.0;acl "permission:ipa:Read
>>> Netgroups";allow (read,compare,search) userdn = "ldap:///all";)
>>>
>>>
>>>
>>> Patches:
>>>
>>> 0473: Enables refactoring that will make it more clear (to humans and
>>> machines)
>>> what plugins code depends on.
>>> https://fedorahosted.org/freeipa/ticket/4185
>>>
>>> 0474: Fix handling of the search term for legacy permissions
>>> My code that's in master now handles the search term incorrectly.
>>> This does a
>>> better job.
>>>
>>> 0475: Fix tests that relied on some assumptions I'll be breaking
>>>
>>> 0476: Allow modifying (but not creating) permissions with ":" in the
>>> name
>>>
>>> 0477: Permission updater & sample metadata
>>>
>>
>> 1) 476: Typo in test name:
>>
>> +            desc='Search fo rnonexisting permission with ":" in the
>> name',
>
> Will fix.
>
>> 2) 477: do we want to log anything when permission is up to date?
>>
>> +            try:
>> +                ldap.update_entry(entry)
>> +            except errors.EmptyModlist:
>> +                return
>
> I don't think that's needed, after all that's the expected behavior in
> all but the first upgrade.
> But I'll be happy to add it if you think it would be better.
>
>> 3) I am not sure if this was initiated by this patch, but when I
>> checked access
>> log for a "permission-find --name ipa" operation, it produced over 170
>> LDAP
>> operations, most of them asking for the same information many times. See
>> attached access log excerpt.
>
> It's unrelated to this patch. I've started optimizing permission-find
> with many legacy permissions, expect a patch soon.
>
>> 4) I have been quite resilient to the prefixes for the permissions,
>> but it
>> seems it is the easier possible approach to fix conflicts with user
>> permissions
>> without having to check that later for every upgrade or doing more
>> complex
>> stuff like multiple RDNs or different container for system and user
>> permissions.
>>
>> I am now just thinking about the prefixing. Now you use this name:
>>
>> ipa:Read Netgroups
>>
>> Would it be "nicer" to use:
>>
>> IPA:Read Netgroups
>> or
>> IPA: Read Netgroups
>> or even
>> [IPA] Read Netgroups
>> ? :-)
>
> Bikeshedding time!
> Everyone on the list, please chime in!

Bikeshedding results from today's meeting:

"ipa: " pviktori++
"System: " mkosek++ simo+ ab++
"Builtin: " simo++ pvo+
"Default: "

The winner is "System: ", so I'll go and change to that.

> I don't really have a preference.
>
>
>> 5) Are we sure we want to make our code Python 2.7 dependent?
>>
>> +            'ipapermright': {'read', 'search', 'compare'},
>>
>> I did not test if we do not require some python 2.7 feature elsewhere
>> already,
>> but this construct raised a warning for me.
>
> That ship has sailed already.
> Recently there was commit a8ba5e0 which explicitly mentions set literals
> (and which, coincidentally, you acked...), but I'm pretty sure IPA
> needed Python 2.7+ before that.
> After all we don't test on Python 2.6 and don't support any OS with
> Python 2.6.
>
>> Otherwise the patches seemed to work fine. I also tried to play with
>> the global
>> ACI blacklist blacklisting and it worked as well, when I for example
>> added
>> userpassword to netgroup managed_permissions.
>



-- 
Petr³