On 03/07/2014 10:45 AM, Martin Kosek wrote:
On 03/05/2014 01:48 PM, Petr Viktorin wrote:
On 03/03/2014 04:10 PM, Petr Viktorin wrote:
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
[...]


1) 476: Typo in test name:

+            desc='Search fo rnonexisting permission with ":" in the
name',

Will fix.

Fixed

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.

I've added a DEBUG message here.

[...]
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.

Done.

Thanks. The patch set looks good now, I just see that the update process may
not be optimal After I run the upgrade second time, it still tries to update
the ACI even though there was no change done to the permission object and
should thus raise errors.EmptyModlist and skip the ACI update:

2014-03-07T09:44:34Z INFO Updating managed permissions for netgroup
2014-03-07T09:44:34Z INFO Updating managed permission: System: Read Netgroups
2014-03-07T09:44:34Z DEBUG Updating ACI for managed permission: System: Read
Netgroups
2014-03-07T09:44:34Z DEBUG Removing ACI u'(targetattr = "cn || description ||
hostcategory || ipaenabledflag || ipauniqueid || nisdomainname ||
usercategory")(targetfilter = "(objectclass=ipanisnetgroup)")(version 3.0;acl
"permission:System: Read Netgroups";allow (read,compare,search) userdn =
"ldap:///all";;)' from cn=ng,cn=alt,dc=idm,dc=lab,dc=eng,dc=brq,dc=redhat,dc=com
2014-03-07T09:44:34Z DEBUG Adding ACI u'(targetattr = "cn || description ||
hostcategory || ipaenabledflag || ipauniqueid || nisdomainname ||
usercategory")(targetfilter = "(objectclass=ipanisnetgroup)")(version 3.0;acl
"permission:System: Read Netgroups";allow (read,compare,search) userdn =
"ldap:///all";;)' to cn=ng,cn=alt,dc=idm,dc=lab,dc=eng,dc=brq,dc=redhat,dc=com
2014-03-07T09:44:34Z INFO No changes to ACI
2014-03-07T09:44:34Z INFO Updating managed permission: System: Read Netgroup
Membership

Any idea what could cause this?

Ah, you're right. The updater tried to remove the 'top' objectclass, since it found that in the permission but it wasn't in permission.object_class.
I added 'top' to the list in a new patch.

There was a minor conflict with master; I'm attaching the whole rebased patchset for convenience.

--
PetrĀ³

From 15761fedf08569c37187e246528695bef0b9d8ff Mon Sep 17 00:00:00 2001
From: Petr Viktorin <pvikt...@redhat.com>
Date: Wed, 12 Feb 2014 16:17:39 +0100
Subject: [PATCH] Allow indexing API object types by class

This allows code like:
    from ipalib.plugins.dns import dnszone_mod

    api.Command[dnszone_mod]

This form should be preferred when getting specific objects
because it ensures that the appropriate plugin is imported.

https://fedorahosted.org/freeipa/ticket/4185
---
 ipalib/base.py                    | 15 +++++++++++++--
 ipatests/test_ipalib/test_base.py | 12 ++++++++++--
 2 files changed, 23 insertions(+), 4 deletions(-)

diff --git a/ipalib/base.py b/ipalib/base.py
index 83d778fe751e66414fe34d0d3243cdae34941ad6..91259c7f3b480ffc0e70061e1efa0135ff500478 100644
--- a/ipalib/base.py
+++ b/ipalib/base.py
@@ -287,7 +287,7 @@ class NameSpace(ReadOnly):
     >>> class Member(object):
     ...     def __init__(self, i):
     ...         self.i = i
-    ...         self.name = 'member%d' % i
+    ...         self.name = self.__name__ = 'member%d' % i
     ...     def __repr__(self):
     ...         return 'Member(%d)' % self.i
     ...
@@ -378,6 +378,14 @@ class NameSpace(ReadOnly):
     >>> unsorted_ns[0]
     Member(7)
 
+    As a special extension, NameSpace objects can be indexed by objects that
+    have a "__name__" attribute (e.g. classes). These lookups are converted
+    to lookups on the name:
+
+    >>> class_ns = NameSpace([Member(7), Member(3), Member(5)], sort=False)
+    >>> unsorted_ns[Member(3)]
+    Member(3)
+
     The `NameSpace` class is used in many places throughout freeIPA.  For a few
     examples, see the `plugable.API` and the `frontend.Command` classes.
     """
@@ -447,6 +455,7 @@ def __contains__(self, name):
         """
         Return ``True`` if namespace has a member named ``name``.
         """
+        name = getattr(name, '__name__', name)
         return name in self.__map
 
     def __getitem__(self, key):
@@ -455,12 +464,14 @@ def __getitem__(self, key):
 
         :param key: The name or index of a member, or a slice object.
         """
