On 03/13/2014 02:01 PM, Petr Viktorin wrote:
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.

Rebased again.

--
PetrĀ³
From bdb729f08dc6301896bb4db37772fab69ae1cb72 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 f6229b3ea213094325bfdb36ea244a2410a8ea69 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 65220b6e058aadd635d032748e8eb8ce11b860ea..9d39ac9769c5ded230f26f0c2460f7c162a40c13 100644
--- a/ipalib/plugins/permission.py
+++ b/ipalib/plugins/permission.py
@@ -1173,8 +1173,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)
@@ -1206,22 +1207,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 fca9fc4ba11bcd27315d401218e4901b1d9924ab 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 c4fa982c85db320a9658432998ca77df095e96f2..a5554be8caa101187e231e1afa0a0cd8c64b2af9 100644
--- a/ipatests/test_xmlrpc/test_old_permission_plugin.py
+++ b/ipatests/test_xmlrpc/test_old_permission_plugin.py
@@ -554,31 +554,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 678f9f918d9aea8cd5432f7332275afbac59749b..4d515e695f4465b36d41c98cbaf9799275c834ef 100644
--- a/ipatests/test_xmlrpc/test_permission_plugin.py
+++ b/ipatests/test_xmlrpc/test_permission_plugin.py
@@ -836,36 +836,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']],
             ),
         ),
 
@@ -2830,7 +2813,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,
@@ -2851,6 +2835,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 d14abcd8199f0ad8a058f76a6801803737ecf200 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 8e1f7713ade2b3dc046e9db82fdd6be2d85eec56..edd28626fa7b6540dce803b846f7f16bb8145a0f 100644
--- a/API.txt
+++ b/API.txt
@@ -2350,7 +2350,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)
@@ -2361,7 +2361,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)
@@ -2372,7 +2372,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')
@@ -2384,7 +2384,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('extratargetfilter', attribute=False, autofill=False, cli_name='filter', multivalue=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'))
@@ -2412,7 +2412,7 @@ command: permission_find
 output: Output('truncated', <type 'bool'>, None)
 command: permission_mod
 args: 1,24,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)
@@ -2442,7 +2442,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)
@@ -2453,7 +2453,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 4f01e38c0c3a52ae6293e458fdb4d3e0a14aa8aa..cfe4ecd6715be2fa3a9eaa688f45f69fd84fd476 100644
--- a/VERSION
+++ b/VERSION
@@ -89,5 +89,5 @@ IPA_DATA_VERSION=20100614120000
 #                                                      #
 ########################################################
 IPA_API_VERSION_MAJOR=2
-IPA_API_VERSION_MINOR=78
-# Last change: pviktori - permission extratargetfilter
+IPA_API_VERSION_MINOR=79
+# Last change: pviktori - ":" in permission names
diff --git a/ipalib/plugins/permission.py b/ipalib/plugins/permission.py
index 9d39ac9769c5ded230f26f0c2460f7c162a40c13..7c95cbe29ae179b0eea9062c38c79a2f48f4c65c 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*',
@@ -877,6 +890,13 @@ def execute(self, *keys, **options):
         self.obj.preprocess_options(options, merge_targetfilter=True)
         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)
@@ -966,6 +986,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 4d515e695f4465b36d41c98cbaf9799275c834ef..038fb00991b510c0a3b918d3c46749eaef06d0a9 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='Try to create permission with full and extra target filter',
             command=('permission_add', [permission1], dict(
                     type=u'user',
@@ -1522,6 +1534,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 159f6e7a9ee61acb67dce4157c61fa4f56343271 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 3d92a8b509fd5b925992b8266c5decc0ab8758ce 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 7c95cbe29ae179b0eea9062c38c79a2f48f4c65c..ea2eb4ce690655deb6a87e3cdfa649e711b1e2c9 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