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

Mon, 03 Mar 2014 07:11:28 -0800 

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³

_______________________________________________
Freeipa-devel mailing list
Freeipa-devel@redhat.com
https://www.redhat.com/mailman/listinfo/freeipa-devel

Reply via email to