+        key = getattr(key, '__name__',  key)
         if isinstance(key, basestring):
             return self.__map[key]
         if type(key) in (int, slice):
             return self.__members[key]
         raise TypeError(
-            TYPE_ERROR % ('key', (str, int, slice), key, type(key))
+            TYPE_ERROR % ('key', (str, int, slice, 'object with __name__'),
+                          key, type(key))
         )
 
     def __repr__(self):
diff --git a/ipatests/test_ipalib/test_base.py b/ipatests/test_ipalib/test_base.py
index ef6c180c798632cb0cebd9b6ad89caaf744d7931..0117946bc2de8cb2aa014e42e6c6569b33f0900b 100644
--- a/ipatests/test_ipalib/test_base.py
+++ b/ipatests/test_ipalib/test_base.py
@@ -195,7 +195,7 @@ def membername(i):
 class DummyMember(object):
     def __init__(self, i):
         self.i = i
-        self.name = membername(i)
+        self.name = self.__name__ = membername(i)
 
 
 def gen_members(*indexes):
@@ -283,8 +283,10 @@ def test_contains(self):
             for i in yes:
                 assert membername(i) in o
                 assert membername(i).upper() not in o
+                assert DummyMember(i) in o
             for i in no:
                 assert membername(i) not in o
+                assert DummyMember(i) not in o
 
     def test_getitem(self):
         """
@@ -320,9 +322,15 @@ def test_getitem(self):
             assert o[:-4] == members[:-4]
             assert o[-9:-4] == members[-9:-4]
 
+            # Test retrieval by value
+            for member in members:
+                assert o[DummyMember(member.i)] is member
+
             # Test that TypeError is raised with wrong type
             e = raises(TypeError, o.__getitem__, 3.0)
-            assert str(e) == TYPE_ERROR % ('key', (str, int, slice), 3.0, float)
+            assert str(e) == TYPE_ERROR % (
+                'key', (str, int, slice, 'object with __name__'),
+                3.0, float)
 
     def test_repr(self):
         """
-- 
1.8.5.3

From cc752f8b2abbe8c1972de1b7fff2ce8f086be652 Mon Sep 17 00:00:00 2001
From: Petr Viktorin <pvikt...@redhat.com>
Date: Mon, 24 Feb 2014 10:36:47 +0100
Subject: [PATCH] permission-find: Fix handling of the search term for legacy
 permissions

Previously the search term was only applied to the name.
Fix it so that it filters results based on any attribute.
---
 ipalib/plugins/permission.py | 41 ++++++++++++++++++++++++-----------------
 1 file changed, 24 insertions(+), 17 deletions(-)

diff --git a/ipalib/plugins/permission.py b/ipalib/plugins/permission.py
index bd7f5da6adf7fd7f576d69f4a9c6c4051ec1ac94..9193dcaa2eb2c8fd293e851d2a2bb1c380c5bcfd 100644
--- a/ipalib/plugins/permission.py
+++ b/ipalib/plugins/permission.py
@@ -1096,8 +1096,9 @@ def post_callback(self, ldap, entries, truncated, *args, **options):
 
             filters = ['(objectclass=ipaPermission)',
                        '(!(ipaPermissionType=V2))']
-            if args:
-                filters.append(ldap.make_filter_from_attr('cn', args[0],
+            if 'name' in options:
+                filters.append(ldap.make_filter_from_attr('cn',
+                                                          options['name'],
                                                           exact=False))
             attrs_list = list(self.obj.default_attributes)
             attrs_list += list(self.obj.attribute_members)
@@ -1129,22 +1130,28 @@ def post_callback(self, ldap, entries, truncated, *args, **options):
                     break
                 self.obj.upgrade_permission(entry, output_only=True,
                                             cached_acientry=root_entry)
-                cn = entry.single_value['cn']
-                if any(a.lower() in cn.lower() for a in args if a):
-                    entries.append(entry)
+                # If all given options match, include the entry
+                # Do a case-insensitive match, on any value if multi-valued
+                for opt in attribute_options:
+                    optval = options[opt]
+                    if not isinstance(optval, (tuple, list)):
+                        optval = [optval]
+                    value = entry.get(opt)
+                    if not value:
+                        break
+                    if not all(any(str(ov).lower() in str(v).lower()
+                                for v in value) for ov in optval):
+                        break
                 else:
-                    # If all given options match, include the entry
-                    # Do a case-insensitive match, on any value if multi-valued
-                    for opt in attribute_options:
-                        optval = options[opt]
-                        if not isinstance(optval, (tuple, list)):
-                            optval = [optval]
-                        value = entry.get(opt)
-                        if not value:
-                            break
-                        if not all(any(str(ov).lower() in str(v).lower()
-                                   for v in value) for ov in optval):
-                            break
+                    # Each search term must be present in some
+                    # attribute value
+                    for arg in args:
+                        if arg:
+                            arg = arg.lower()
+                            if not any(arg in str(value).lower()
+                                       for values in entry.values()
+                                       for value in values):
+                                break
                     else:
                         entries.append(entry)
 
-- 
1.8.5.3

From 7fb632c5f2bf9d68fcd271490613d7117ee9bc38 Mon Sep 17 00:00:00 2001
From: Petr Viktorin <pvikt...@redhat.com>
Date: Fri, 21 Feb 2014 12:29:39 +0100
Subject: [PATCH] test_permission_plugin: Fix tests that make too broad
 assumptions

The test that searches with a limit of 1 assumes a specific order
LDAP returns entries in. Future patches will change this order.
Do not check the specific entry returned.

The test that searched for --bindtype assumed that no anonymous
permissions exist in a clean install. Again, this will be changed
in future patches.
Add a name to the bindtype test, and add a negatitive test to
verify the filtering works.
---
 ipatests/test_xmlrpc/test_old_permission_plugin.py | 26 ++++--------
 ipatests/test_xmlrpc/test_permission_plugin.py     | 46 ++++++++++------------
 2 files changed, 28 insertions(+), 44 deletions(-)

diff --git a/ipatests/test_xmlrpc/test_old_permission_plugin.py b/ipatests/test_xmlrpc/test_old_permission_plugin.py
index 72c218208a0d512c66a995de9d2a1d72ea5e1d8c..816890462fb2b764becb6a0cec48c7219dffc0f7 100644
--- a/ipatests/test_xmlrpc/test_old_permission_plugin.py
+++ b/ipatests/test_xmlrpc/test_old_permission_plugin.py
@@ -565,31 +565,19 @@ class test_old_permission(Declarative):
 
         # This tests setting truncated to True in the post_callback of
         # permission_find(). The return order in LDAP is not guaranteed
-        # but in practice this is the first entry it finds. This is subject
-        # to change.
+        # so do not check the actual entry.
         dict(
             desc='Search for permissions by attr with a limit of 1 (truncated)',
-            command=('permission_find', [], dict(attrs=u'ipaenabledflag',
-                                                 sizelimit=1)),
+            command=('permission_find', [u'Modify'],
+                     dict(attrs=u'ipaenabledflag', sizelimit=1)),
             expected=dict(
                 count=1,
                 truncated=True,
                 summary=u'1 permission matched',
-                result=[
-                    {
-                        'dn': DN(('cn', 'Modify HBAC rule'),
-                                 api.env.container_permission, api.env.basedn),
-                        'cn': [u'Modify HBAC rule'],
-                        'objectclass': objectclasses.permission,
-                        'member_privilege': [u'HBAC Administrator'],
-                        'memberindirect_role': [u'IT Security Specialist'],
-                        'permissions' : [u'write'],
-                        'attrs': [u'servicecategory', u'sourcehostcategory', u'cn', u'description', u'ipaenabledflag', u'accesstime', u'usercategory', u'hostcategory', u'accessruletype', u'sourcehost'],
-                        'ipapermbindruletype': [u'permission'],
-                        'ipapermtarget': [DN('ipauniqueid=*', hbac_dn)],
-                        'subtree': u'ldap:///%s' % api.env.basedn,
-                    },
-                ],
+                result=[lambda res:
+                    DN(res['dn']).endswith(DN(api.env.container_permission,
+                                              api.env.basedn)) and
+                    'ipapermission' in res['objectclass']],
             ),
         ),
 
diff --git a/ipatests/test_xmlrpc/test_permission_plugin.py b/ipatests/test_xmlrpc/test_permission_plugin.py
index 62ff20e563c4653a41223959d02170c7a6e519f8..da48a17a790b9c5cf8a675a9d3d8520fae7af974 100644
--- a/ipatests/test_xmlrpc/test_permission_plugin.py
+++ b/ipatests/test_xmlrpc/test_permission_plugin.py
@@ -821,36 +821,19 @@ class test_permission(Declarative):
 
         # This tests setting truncated to True in the post_callback of
         # permission_find(). The return order in LDAP is not guaranteed
-        # but in practice this is the first entry it finds. This is subject
-        # to change.
+        # so do not check the actual entry.
         dict(
             desc='Search for permissions by attr with a limit of 1 (truncated)',
-            command=('permission_find', [], dict(attrs=u'ipaenabledflag',
-                                                 sizelimit=1)),
+            command=('permission_find', [u'Modify'],
+                     dict(attrs=u'ipaenabledflag', sizelimit=1)),
             expected=dict(
                 count=1,
                 truncated=True,
                 summary=u'1 permission matched',
-                result=[
-                    {
-                        'dn': DN(('cn', 'Modify HBAC rule'),
-                                 api.env.container_permission, api.env.basedn),
-                        'cn': [u'Modify HBAC rule'],
-                        'objectclass': objectclasses.permission,
-                        'member_privilege': [u'HBAC Administrator'],
-                        'memberindirect_role': [u'IT Security Specialist'],
-                        'ipapermright' : [u'write'],
-                        'attrs': [u'servicecategory', u'sourcehostcategory',
-                                  u'cn', u'description', u'ipaenabledflag',
-                                  u'accesstime', u'usercategory',
-                                  u'hostcategory', u'accessruletype',
-                                  u'sourcehost'],
-                        'ipapermtarget': [DN(('ipauniqueid', '*'),
-                                             ('cn', 'hbac'), api.env.basedn)],
-                        'ipapermbindruletype': [u'permission'],
-                        'ipapermlocation': [api.env.basedn],
-                    },
-                ],
+                result=[lambda res:
+                    DN(res['dn']).endswith(DN(api.env.container_permission,
+                                              api.env.basedn)) and
+                    'ipapermission' in res['objectclass']],
             ),
         ),
 
@@ -2367,7 +2350,8 @@ class test_permission_bindtype(Declarative):
 
         dict(
             desc='Search for %r using --bindtype' % permission1,
-            command=('permission_find', [], {'ipapermbindruletype': u'all'}),
+            command=('permission_find', [permission1],
+                     {'ipapermbindruletype': u'all'}),
             expected=dict(
                 count=1,
                 truncated=False,
@@ -2389,6 +2373,18 @@ class test_permission_bindtype(Declarative):
         ),
 
         dict(
+            desc='Search for %r using bad --bindtype' % permission1,
+            command=('permission_find', [permission1],
+                     {'ipapermbindruletype': u'anonymous'}),
+            expected=dict(
+                count=0,
+                truncated=False,
+                summary=u'0 permissions matched',
+                result=[],
+            ),
+        ),
+
+        dict(
             desc='Add zero permissions to %r' % (privilege1),
             command=('privilege_add_permission', [privilege1], {}),
             expected=dict(
-- 
1.8.5.3

From 83c4af0ef13864e570d0b9ca2ac278e2493e2e99 Mon Sep 17 00:00:00 2001
From: Petr Viktorin <pvikt...@redhat.com>
Date: Tue, 25 Feb 2014 17:24:02 +0100
Subject: [PATCH] Allow modifying permissions with ":" in the name

The ":" character will be reserved for default permissions, so that
users cannot create a permission with a name that will later be
added as a default.

Allow the ":" character modifying/deleting permissions*, but not
when creating them. Also do not allow the new name to contain ":"
when renaming.

(* modify/delete have unrelated restrictions on managed permissions)
---
 API.txt                                        | 14 ++++++------
 VERSION                                        |  4 ++--
 ipalib/plugins/permission.py                   | 31 ++++++++++++++++++++++++--
 ipatests/test_xmlrpc/test_permission_plugin.py | 25 +++++++++++++++++++++
 4 files changed, 63 insertions(+), 11 deletions(-)

diff --git a/API.txt b/API.txt
index 5d106338608698a499be3e56a00eb2a220ec87cf..3829763d8a743014c44695d79500fca332263c1f 100644
--- a/API.txt
+++ b/API.txt
@@ -2349,7 +2349,7 @@ command: permission_add
 output: Output('value', <type 'unicode'>, None)
 command: permission_add_member
 args: 1,5,3
-arg: Str('cn', attribute=True, cli_name='name', multivalue=False, pattern='^[-_ a-zA-Z0-9.]+$', primary_key=True, query=True, required=True)
+arg: Str('cn', attribute=True, cli_name='name', multivalue=False, pattern='^[-_ a-zA-Z0-9.:]+$', primary_key=True, query=True, required=True)
 option: Flag('all', autofill=True, cli_name='all', default=False, exclude='webui')
 option: Flag('no_members', autofill=True, default=False, exclude='webui')
 option: Str('privilege*', alwaysask=True, cli_name='privileges', csv=True)
@@ -2360,7 +2360,7 @@ command: permission_add_member
 output: Entry('result', <type 'dict'>, Gettext('A dictionary representing an LDAP entry', domain='ipa', localedir=None))
 command: permission_add_noaci
 args: 1,5,3
-arg: Str('cn', attribute=True, cli_name='name', multivalue=False, pattern='^[-_ a-zA-Z0-9.]+$', primary_key=True, required=True)
+arg: Str('cn', attribute=True, cli_name='name', multivalue=False, pattern='^[-_ a-zA-Z0-9.:]+$', primary_key=True, required=True)
 option: Flag('all', autofill=True, cli_name='all', default=False, exclude='webui', multivalue=False, required=True)
 option: Str('ipapermissiontype', cli_name='ipapermissiontype', multivalue=True, required=True)
 option: Flag('no_members', autofill=True, cli_name='no_members', default=False, exclude='webui', multivalue=False, required=True)
@@ -2371,7 +2371,7 @@ command: permission_add_noaci
 output: Output('value', <type 'unicode'>, None)
 command: permission_del
 args: 1,3,3
-arg: Str('cn', attribute=True, cli_name='name', multivalue=True, pattern='^[-_ a-zA-Z0-9.]+$', primary_key=True, query=True, required=True)
+arg: Str('cn', attribute=True, cli_name='name', multivalue=True, pattern='^[-_ a-zA-Z0-9.:]+$', primary_key=True, query=True, required=True)
 option: Flag('continue', autofill=True, cli_name='continue', default=False)
 option: Flag('force', autofill=True, default=False)
 option: Str('version?', exclude='webui')
@@ -2383,7 +2383,7 @@ command: permission_find
 arg: Str('criteria?', noextrawhitespace=False)
 option: Flag('all', autofill=True, cli_name='all', default=False, exclude='webui')
 option: Str('attrs', attribute=False, autofill=False, cli_name='attrs', multivalue=True, query=True, required=False)
-option: Str('cn', attribute=True, autofill=False, cli_name='name', multivalue=False, pattern='^[-_ a-zA-Z0-9.]+$', primary_key=True, query=True, required=False)
+option: Str('cn', attribute=True, autofill=False, cli_name='name', multivalue=False, pattern='^[-_ a-zA-Z0-9.:]+$', primary_key=True, query=True, required=False)
 option: Str('filter', attribute=False, autofill=False, cli_name='filter', multivalue=True, query=True, required=False)
 option: StrEnum('ipapermbindruletype', attribute=True, autofill=False, cli_name='bindtype', default=u'permission', multivalue=False, query=True, required=False, values=(u'permission', u'all', u'anonymous'))
 option: Str('ipapermdefaultattr', attribute=True, autofill=False, cli_name='defaultattrs', multivalue=True, query=True, required=False)
@@ -2410,7 +2410,7 @@ command: permission_find
 output: Output('truncated', <type 'bool'>, None)
 command: permission_mod
 args: 1,23,3
-arg: Str('cn', attribute=True, cli_name='name', multivalue=False, pattern='^[-_ a-zA-Z0-9.]+$', primary_key=True, query=True, required=True)
+arg: Str('cn', attribute=True, cli_name='name', multivalue=False, pattern='^[-_ a-zA-Z0-9.:]+$', primary_key=True, query=True, required=True)
 option: Str('addattr*', cli_name='addattr', exclude='webui')
 option: Flag('all', autofill=True, cli_name='all', default=False, exclude='webui')
 option: Str('attrs', attribute=False, autofill=False, cli_name='attrs', multivalue=True, required=False)
@@ -2439,7 +2439,7 @@ command: permission_mod
 output: Output('value', <type 'unicode'>, None)
 command: permission_remove_member
 args: 1,5,3
-arg: Str('cn', attribute=True, cli_name='name', multivalue=False, pattern='^[-_ a-zA-Z0-9.]+$', primary_key=True, query=True, required=True)
+arg: Str('cn', attribute=True, cli_name='name', multivalue=False, pattern='^[-_ a-zA-Z0-9.:]+$', primary_key=True, query=True, required=True)
 option: Flag('all', autofill=True, cli_name='all', default=False, exclude='webui')
 option: Flag('no_members', autofill=True, default=False, exclude='webui')
 option: Str('privilege*', alwaysask=True, cli_name='privileges', csv=True)
@@ -2450,7 +2450,7 @@ command: permission_remove_member
 output: Entry('result', <type 'dict'>, Gettext('A dictionary representing an LDAP entry', domain='ipa', localedir=None))
 command: permission_show
 args: 1,5,3
-arg: Str('cn', attribute=True, cli_name='name', multivalue=False, pattern='^[-_ a-zA-Z0-9.]+$', primary_key=True, query=True, required=True)
+arg: Str('cn', attribute=True, cli_name='name', multivalue=False, pattern='^[-_ a-zA-Z0-9.:]+$', primary_key=True, query=True, required=True)
 option: Flag('all', autofill=True, cli_name='all', default=False, exclude='webui')
 option: Flag('no_members', autofill=True, default=False, exclude='webui')
 option: Flag('raw', autofill=True, cli_name='raw', default=False, exclude='webui')
diff --git a/VERSION b/VERSION
index e889bced8a3ad2450555a876236302bb4b58c5db..aff56ad96f08e6030691cdfec73f97fe335c5ea4 100644
--- a/VERSION
+++ b/VERSION
@@ -89,5 +89,5 @@ IPA_DATA_VERSION=20100614120000
 #                                                      #
 ########################################################
 IPA_API_VERSION_MAJOR=2
-IPA_API_VERSION_MINOR=77
-# Last change: pviktori - permissions: multivalued memberof
+IPA_API_VERSION_MINOR=78
+# Last change: pviktori - ":" in permission names
diff --git a/ipalib/plugins/permission.py b/ipalib/plugins/permission.py
index 9193dcaa2eb2c8fd293e851d2a2bb1c380c5bcfd..656b4dbb189166093db6578a35f349a1d5140c75 100644
--- a/ipalib/plugins/permission.py
+++ b/ipalib/plugins/permission.py
@@ -147,6 +147,18 @@ def validate_type(ugettext, typestr):
         return _('"%s" is not a valid permission type') % typestr
 
 
+def _disallow_colon(option):
+    """Given a "cn" option, return a new "cn" option with ':' disallowed
+
+    Used in permission-add and for --rename in permission-mod to prevent user
+    from creating new permissions with ":" in the name.
+    """
+    return option.clone(
+        pattern='^[-_ a-zA-Z0-9.]+$',
+        pattern_errmsg="May only contain letters, numbers, -, _, ., and space",
+    )
+
+
 @register()
 class permission(baseldap.LDAPObject):
     """
@@ -176,8 +188,9 @@ class permission(baseldap.LDAPObject):
             cli_name='name',
             label=_('Permission name'),
             primary_key=True,
-            pattern='^[-_ a-zA-Z0-9.]+$',
-            pattern_errmsg="May only contain letters, numbers, -, _, ., and space",
+            pattern='^[-_ a-zA-Z0-9.:]+$',
+            pattern_errmsg="May only contain letters, numbers, "
+                           "-, _, ., :, and space",
         ),
         StrEnum(
             'ipapermright*',
@@ -812,6 +825,13 @@ def execute(self, *keys, **options):
         self.obj.preprocess_options(options)
         return super(permission_add, self).execute(*keys, **options)
 
+    def get_args(self):
+        for arg in super(permission_add, self).get_args():
+            if arg.name == 'cn':
+                yield _disallow_colon(arg)
+            else:
+                yield arg
+
     def pre_callback(self, ldap, dn, entry, attrs_list, *keys, **options):
         entry['ipapermissiontype'] = ['SYSTEM', 'V2']
         entry['cn'] = list(keys)
@@ -901,6 +921,13 @@ def execute(self, *keys, **options):
             options, return_filter_ops=True)
         return super(permission_mod, self).execute(*keys, **options)
 
+    def get_options(self):
+        for opt in super(permission_mod, self).get_options():
+            if opt.name == 'rename':
+                yield _disallow_colon(opt)
+            else:
+                yield opt
+
     def pre_callback(self, ldap, dn, entry, attrs_list, *keys, **options):
         if 'rename' in options and not options['rename']:
             raise errors.ValidationError(name='rename',
diff --git a/ipatests/test_xmlrpc/test_permission_plugin.py b/ipatests/test_xmlrpc/test_permission_plugin.py
index da48a17a790b9c5cf8a675a9d3d8520fae7af974..aae3fde574810cb3dedcdff904fd833f7ef1a059 100644
--- a/ipatests/test_xmlrpc/test_permission_plugin.py
+++ b/ipatests/test_xmlrpc/test_permission_plugin.py
@@ -245,6 +245,18 @@ class test_permission_negative(Declarative):
         verify_permission_aci_missing(permission1, users_dn),
 
         dict(
+            desc='Try to create permission with : in the name',
+            command=('permission_add', ['bad:' + permission1], dict(
+                    type=u'user',
+                    ipapermright=u'write',
+                )),
+            expected=errors.ValidationError(name='name',
+                error='May only contain letters, numbers, -, _, ., and space'),
+        ),
+
+        verify_permission_aci_missing(permission1, users_dn),
+
+        dict(
             desc='Create %r so we can try breaking it' % permission1,
             command=(
                 'permission_add', [permission1], dict(
@@ -1535,6 +1547,19 @@ class test_permission(Declarative):
                 name='ipapermlocation',
                 error='Entry %s does not exist' % nonexistent_dn)
         ),
+
+        dict(
+            desc='Search for nonexisting permission with ":" in the name',
+            command=(
+                'permission_find', ['doesnotexist:' + permission1], {}
+            ),
+            expected=dict(
+                count=0,
+                truncated=False,
+                summary=u'0 permissions matched',
+                result=[],
+            ),
+        ),
     ]
 
 
-- 
1.8.5.3

From 8f80f3e8ed97f7565bb3fe7bad35fd5ec74a65d7 Mon Sep 17 00:00:00 2001
From: Petr Viktorin <pvikt...@redhat.com>
Date: Thu, 19 Sep 2013 17:41:04 +0200
Subject: [PATCH] Add Object metadata and update plugin for managed permissions

The default read permission is added for Netgroup as an example.

Part of the work for: https://fedorahosted.org/freeipa/ticket/3566
Design: http://www.freeipa.org/page/V3/Managed_Read_permissions
---
 ipalib/plugins/baseldap.py                         |   1 +
 ipalib/plugins/netgroup.py                         |  19 +++
 .../install/plugins/update_managed_permissions.py  | 160 +++++++++++++++++++++
 3 files changed, 180 insertions(+)
 create mode 100644 ipaserver/install/plugins/update_managed_permissions.py

diff --git a/ipalib/plugins/baseldap.py b/ipalib/plugins/baseldap.py
index c4951eb56c044cfa08b617d546d9b9332adc4cc9..6a8b4f822959fd16c074df56695d5ebf4bae7756 100644
--- a/ipalib/plugins/baseldap.py
+++ b/ipalib/plugins/baseldap.py
@@ -469,6 +469,7 @@ class LDAPObject(Object):
     }
     label = _('Entry')
     label_singular = _('Entry')
+    managed_permissions = {}
 
     container_not_found_msg = _('container entry (%(container)s) not found')
     parent_not_found_msg = _('%(parent)s: %(oname)s not found')
diff --git a/ipalib/plugins/netgroup.py b/ipalib/plugins/netgroup.py
index fe27e6cb6e623496f03a78e27b8c5e44b3ea6585..7136c18f90feef6a1bc46a45c0a2b16a1281476f 100644
--- a/ipalib/plugins/netgroup.py
+++ b/ipalib/plugins/netgroup.py
@@ -105,6 +105,25 @@ class netgroup(LDAPObject):
         'memberuser': ('Member', '', 'no_'),
         'memberhost': ('Member', '', 'no_'),
     }
+    managed_permissions = {
+        'System: Read Netgroups': {
+            'replaces_global_anonymous_aci': True,
+            'ipapermbindruletype': 'all',
+            'ipapermright': {'read', 'search', 'compare'},
+            'ipapermdefaultattr': {
+                'cn', 'description', 'hostcategory', 'ipaenabledflag',
+                'ipauniqueid', 'nisdomainname', 'usercategory'
+            },
+        },
+        'System: Read Netgroup Membership': {
+            'replaces_global_anonymous_aci': True,
+            'ipapermbindruletype': 'all',
+            'ipapermright': {'read', 'search', 'compare'},
+            'ipapermdefaultattr': {
+                'externalhost', 'member', 'memberof', 'memberuser'
+            },
+        },
+    }
 
     label = _('Netgroups')
     label_singular = _('Netgroup')
diff --git a/ipaserver/install/plugins/update_managed_permissions.py b/ipaserver/install/plugins/update_managed_permissions.py
new file mode 100644
index 0000000000000000000000000000000000000000..603f3f0b74c97b14be0992d2c110f5bb6cd0e0e6
--- /dev/null
+++ b/ipaserver/install/plugins/update_managed_permissions.py
@@ -0,0 +1,160 @@
+# Authors:
+#   Petr Viktorin <pvikt...@redhat.com>
+#
+# Copyright (C) 2014  Red Hat
+# see file 'COPYING' for use and warranty information
+#
+# This program is free software; you can redistribute it and/or modify
+# it under the terms of the GNU General Public License as published by
+# the Free Software Foundation, either version 3 of the License, or
+# (at your option) any later version.
+#
+# This program is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+# GNU General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License
+# along with this program.  If not, see <http://www.gnu.org/licenses/>.
+
+from ipalib import errors
+from ipapython.dn import DN
+from ipalib.plugable import Registry
+from ipalib.plugins import aci
+from ipalib.plugins.permission import permission
+from ipaserver.plugins.ldap2 import ldap2
+from ipaserver.install.plugins import LAST
+from ipaserver.install.plugins.baseupdate import PostUpdate
+
+
+register = Registry()
+
+
+@register()
+class update_managed_permissions(PostUpdate):
+    """Update managed permissions after an update.
+
+    Update managed permissions according to templates specified in plugins.
+    For read permissions, puts any attributes specified in the legacy
+    Anonymous access ACI in the exclude list when creating the permission.
+    """
+    order = LAST
+
+    def get_anonymous_read_blacklist(self, ldap):
+        """Get the list of attributes from the legacy anonymous access ACI"""
+        aciname = u'Enable Anonymous access'
+        aciprefix = u'none'
+
+        base_entry = ldap.get_entry(self.api.env.basedn, ['aci'])
+
+        acistrs = base_entry.get('aci', [])
+        acilist = aci._convert_strings_to_acis(acistrs)
+        try:
+            rawaci = aci._find_aci_by_name(acilist, aciprefix, aciname)
+        except errors.NotFound:
+            self.log.info('Anonymous ACI not found, using no blacklist')
+            return []
+
+        return rawaci.target['targetattr']['expression']
+
+    def execute(self, **options):
+        ldap = self.api.Backend[ldap2]
+
+        anonymous_read_blacklist = self.get_anonymous_read_blacklist(ldap)
+
+        self.log.info('Anonymous read blacklist: %s', anonymous_read_blacklist)
+
+        for obj in self.api.Object():
+            managed_permissions = getattr(obj, 'managed_permissions', {})
+            if managed_permissions:
+                self.log.info('Updating managed permissions for %s', obj.name)
+            for name, template in managed_permissions.items():
+                assert name.startswith('System:')
+                self.update_permission(ldap,
+                                       obj,
+                                       unicode(name),
+                                       template,
+                                       anonymous_read_blacklist)
+
+        return False, False, ()
+
+    def update_permission(self, ldap, obj, name, template,
+                          anonymous_read_blacklist):
+        """Update the given permission and the corresponding ACI"""
+        dn = self.api.Object[permission].get_dn(name)
+
+        try:
+            attrs_list = self.api.Object[permission].default_attributes
+            entry = ldap.get_entry(dn, attrs_list)
+            is_new = False
+        except errors.NotFound:
+            entry = ldap.make_entry(dn)
+            is_new = True
+
+        self.log.info('Updating managed permission: %s', name)
+        self.update_entry(obj, entry, template,
+                          anonymous_read_blacklist, is_new=is_new)
+
+        if is_new:
+            ldap.add_entry(entry)
+        else:
+            try:
+                ldap.update_entry(entry)
+            except errors.EmptyModlist:
+                self.log.debug('No changes to permission: %s', name)
+                return
+
+        self.log.debug('Updating ACI for managed permission: %s', name)
+
+        self.api.Object[permission].update_aci(entry)
+
+    def update_entry(self, obj, entry, template,
+                     anonymous_read_blacklist, is_new):
+        """Update the given permission Entry (without contacting LDAP)"""
+
+        [name_ava] = entry.dn[0]
+        assert name_ava.attr == 'cn'
+        name = name_ava.value
+        entry.single_value['cn'] = name
+
+        template = dict(template)
+
+        # Common attributes
+        entry['objectclass'] = self.api.Object[permission].object_class
+
+        entry['ipapermissiontype'] = [u'SYSTEM', u'V2', u'MANAGED']
+
+        # Object-specific attributes
+        ldap_filter = ['(objectclass=%s)' % oc
+                       for oc in obj.permission_filter_objectclasses]
+        entry['ipapermtargetfilter'] = ldap_filter
+
+        ipapermlocation = DN(obj.container_dn, self.api.env.basedn)
+        entry.single_value['ipapermlocation'] = ipapermlocation
+
+        # Attributes from template
+        bindruletype = template.pop('ipapermbindruletype')
+        entry.single_value['ipapermbindruletype'] = bindruletype
+
+        entry['ipapermright'] = list(template.pop('ipapermright'))
+
+        # Add to the set of default attributes
+        attributes = set(template.pop('ipapermdefaultattr', ()))
+        attributes.update(entry.get('ipapermdefaultattr', ()))
+        attributes = set(a.lower() for a in attributes)
+        entry['ipapermdefaultattr'] = list(attributes)
+
+        # Exclude attributes filtered from the global read ACI
+        if template.pop('replaces_global_anonymous_aci', False) and is_new:
+            read_blacklist = set(a.lower() for a in anonymous_read_blacklist)
+            read_blacklist &= attributes
+            if read_blacklist:
+                self.log.info('Excluded attributes for %s: %s',
+                              name, ', '.join(read_blacklist))
+                entry['ipapermexcludedattr'] = list(read_blacklist)
+
+        # Sanity check
+        if template:
+            raise ValueError(
+                'Unknown key(s) in managed permission template %s: %s' % (
+                    name, ', '.join(template.keys())))
-- 
1.8.5.3

From 23ba7234d7be5ab952714c1165ab11797371e8b1 Mon Sep 17 00:00:00 2001
From: Petr Viktorin <pvikt...@redhat.com>
Date: Thu, 13 Mar 2014 17:27:08 +0530
Subject: [PATCH] permission plugin: Add 'top' to the list of object classes

The 'top' objectclass is added by DS if not present. On every
update the managed permission updater compared the object_class
list with the state from LDAP, saw that there's an extra 'top'
value, and tried deleting it.

Add 'top' to the list to match the entry in LDAP.
---
 ipalib/plugins/permission.py | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/ipalib/plugins/permission.py b/ipalib/plugins/permission.py
index 656b4dbb189166093db6578a35f349a1d5140c75..9d3179aba02666b9edffee52c11b739ab4c85411 100644
--- a/ipalib/plugins/permission.py
+++ b/ipalib/plugins/permission.py
@@ -167,7 +167,9 @@ class permission(baseldap.LDAPObject):
     container_dn = api.env.container_permission
     object_name = _('permission')
     object_name_plural = _('permissions')
-    object_class = ['groupofnames', 'ipapermission', 'ipapermissionv2']
+    # For use the complete object_class list, including 'top', so
+    # the updater doesn't try to delete 'top' every time.
+    object_class = ['top', 'groupofnames', 'ipapermission', 'ipapermissionv2']
     default_attributes = ['cn', 'member', 'memberof',
         'memberindirect', 'ipapermissiontype', 'objectclass',
         'ipapermdefaultattr', 'ipapermincludedattr', 'ipapermexcludedattr',
-- 
1.8.5.3

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

Reply via email to