[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]

Re: [Freeipa-devel] [PATCHES] 0322-0327 New permissions system



On 12/02/2013 01:04 PM, Martin Kosek wrote:
On 12/01/2013 11:46 PM, Petr Viktorin wrote:
This seems to work now. Please tell me what I missed.

Design: http://www.freeipa.org/page/V3/Permissions_V2
Ticket: https://fedorahosted.org/freeipa/ticket/4034


0322 Allow sets for initialization of frozenset-typed Param keywords
because my OCD compels me to use sets instead of lists when the order does not
matter.


0323 Allow Declarative test classes to specify the API version
For the next patch, I want to test how the rewrite handles old clients. To make
that easy I made the default API version a testclass attribute


0324 Add tests for permission plugin with older clients
These tests will not pass yet, but comparing this file with the old
test_permission_plugin.py will can serve as a nice summary of API changes. A
summary of the summary:
- Lots of new attributes will be added for output
- The `type` and `subtree` options now interact in a different way: setting one
affects the other. Same with `type`/`filter` and `memberof`/`targetfilter`.
(Some change here was necessary for https://fedorahosted.org/freeipa/ticket/2355)
- Validation will be stricter (and/or done in different order)
- Some error messages will change (hopefully for the better)
- `subtree` must now point to an existing entry
- Permission names may now contain '.' (this is to allow names of DNS
permissions that were previously internal)

P.S. a handy command for listing the changes (once this patch is applied):
git diff ipa-3-3:ipatests/test_xmlrpc/test_permission_plugin.py
ipatests/test_xmlrpc/test_old_permission_plugin.py


0325 Add new permission schema
Introducing the new OIDs


0326 Rewrite the Permission plugin
See the design for what this does.

The new permission plugin does not use aci plugin at all. The plan is to retire
the aci plugin when the time comes to also refactor delegation & selfservice.
It does use ipalib's ACI class, mainly for parsing (needed for
upgrading/showing old ACIs).

The permission-find command is a bit faster than the old one, but still
painfully slow (5s instead of 7s on my box). The good news is that it now
scales with the number of *old* permissions, so as you upgrade it'll get faster.

Tests are updated, including privilege and DNS tests that worked with permissions.


0327 Verify ACIs are added correctly in tests
Right after saying I want to get rid of it, I found a new use for the aci
plugin: an tested code path for getting at ACIs (Declaratrive tests can only
use the API, they don't play well with LDAP connections).
Now we can be sure the ACIs are actually changed when we play with permissions.


Great job!

I read through the code and did few tests, this is what I've found so far:

1) When I tried to move ACI to non-existent DN, I got a general error + the
underlying ACI was gone:

# ipa permission-mod "Write Group Description" --subtree=foo=bar
ipa: ERROR: no such entry


When I then tried to delete it, I got Internal Error:
# ipa permission-del "Write Group Description"
ipa: ERROR: an internal error has occurred

...
/python2.7/site-packages/ipalib/plugins/permission.py", line 681, in pre_callback
[Mon Dec 02 10:49:11.972422 2013] [:error] [pid 14181]
self.obj.remove_aci(entry)
[Mon Dec 02 10:49:11.972426 2013] [:error] [pid 14181]   File
"/usr/lib/python2.7/site-packages/ipalib/plugins/permission.py", line 374, in
remove_aci
[Mon Dec 02 10:49:11.972430 2013] [:error] [pid 14181]
self._replace_aci(permission_entry)
[Mon Dec 02 10:49:11.972434 2013] [:error] [pid 14181]   File
"/usr/lib/python2.7/site-packages/ipalib/plugins/permission.py", line 392, in
_replace_aci
[Mon Dec 02 10:49:11.972438 2013] [:error] [pid 14181]     acidn = acientry.dn
  # pylint: disable=E1103
[Mon Dec 02 10:49:11.972441 2013] [:error] [pid 14181] AttributeError: 'dict'
object has no attribute 'dn'

I think we should add a check for that + at least try to rollback if we fail to
move the ACI.

Fixed. I did in in 2 additional patches for clarity:
- 0331 adds rollback
- 0332 adds the check (you can test the rollback without this applied)

2) In filter check:

+        # Rough filter validation by a search
+        if self.env.in_server and 'ipapermtargetfilter' in options:
+            ldap = self.Backend.ldap2
+            try:
+                ldap.find_entries(
+                    filter=options['ipapermtargetfilter'],
+                    base_dn=self.env.basedn,
+                    size_limit=0)

This is great for starters, but I would make it less costly and only search
with BASE scope and sizelimit==1 to avoid costly, possibly unindexed searches
across whole database.

Agreed, fixed.

3) I see that I can add ALL bind type permission as a member to a privilege:

# ipa permission-show "Write Group Description 2"
   Permission name: Write Group Description 2
   Permissions: write
   Attributes: description
   Bind rule type: all
   Subtree: cn=groups,cn=accounts,dc=example,dc=com
   ACI target DN: cn=*,cn=groups,cn=accounts,dc=example,dc=com
   Type: group

# ipa privilege-add-permission foo --permissions "Write Group Description 2"
   Privilege name: foo
   Description: foo
   Permissions: write group description 2
-----------------------------
Number of permissions added 1
-----------------------------

Is this a bug or do you already plan to fix it later?

Yes, I plan to fix that soon (#4032).
I've modified the patch to allow "permission" only. I'll re-introduce the others when I add the necessary checks.

4) Typo in refactored permission plugin:

+            errors.NotFound('ACI of to permission %s was not found' %
+                            keys[0])

Fixed, thanks for the catch!

--
PetrĀ³

From 4e5cb5f2c61199e90d1a3b5419242d961a5c87cb Mon Sep 17 00:00:00 2001
From: Petr Viktorin <pviktori redhat com>
Date: Thu, 12 Sep 2013 10:46:52 +0200
Subject: [PATCH] Allow sets for initialization of frozenset-typed Param
 keywords

Lists and tuples are already allowed for convenience; it is easier to write
(1, 2, 3) or [1, 2, 3] than frozenset([1, 2, 3]).
This allows the set literal syntax, {1, 2, 3}, as well.
---
 ipalib/parameters.py | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/ipalib/parameters.py b/ipalib/parameters.py
index 9472c90063f3a564ebcc78d88689a6d3d418ffff..757c185655bbe1b586fbf088256891a7e7558876 100644
--- a/ipalib/parameters.py
+++ b/ipalib/parameters.py
@@ -459,7 +459,7 @@ def __init__(self, name, *rules, **kw):
             value = kw.get(key, default)
             if value is not None:
                 if kind is frozenset:
-                    if type(value) in (list, tuple):
+                    if type(value) in (list, tuple, set):
                         value = frozenset(value)
                     elif type(value) is str:
                         value = frozenset([value])
-- 
1.8.3.1

From 56b13f032140effc48c9b43073ef70cba96fe615 Mon Sep 17 00:00:00 2001
From: Petr Viktorin <pviktori redhat com>
Date: Fri, 29 Nov 2013 18:31:32 +0100
Subject: [PATCH] Allow Declarative test classes to specify the API version

This makes it possible to test behavior with older clients.
---
 ipatests/test_xmlrpc/xmlrpc_test.py | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/ipatests/test_xmlrpc/xmlrpc_test.py b/ipatests/test_xmlrpc/xmlrpc_test.py
index 04d69475abba6812dcc20a88eb9273fbce4e852f..21959fbb110572372bd222dc156da8919223d343 100644
--- a/ipatests/test_xmlrpc/xmlrpc_test.py
+++ b/ipatests/test_xmlrpc/xmlrpc_test.py
@@ -241,6 +241,7 @@ class Declarative(XMLRPC_test):
     and must not fail.
     """
 
+    default_version = API_VERSION
     cleanup_commands = tuple()
     tests = tuple()
 
@@ -290,7 +291,7 @@ def test_generator(self):
 
     def check(self, nice, desc, command, expected, extra_check=None):
         (cmd, args, options) = command
-        options.setdefault('version', API_VERSION)
+        options.setdefault('version', self.default_version)
         if cmd not in api.Command:
             raise nose.SkipTest('%r not in api.Command' % cmd)
         if isinstance(expected, errors.PublicError):
-- 
1.8.3.1

From bad4689edf239dc427899e7e676fc6188a3951b4 Mon Sep 17 00:00:00 2001
From: Petr Viktorin <pviktori redhat com>
Date: Fri, 29 Nov 2013 18:32:35 +0100
Subject: [PATCH] Add tests for permission plugin with older clients

These tests use an old API version, which triggers
backwards-compatible behavior in the plugin.
---
 ipatests/test_xmlrpc/test_old_permission_plugin.py | 1124 ++++++++++++++++++++
 1 file changed, 1124 insertions(+)
 create mode 100644 ipatests/test_xmlrpc/test_old_permission_plugin.py

diff --git a/ipatests/test_xmlrpc/test_old_permission_plugin.py b/ipatests/test_xmlrpc/test_old_permission_plugin.py
new file mode 100644
index 0000000000000000000000000000000000000000..cb4a62317116a606fc73127c65961b257628d2bd
--- /dev/null
+++ b/ipatests/test_xmlrpc/test_old_permission_plugin.py
@@ -0,0 +1,1124 @@
+# Authors:
+#   Rob Crittenden <rcritten redhat com>
+#   Pavel Zuna <pzuna redhat com>
+#
+# Copyright (C) 2010  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/>.
+
+"""
+Test the `ipalib/plugins/permission.py` module with old API.
+
+This ensures basic backwards compatibility for code before
+http://www.freeipa.org/page/V3/Permissions_V2
+"""
+
+from ipalib import api, errors
+from ipatests.test_xmlrpc import objectclasses
+from xmlrpc_test import Declarative, fuzzy_digits, fuzzy_uuid
+from ipapython.dn import DN
+
+permission1 = u'testperm'
+permission1_dn = DN(('cn',permission1),
+                    api.env.container_permission,api.env.basedn)
+
+
+permission1_renamed = u'testperm1_rn'
+permission1_renamed_dn = DN(('cn',permission1_renamed),
+                            api.env.container_permission,api.env.basedn)
+
+permission1_renamed_ucase = u'Testperm_RN'
+permission1_renamed_ucase_dn = DN(('cn',permission1_renamed_ucase),
+                            api.env.container_permission,api.env.basedn)
+
+
+permission2 = u'testperm2'
+permission2_dn = DN(('cn',permission2),
+                    api.env.container_permission,api.env.basedn)
+
+permission3 = u'testperm3'
+permission3_dn = DN(('cn',permission3),
+                    api.env.container_permission,api.env.basedn)
+permission3_attributelevelrights = {
+                                     'member': u'rscwo',
+                                     'seealso': u'rscwo',
+                                     'ipapermissiontype': u'rscwo',
+                                     'cn': u'rscwo',
+                                     'businesscategory': u'rscwo',
+                                     'objectclass': u'rscwo',
+                                     'memberof': u'rscwo',
+                                     'aci': u'rscwo',
+                                     'o': u'rscwo',
+                                     'owner': u'rscwo',
+                                     'ou': u'rscwo',
+                                     'targetgroup': u'rscwo',
+                                     'type': u'rscwo',
+                                     'nsaccountlock': u'rscwo',
+                                     'description': u'rscwo',
+                                     'ipapermallowedattr': u'rscwo',
+                                     'ipapermbindruletype': u'rscwo',
+                                     'ipapermdefaultattr': u'rscwo',
+                                     'ipapermexcludedattr': u'rscwo',
+                                     'ipapermlocation': u'rscwo',
+                                     'ipapermright': u'rscwo',
+                                     'ipapermtarget': u'rscwo',
+                                     'ipapermtargetfilter': u'rscwo',
+                                   }
+
+privilege1 = u'testpriv1'
+privilege1_dn = DN(('cn',privilege1),
+                   api.env.container_privilege,api.env.basedn)
+
+invalid_permission1 = u'bad;perm'
+
+users_dn = DN(api.env.container_user, api.env.basedn)
+groups_dn = DN(api.env.container_group, api.env.basedn)
+hbac_dn = DN(api.env.container_hbac, api.env.basedn)
+
+class test_old_permission(Declarative):
+    default_version = u'2.65'
+
+    cleanup_commands = [
+        ('permission_del', [permission1], {}),
+        ('permission_del', [permission2], {}),
+        ('permission_del', [permission3], {}),
+        ('privilege_del', [privilege1], {}),
+    ]
+
+    tests = [
+
+        dict(
+            desc='Try to retrieve non-existent %r' % permission1,
+            command=('permission_show', [permission1], {}),
+            expected=errors.NotFound(
+                reason=u'%s: permission not found' % permission1),
+        ),
+
+
+        dict(
+            desc='Try to update non-existent %r' % permission1,
+            command=('permission_mod', [permission1], dict(permissions=u'all')),
+            expected=errors.NotFound(
+                reason=u'%s: permission not found' % permission1),
+        ),
+
+
+        dict(
+            desc='Try to delete non-existent %r' % permission1,
+            command=('permission_del', [permission1], {}),
+            expected=errors.NotFound(
+                reason=u'%s: permission not found' % permission1),
+        ),
+
+
+        dict(
+            desc='Search for non-existent %r' % permission1,
+            command=('permission_find', [permission1], {}),
+            expected=dict(
+                count=0,
+                truncated=False,
+                summary=u'0 permissions matched',
+                result=[],
+            ),
+        ),
+
+
+        dict(
+            desc='Create %r' % permission1,
+            command=(
+                'permission_add', [permission1], dict(
+                     type=u'user',
+                     permissions=u'write',
+                )
+            ),
+            expected=dict(
+                value=permission1,
+                summary=u'Added permission "%s"' % permission1,
+                result=dict(
+                    dn=permission1_dn,
+                    cn=[permission1],
+                    objectclass=objectclasses.permission,
+                    type=u'user',
+                    permissions=[u'write'],
+                    ipapermbindruletype=[u'permission'],
+                    ipapermissiontype=[u'V2', u'SYSTEM'],
+                    ipapermtarget=[DN('uid=*', users_dn)],
+                    subtree=u'ldap:///%s' % users_dn,
+                ),
+            ),
+        ),
+
+
+        dict(
+            desc='Try to create duplicate %r' % permission1,
+            command=(
+                'permission_add', [permission1], dict(
+                     type=u'user',
+                     permissions=u'write',
+                ),
+            ),
+            expected=errors.DuplicateEntry(
+                message='permission with name "%s" already exists' % permission1),
+        ),
+
+
+        dict(
+            desc='Create %r' % privilege1,
+            command=('privilege_add', [privilege1],
+                dict(description=u'privilege desc. 1')
+            ),
+            expected=dict(
+                value=privilege1,
+                summary=u'Added privilege "%s"' % privilege1,
+                result=dict(
+                    dn=privilege1_dn,
+                    cn=[privilege1],
+                    description=[u'privilege desc. 1'],
+                    objectclass=objectclasses.privilege,
+                ),
+            ),
+        ),
+
+
+        dict(
+            desc='Add permission %r to privilege %r' % (permission1, privilege1),
+            command=('privilege_add_permission', [privilege1],
+                dict(permission=permission1)
+            ),
+            expected=dict(
+                completed=1,
+                failed=dict(
+                    member=dict(
+                        permission=[],
+                    ),
+                ),
+                result={
+                    'dn': privilege1_dn,
+                    'cn': [privilege1],
+                    'description': [u'privilege desc. 1'],
+                    'memberof_permission': [permission1],
+                    'objectclass': objectclasses.privilege,
+                }
+            ),
+        ),
+
+
+        dict(
+            desc='Retrieve %r' % permission1,
+            command=('permission_show', [permission1], {}),
+            expected=dict(
+                value=permission1,
+                summary=None,
+                result={
+                    'dn': permission1_dn,
+                    'cn': [permission1],
+                    'objectclass': objectclasses.permission,
+                    'member_privilege': [privilege1],
+                    'type': u'user',
+                    'permissions': [u'write'],
+                    'ipapermbindruletype': [u'permission'],
+                    'ipapermissiontype': [u'V2', u'SYSTEM'],
+                    'ipapermtarget': [DN('uid=*', users_dn)],
+                    'subtree': u'ldap:///%s' % users_dn,
+                },
+            ),
+        ),
+
+
+        dict(
+            desc='Retrieve %r with --raw' % permission1,
+            command=('permission_show', [permission1], {'raw' : True}),
+            expected=dict(
+                value=permission1,
+                summary=None,
+                result={
+                    'dn': permission1_dn,
+                    'cn': [permission1],
+                    'objectclass': objectclasses.permission,
+                    'member': [privilege1_dn],
+                    'aci': u'(target = "ldap:///%s";)(version 3.0;acl "permission:testperm";allow (write) groupdn = "ldap:///%s";;)' % \
+                        (DN(('uid', '*'), ('cn', 'users'), ('cn', 'accounts'), api.env.basedn),
+                         DN(('cn', 'testperm'), ('cn', 'permissions'), ('cn', 'pbac'), api.env.basedn)),
+                    'ipapermright': [u'write'],
+                    'ipapermbindruletype': [u'permission'],
+                    'ipapermissiontype': [u'V2', u'SYSTEM'],
+                    'ipapermtarget': [DN('uid=*', users_dn)],
+                    'ipapermlocation': [users_dn],
+                },
+            ),
+        ),
+
+
+        dict(
+            desc='Search for %r' % permission1,
+            command=('permission_find', [permission1], {}),
+            expected=dict(
+                count=1,
+                truncated=False,
+                summary=u'1 permission matched',
+                result=[
+                    {
+                        'dn': permission1_dn,
+                        'cn': [permission1],
+                        'objectclass': objectclasses.permission,
+                        'member_privilege': [privilege1],
+                        'type': u'user',
+                        'permissions': [u'write'],
+                        'ipapermbindruletype': [u'permission'],
+                        'ipapermissiontype': [u'V2', u'SYSTEM'],
+                        'ipapermtarget': [DN('uid=*', users_dn)],
+                        'subtree': u'ldap:///%s' % users_dn,
+                    },
+                ],
+            ),
+        ),
+
+
+        dict(
+            desc='Search for %r using --name' % permission1,
+            command=('permission_find', [], {'cn': permission1}),
+            expected=dict(
+                count=1,
+                truncated=False,
+                summary=u'1 permission matched',
+                result=[
+                    {
+                        'dn': permission1_dn,
+                        'cn': [permission1],
+                        'objectclass': objectclasses.permission,
+                        'member_privilege': [privilege1],
+                        'type': u'user',
+                        'permissions': [u'write'],
+                        'ipapermbindruletype': [u'permission'],
+                        'ipapermissiontype': [u'V2', u'SYSTEM'],
+                        'ipapermtarget': [DN('uid=*', users_dn)],
+                        'subtree': u'ldap:///%s' % users_dn,
+                    },
+                ],
+            ),
+        ),
+
+
+        dict(
+            desc='Search for non-existence permission using --name',
+            command=('permission_find', [], {'cn': u'notfound'}),
+            expected=dict(
+                count=0,
+                truncated=False,
+                summary=u'0 permissions matched',
+                result=[],
+            ),
+        ),
+
+
+        dict(
+            desc='Search for %r' % privilege1,
+            command=('permission_find', [privilege1], {}),
+            expected=dict(
+                count=1,
+                truncated=False,
+                summary=u'1 permission matched',
+                result=[
+                    {
+                        'dn': permission1_dn,
+                        'cn': [permission1],
+                        'objectclass': objectclasses.permission,
+                        'member_privilege': [privilege1],
+                        'type': u'user',
+                        'permissions': [u'write'],
+                        'ipapermbindruletype': [u'permission'],
+                        'ipapermissiontype': [u'V2', u'SYSTEM'],
+                        'ipapermtarget': [DN('uid=*', users_dn)],
+                        'subtree': u'ldap:///%s' % users_dn,
+                    },
+                ],
+            ),
+        ),
+
+
+        dict(
+            desc='Search for %r with --raw' % permission1,
+            command=('permission_find', [permission1], {'raw' : True}),
+            expected=dict(
+                count=1,
+                truncated=False,
+                summary=u'1 permission matched',
+                result=[
+                    {
+                        'dn': permission1_dn,
+                        'cn': [permission1],
+                        'objectclass': objectclasses.permission,
+                        'member': [privilege1_dn],
+                        'aci': u'(target = "ldap:///%s";)(version 3.0;acl "permission:testperm";allow (write) groupdn = "ldap:///%s";;)' % \
+                            (DN(('uid', '*'), ('cn', 'users'), ('cn', 'accounts'), api.env.basedn),
+                             DN(('cn', 'testperm'), ('cn', 'permissions'), ('cn', 'pbac'), api.env.basedn)),
+                        'ipapermright': [u'write'],
+                        'ipapermbindruletype': [u'permission'],
+                        'ipapermissiontype': [u'V2', u'SYSTEM'],
+                        'ipapermtarget': [DN('uid=*', users_dn)],
+                        'ipapermlocation': [users_dn],
+                    },
+                ],
+            ),
+        ),
+
+
+        dict(
+            desc='Create %r' % permission2,
+            command=(
+                'permission_add', [permission2], dict(
+                     type=u'user',
+                     permissions=u'write',
+                     setattr=u'owner=cn=test',
+                     addattr=u'owner=cn=test2',
+                )
+            ),
+            expected=dict(
+                value=permission2,
+                summary=u'Added permission "%s"' % permission2,
+                result=dict(
+                    dn=permission2_dn,
+                    cn=[permission2],
+                    objectclass=objectclasses.permission,
+                    type=u'user',
+                    permissions=[u'write'],
+                    owner=[u'cn=test', u'cn=test2'],
+                    ipapermbindruletype=[u'permission'],
+                    ipapermissiontype=[u'V2', u'SYSTEM'],
+                    ipapermtarget=[DN('uid=*', users_dn)],
+                    subtree=u'ldap:///%s' % users_dn,
+                ),
+            ),
+        ),
+
+
+        dict(
+            desc='Search for %r' % permission1,
+            command=('permission_find', [permission1], {}),
+            expected=dict(
+                count=2,
+                truncated=False,
+                summary=u'2 permissions matched',
+                result=[
+                    {
+                        'dn': permission1_dn,
+                        'cn': [permission1],
+                        'objectclass': objectclasses.permission,
+                        'member_privilege': [privilege1],
+                        'type': u'user',
+                        'permissions': [u'write'],
+                        'ipapermbindruletype': [u'permission'],
+                        'ipapermissiontype': [u'V2', u'SYSTEM'],
+                        'ipapermtarget': [DN('uid=*', users_dn)],
+                        'subtree': u'ldap:///%s' % users_dn,
+                    },
+                    {
+                        'dn': permission2_dn,
+                        'cn': [permission2],
+                        'objectclass': objectclasses.permission,
+                        'type': u'user',
+                        'permissions': [u'write'],
+                        'ipapermbindruletype': [u'permission'],
+                        'ipapermissiontype': [u'V2', u'SYSTEM'],
+                        'ipapermtarget': [DN('uid=*', users_dn)],
+                        'subtree': u'ldap:///%s' % users_dn,
+                    },
+                ],
+            ),
+        ),
+
+
+        dict(
+            desc='Search for %r with --pkey-only' % permission1,
+            command=('permission_find', [permission1], {'pkey_only' : True}),
+            expected=dict(
+                count=2,
+                truncated=False,
+                summary=u'2 permissions matched',
+                result=[
+                    {
+                        'dn': permission1_dn,
+                        'cn': [permission1],
+                    },
+                    {
+                        'dn': permission2_dn,
+                        'cn': [permission2],
+                    },
+                ],
+            ),
+        ),
+
+
+        dict(
+            desc='Search by ACI attribute with --pkey-only',
+            command=('permission_find', [], {'pkey_only': True,
+                                             'attrs': [u'krbminpwdlife']}),
+            expected=dict(
+                count=1,
+                truncated=False,
+                summary=u'1 permission matched',
+                result=[
+                    {
+                        'dn': DN(('cn','Modify Group Password Policy'),
+                                 api.env.container_permission, api.env.basedn),
+                        'cn': [u'Modify Group Password Policy'],
+                    },
+                ],
+            ),
+        ),
+
+
+        dict(
+            desc='Search for %r' % privilege1,
+            command=('privilege_find', [privilege1], {}),
+            expected=dict(
+                count=1,
+                truncated=False,
+                summary=u'1 privilege matched',
+                result=[
+                    {
+                        'dn': privilege1_dn,
+                        'cn': [privilege1],
+                        'description': [u'privilege desc. 1'],
+                        'memberof_permission': [permission1],
+                    },
+                ],
+            ),
+        ),
+
+
+        dict(
+            desc='Search for %r with a limit of 1 (truncated)' % permission1,
+            command=('permission_find', [permission1], dict(sizelimit=1)),
+            expected=dict(
+                count=1,
+                truncated=True,
+                summary=u'1 permission matched',
+                result=[
+                    {
+                        'dn': permission1_dn,
+                        'cn': [permission1],
+                        'objectclass': objectclasses.permission,
+                        'member_privilege': [privilege1],
+                        'type': u'user',
+                        'permissions': [u'write'],
+                        'ipapermbindruletype': [u'permission'],
+                        'ipapermissiontype': [u'V2', u'SYSTEM'],
+                        'ipapermtarget': [DN('uid=*', users_dn)],
+                        'subtree': u'ldap:///%s' % users_dn,
+                    },
+                ],
+            ),
+        ),
+
+
+        dict(
+            desc='Search for %r with a limit of 2' % permission1,
+            command=('permission_find', [permission1], dict(sizelimit=2)),
+            expected=dict(
+                count=2,
+                truncated=False,
+                summary=u'2 permissions matched',
+                result=[
+                    {
+                        'dn': permission1_dn,
+                        'cn': [permission1],
+                        'objectclass': objectclasses.permission,
+                        'member_privilege': [privilege1],
+                        'type': u'user',
+                        'permissions': [u'write'],
+                        'ipapermbindruletype': [u'permission'],
+                        'ipapermissiontype': [u'V2', u'SYSTEM'],
+                        'ipapermtarget': [DN('uid=*', users_dn)],
+                        'subtree': u'ldap:///%s' % users_dn,
+                    },
+                    {
+                        'dn': permission2_dn,
+                        'cn': [permission2],
+                        'objectclass': objectclasses.permission,
+                        'type': u'user',
+                        'permissions': [u'write'],
+                        'ipapermbindruletype': [u'permission'],
+                        'ipapermissiontype': [u'V2', u'SYSTEM'],
+                        'ipapermtarget': [DN('uid=*', users_dn)],
+                        'subtree': u'ldap:///%s' % users_dn,
+                    },
+                ],
+            ),
+        ),
+
+
+        # 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.
+        dict(
+            desc='Search for permissions by attr with a limit of 1 (truncated)',
+            command=('permission_find', [], 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)],
+                    },
+                ],
+            ),
+        ),
+
+
+        dict(
+            desc='Update %r' % permission1,
+            command=(
+                'permission_mod', [permission1], dict(
+                    permissions=u'read',
+                    memberof=u'ipausers',
+                    setattr=u'owner=cn=other-test',
+                    addattr=u'owner=cn=other-test2',
+                )
+            ),
+            expected=dict(
+                value=permission1,
+                summary=u'Modified permission "%s"' % permission1,
+                result=dict(
+                    dn=permission1_dn,
+                    cn=[permission1],
+                    objectclass=objectclasses.permission,
+                    member_privilege=[privilege1],
+                    type=u'user',
+                    permissions=[u'read'],
+                    memberof=u'ipausers',
+                    owner=[u'cn=other-test', u'cn=other-test2'],
+                    ipapermbindruletype=[u'permission'],
+                    ipapermissiontype=[u'V2', u'SYSTEM'],
+                    ipapermtarget=[DN('uid=*', users_dn)],
+                    filter=[u'memberOf=%s' % DN('cn=ipausers', groups_dn)],
+                    subtree=u'ldap:///%s' % users_dn,
+                ),
+            ),
+        ),
+
+
+        dict(
+            desc='Retrieve %r to verify update' % permission1,
+            command=('permission_show', [permission1], {}),
+            expected=dict(
+                value=permission1,
+                summary=None,
+                result={
+                    'dn': permission1_dn,
+                    'cn': [permission1],
+                    'objectclass': objectclasses.permission,
+                    'member_privilege': [privilege1],
+                    'type': u'user',
+                    'permissions': [u'read'],
+                    'memberof': u'ipausers',
+                    'ipapermbindruletype': [u'permission'],
+                    'ipapermissiontype': [u'V2', u'SYSTEM'],
+                    'ipapermtarget': [DN('uid=*', users_dn)],
+                    'filter': [u'memberOf=%s' % DN('cn=ipausers', groups_dn)],
+                    'subtree': u'ldap:///%s' % users_dn,
+                },
+            ),
+        ),
+
+
+
+        dict(
+            desc='Try to rename %r to existing permission %r' % (permission1,
+                                                                 permission2),
+            command=(
+                'permission_mod', [permission1], dict(rename=permission2,
+                                                      permissions=u'all',)
+            ),
+            expected=errors.DuplicateEntry(),
+        ),
+
+
+        dict(
+            desc='Try to rename %r to empty name' % (permission1),
+            command=(
+                'permission_mod', [permission1], dict(rename=u'',
+                                                      permissions=u'all',)
+            ),
+            expected=errors.ValidationError(name='rename',
+                                    error=u'New name can not be empty'),
+        ),
+
+
+        dict(
+            desc='Check integrity of original permission %r' % permission1,
+            command=('permission_show', [permission1], {}),
+            expected=dict(
+                value=permission1,
+                summary=None,
+                result={
+                    'dn': permission1_dn,
+                    'cn': [permission1],
+                    'objectclass': objectclasses.permission,
+                    'member_privilege': [privilege1],
+                    'type': u'user',
+                    'permissions': [u'read'],
+                    'memberof': u'ipausers',
+                    'ipapermbindruletype': [u'permission'],
+                    'ipapermissiontype': [u'V2', u'SYSTEM'],
+                    'ipapermtarget': [DN('uid=*', users_dn)],
+                    'filter': [u'memberOf=%s' % DN('cn=ipausers', groups_dn)],
+                    'subtree': u'ldap:///%s' % users_dn,
+                },
+            ),
+        ),
+
+
+        dict(
+            desc='Rename %r to permission %r' % (permission1,
+                                                 permission1_renamed),
+            command=(
+                'permission_mod', [permission1], dict(rename=permission1_renamed,
+                                                      permissions= u'all',)
+            ),
+            expected=dict(
+                value=permission1,
+                summary=u'Modified permission "%s"' % permission1,
+                result={
+                    'dn': permission1_renamed_dn,
+                    'cn': [permission1_renamed],
+                    'objectclass': objectclasses.permission,
+                    'member_privilege': [privilege1],
+                    'type': u'user',
+                    'permissions': [u'all'],
+                    'memberof': u'ipausers',
+                    'ipapermbindruletype': [u'permission'],
+                    'ipapermissiontype': [u'V2', u'SYSTEM'],
+                    'ipapermtarget': [DN('uid=*', users_dn)],
+                    'filter': [u'memberOf=%s' % DN('cn=ipausers', groups_dn)],
+                    'subtree': u'ldap:///%s' % users_dn,
+                },
+            ),
+        ),
+
+
+        dict(
+            desc='Rename %r to permission %r' % (permission1_renamed,
+                                                 permission1_renamed_ucase),
+            command=(
+                'permission_mod', [permission1_renamed], dict(rename=permission1_renamed_ucase,
+                                                      permissions= u'write',)
+            ),
+            expected=dict(
+                value=permission1_renamed,
+                summary=u'Modified permission "%s"' % permission1_renamed,
+                result={
+                    'dn': permission1_renamed_ucase_dn,
+                    'cn': [permission1_renamed_ucase],
+                    'objectclass': objectclasses.permission,
+                    'member_privilege': [privilege1],
+                    'type': u'user',
+                    'permissions': [u'write'],
+                    'memberof': u'ipausers',
+                    'ipapermbindruletype': [u'permission'],
+                    'ipapermissiontype': [u'V2', u'SYSTEM'],
+                    'ipapermtarget': [DN('uid=*', users_dn)],
+                    'filter': [u'memberOf=%s' % DN('cn=ipausers', groups_dn)],
+                    'subtree': u'ldap:///%s' % users_dn,
+                },
+            ),
+        ),
+
+
+        dict(
+            desc='Change %r to a subtree type' % permission1_renamed_ucase,
+            command=(
+                'permission_mod', [permission1_renamed_ucase],
+                dict(subtree=u'ldap:///%s' % DN(('cn', 'accounts'), api.env.basedn),
+                     type=None)
+            ),
+            expected=dict(
+                value=permission1_renamed_ucase,
+                summary=u'Modified permission "%s"' % permission1_renamed_ucase,
+                result=dict(
+                    dn=permission1_renamed_ucase_dn,
+                    cn=[permission1_renamed_ucase],
+                    objectclass=objectclasses.permission,
+                    member_privilege=[privilege1],
+                    subtree=u'ldap:///%s' % DN(('cn', 'accounts'), api.env.basedn),
+                    permissions=[u'write'],
+                    memberof=u'ipausers',
+                    ipapermbindruletype=[u'permission'],
+                    ipapermissiontype=[u'V2', u'SYSTEM'],
+                    filter=[u'memberOf=%s' % DN('cn=ipausers', groups_dn)],
+                ),
+            ),
+        ),
+
+
+        dict(
+            desc='Search for %r using --subtree' % permission1,
+            command=('permission_find', [],
+                     {'subtree': u'ldap:///%s' % DN(('cn', 'accounts'), api.env.basedn)}),
+            expected=dict(
+                count=1,
+                truncated=False,
+                summary=u'1 permission matched',
+                result=[
+                    {
+                        'dn':permission1_renamed_ucase_dn,
+                        'cn':[permission1_renamed_ucase],
+                        'objectclass': objectclasses.permission,
+                        'member_privilege':[privilege1],
+                        'subtree':u'ldap:///%s' % DN(('cn', 'accounts'), api.env.basedn),
+                        'permissions':[u'write'],
+                        'memberof':u'ipausers',
+                        'ipapermbindruletype': [u'permission'],
+                        'ipapermissiontype': [u'V2', u'SYSTEM'],
+                        'filter': [
+                            u'memberOf=%s' % DN('cn=ipausers', groups_dn)],
+
+                    },
+                ],
+            ),
+        ),
+
+
+        dict(
+            desc='Search using nonexistent --subtree',
+            command=('permission_find', [], {'subtree': u'ldap:///foo=bar'}),
+            expected=dict(
+                count=0,
+                truncated=False,
+                summary=u'0 permissions matched',
+                result=[],
+            ),
+        ),
+
+
+        dict(
+            desc='Search using --targetgroup',
+            command=('permission_find', [], {'targetgroup': u'ipausers'}),
+            expected=dict(
+                count=1,
+                truncated=False,
+                summary=u'1 permission matched',
+                result=[
+                    {
+                        'dn': DN(('cn','Add user to default group'),
+                                 api.env.container_permission, api.env.basedn),
+                        'cn': [u'Add user to default group'],
+                        'objectclass': objectclasses.permission,
+                        'member_privilege': [u'User Administrators'],
+                        'attrs': [u'member'],
+                        'targetgroup': u'ipausers',
+                        'memberindirect_role': [u'User Administrator'],
+                        'permissions': [u'write'],
+                        'ipapermbindruletype': [u'permission'],
+                        'ipapermtarget': [DN('cn=ipausers', groups_dn)],
+                    }
+                ],
+            ),
+        ),
+
+
+        dict(
+            desc='Delete %r' % permission1_renamed_ucase,
+            command=('permission_del', [permission1_renamed_ucase], {}),
+            expected=dict(
+                result=dict(failed=u''),
+                value=permission1_renamed_ucase,
+                summary=u'Deleted permission "%s"' % permission1_renamed_ucase,
+            )
+        ),
+
+
+        dict(
+            desc='Try to delete non-existent %r' % permission1,
+            command=('permission_del', [permission1], {}),
+            expected=errors.NotFound(
+                reason=u'%s: permission not found' % permission1),
+        ),
+
+
+        dict(
+            desc='Try to retrieve non-existent %r' % permission1,
+            command=('permission_show', [permission1], {}),
+            expected=errors.NotFound(
+                reason=u'%s: permission not found' % permission1),
+        ),
+
+
+        dict(
+            desc='Try to update non-existent %r' % permission1,
+            command=('permission_mod', [permission1], dict(rename=u'Foo')),
+            expected=errors.NotFound(
+                reason=u'%s: permission not found' % permission1),
+        ),
+
+
+        dict(
+            desc='Delete %r' % permission2,
+            command=('permission_del', [permission2], {}),
+            expected=dict(
+                result=dict(failed=u''),
+                value=permission2,
+                summary=u'Deleted permission "%s"' % permission2,
+            )
+        ),
+
+
+        dict(
+            desc='Search for %r' % permission1,
+            command=('permission_find', [permission1], {}),
+            expected=dict(
+                count=0,
+                truncated=False,
+                summary=u'0 permissions matched',
+                result=[],
+            ),
+        ),
+
+
+        dict(
+            desc='Delete %r' % privilege1,
+            command=('privilege_del', [privilege1], {}),
+            expected=dict(
+                result=dict(failed=u''),
+                value=privilege1,
+                summary=u'Deleted privilege "%s"' % privilege1,
+            )
+        ),
+
+        dict(
+            desc='Try to create permission %r with non-existing memberof' % permission1,
+            command=(
+                'permission_add', [permission1], dict(
+                     memberof=u'nonexisting',
+                     permissions=u'write',
+                )
+            ),
+            expected=errors.NotFound(reason=u'nonexisting: group not found'),
+        ),
+
+        dict(
+            desc='Create memberof permission %r' % permission1,
+            command=(
+                'permission_add', [permission1], dict(
+                     memberof=u'editors',
+                     permissions=u'write',
+                     type=u'user',
+                )
+            ),
+            expected=dict(
+                value=permission1,
+                summary=u'Added permission "%s"' % permission1,
+                result=dict(
+                    dn=permission1_dn,
+                    cn=[permission1],
+                    objectclass=objectclasses.permission,
+                    memberof=u'editors',
+                    permissions=[u'write'],
+                    type=u'user',
+                    ipapermbindruletype=[u'permission'],
+                    ipapermissiontype=[u'V2', u'SYSTEM'],
+                    ipapermtarget=[DN('uid=*', users_dn)],
+                    filter=[u'memberOf=%s' % DN('cn=editors', groups_dn)],
+                    subtree=u'ldap:///%s' % users_dn,
+                ),
+            ),
+        ),
+
+        dict(
+            desc='Try to update non-existent memberof of %r' % permission1,
+            command=('permission_mod', [permission1], dict(
+                memberof=u'nonexisting')),
+            expected=errors.NotFound(reason=u'nonexisting: group not found'),
+        ),
+
+        dict(
+            desc='Update memberof permission %r' % permission1,
+            command=(
+                'permission_mod', [permission1], dict(
+                     memberof=u'admins',
+                )
+            ),
+            expected=dict(
+                value=permission1,
+                summary=u'Modified permission "%s"' % permission1,
+                result=dict(
+                    dn=permission1_dn,
+                    cn=[permission1],
+                    objectclass=objectclasses.permission,
+                    memberof=u'admins',
+                    permissions=[u'write'],
+                    type=u'user',
+                    ipapermbindruletype=[u'permission'],
+                    ipapermissiontype=[u'V2', u'SYSTEM'],
+                    ipapermtarget=[DN('uid=*', users_dn)],
+                    filter=[u'memberOf=%s' % DN('cn=admins', groups_dn)],
+                    subtree=u'ldap:///%s' % users_dn,
+                ),
+            ),
+        ),
+
+        dict(
+            desc='Unset memberof of permission %r' % permission1,
+            command=(
+                'permission_mod', [permission1], dict(
+                     memberof=None,
+                )
+            ),
+            expected=dict(
+                summary=u'Modified permission "%s"' % permission1,
+                value=permission1,
+                result=dict(
+                    dn=permission1_dn,
+                    cn=[permission1],
+                    objectclass=objectclasses.permission,
+                    permissions=[u'write'],
+                    type=u'user',
+                    ipapermbindruletype=[u'permission'],
+                    ipapermissiontype=[u'V2', u'SYSTEM'],
+                    ipapermtarget=[DN('uid=*', users_dn)],
+                    subtree=u'ldap:///%s' % users_dn,
+                ),
+            ),
+        ),
+
+
+        dict(
+            desc='Delete %r' % permission1,
+            command=('permission_del', [permission1], {}),
+            expected=dict(
+                result=dict(failed=u''),
+                value=permission1,
+                summary=u'Deleted permission "%s"' % permission1,
+            )
+        ),
+
+
+        dict(
+            desc='Create targetgroup permission %r' % permission1,
+            command=(
+                'permission_add', [permission1], dict(
+                     targetgroup=u'editors',
+                     permissions=u'write',
+                )
+            ),
+            expected=dict(
+                value=permission1,
+                summary=u'Added permission "%s"' % permission1,
+                result=dict(
+                    dn=permission1_dn,
+                    cn=[permission1],
+                    objectclass=objectclasses.permission,
+                    targetgroup=u'editors',
+                    permissions=[u'write'],
+                    ipapermbindruletype=[u'permission'],
+                    ipapermissiontype=[u'V2', u'SYSTEM'],
+                    ipapermtarget=[DN('cn=editors', groups_dn)],
+                ),
+            ),
+        ),
+
+        dict(
+            desc='Try to create invalid %r' % invalid_permission1,
+            command=('permission_add', [invalid_permission1], dict(
+                     type=u'user',
+                     permissions=u'write',
+                )),
+            expected=errors.ValidationError(name='name',
+                error='May only contain letters, numbers, -, _, ., and space'),
+        ),
+
+        dict(
+            desc='Create %r' % permission3,
+            command=(
+                'permission_add', [permission3], dict(
+                     type=u'user',
+                     permissions=u'write',
+		     attrs=[u'cn']
+                )
+            ),
+            expected=dict(
+                value=permission3,
+                summary=u'Added permission "%s"' % permission3,
+                result=dict(
+                    dn=permission3_dn,
+                    cn=[permission3],
+                    objectclass=objectclasses.permission,
+                    type=u'user',
+                    permissions=[u'write'],
+                    attrs=(u'cn',),
+                    ipapermbindruletype=[u'permission'],
+                    ipapermissiontype=[u'V2', u'SYSTEM'],
+                    ipapermtarget=[DN('uid=*', users_dn)],
+                    subtree=u'ldap:///%s' % users_dn,
+                ),
+            ),
+        ),
+
+        dict(
+            desc='Retrieve %r with --all --rights' % permission3,
+            command=('permission_show', [permission3], {'all' : True, 'rights' : True}),
+            expected=dict(
+                value=permission3,
+                summary=None,
+                result=dict(
+		    dn=permission3_dn,
+                    cn=[permission3],
+                    objectclass=objectclasses.permission,
+                    type=u'user',
+                    attrs=(u'cn',),
+                    permissions=[u'write'],
+                    attributelevelrights=permission3_attributelevelrights,
+                    ipapermbindruletype=[u'permission'],
+                    ipapermissiontype=[u'V2', u'SYSTEM'],
+                    ipapermtarget=[DN('uid=*', users_dn)],
+                    subtree=u'ldap:///%s' % users_dn,
+                ),
+            ),
+        ),
+
+        dict(
+            desc='Modify %r with --all -rights' % permission3,
+            command=('permission_mod', [permission3], {'all' : True, 'rights': True, 'attrs':[u'cn',u'uid']}),
+            expected=dict(
+                value=permission3,
+                summary=u'Modified permission "%s"' % permission3,
+                result=dict(
+		    dn=permission3_dn,
+                    cn=[permission3],
+                    objectclass=objectclasses.permission,
+                    type=u'user',
+                    attrs=(u'cn',u'uid'),
+                    permissions=[u'write'],
+                    attributelevelrights=permission3_attributelevelrights,
+                    ipapermbindruletype=[u'permission'],
+                    ipapermissiontype=[u'V2', u'SYSTEM'],
+                    ipapermtarget=[DN('uid=*', users_dn)],
+                    subtree=u'ldap:///%s' % users_dn,
+                ),
+            ),
+        ),
+    ]
-- 
1.8.3.1

From e3daffaa3a1a74a5fbde499a82524e6e2ab2e04b Mon Sep 17 00:00:00 2001
From: Petr Viktorin <pviktori redhat com>
Date: Wed, 6 Nov 2013 14:14:59 +0100
Subject: [PATCH] Add new permission schema

Part of the work for: https://fedorahosted.org/freeipa/ticket/3566
Design: http://www.freeipa.org/page/V3/Permissions_V2
---
 install/share/60basev3.ldif | 9 +++++++++
 1 file changed, 9 insertions(+)

diff --git a/install/share/60basev3.ldif b/install/share/60basev3.ldif
index 1d913542a51356f4620f1a559ae0409c6dcd5eb7..970d9c80b262f6c28245a640e084aada21a4f057 100644
--- a/install/share/60basev3.ldif
+++ b/install/share/60basev3.ldif
@@ -38,6 +38,14 @@ dn: cn=schema
 attributeTypes: (2.16.840.1.113730.3.8.11.39 NAME 'ipaNTSIDBlacklistOutgoing' DESC 'Extra SIDs filtered out from outgoing MS-PAC' EQUALITY caseIgnoreIA5Match SUBSTR caseIgnoreIA5SubstringsMatch SYNTAX 1.3.6.1.4.1.1466.115.121.1.26 X-ORIGIN 'IPA v3')
 attributeTypes: (2.16.840.1.113730.3.8.11.40 NAME 'ipaUserAuthType' DESC 'Allowed authentication methods' EQUALITY caseIgnoreMatch SYNTAX 1.3.6.1.4.1.1466.115.121.1.15 X-ORIGIN 'IPA v3')
 attributeTypes: (2.16.840.1.113730.3.8.11.41 NAME 'ipaRangeType' DESC 'Range type' EQUALITY caseIgnoreIA5Match SUBSTR caseIgnoreIA5SubstringsMatch SYNTAX 1.3.6.1.4.1.1466.115.121.1.26 X-ORIGIN 'IPA v3' )
+attributeTypes: (2.16.840.1.113730.3.8.11.42 NAME 'ipaPermDefaultAttr' DESC 'IPA permission default attribute' EQUALITY caseIgnoreMatch ORDERING caseIgnoreOrderingMatch SYNTAX 1.3.6.1.4.1.1466.115.121.1.15 X-ORIGIN 'IPA v3' )
+attributeTypes: (2.16.840.1.113730.3.8.11.43 NAME 'ipaPermAllowedAttr' DESC 'IPA permission explicitly allowed attribute' EQUALITY caseIgnoreMatch ORDERING caseIgnoreOrderingMatch SYNTAX 1.3.6.1.4.1.1466.115.121.1.15 X-ORIGIN 'IPA v3' )
+attributeTypes: (2.16.840.1.113730.3.8.11.44 NAME 'ipaPermExcludedAttr' DESC 'IPA permission explicitly excluded attribute' EQUALITY caseIgnoreMatch ORDERING caseIgnoreOrderingMatch SYNTAX 1.3.6.1.4.1.1466.115.121.1.15 X-ORIGIN 'IPA v3' )
+attributeTypes: (2.16.840.1.113730.3.8.11.45 NAME 'ipaPermBindRuleType' DESC 'IPA permission bind rule type' EQUALITY caseExactMatch SYNTAX 1.3.6.1.4.1.1466.115.121.1.15 SINGLE-VALUE X-ORIGIN 'IPA v3' )
+attributeTypes: (2.16.840.1.113730.3.8.11.46 NAME 'ipaPermLocation' DESC 'Location of IPA permission ACI' EQUALITY distinguishedNameMatch SYNTAX 1.3.6.1.4.1.1466.115.121.1.12 SINGLE-VALUE X-ORIGIN 'IPA v3' )
+attributeTypes: (2.16.840.1.113730.3.8.11.47 NAME 'ipaPermRight' DESC 'IPA permission rights' EQUALITY caseIgnoreMatch SYNTAX 1.3.6.1.4.1.1466.115.121.1.15 X-ORIGIN 'IPA v3' )
+attributeTypes: (2.16.840.1.113730.3.8.11.48 NAME 'ipaPermTargetFilter' DESC 'IPA permission target filter' EQUALITY caseIgnoreMatch SYNTAX 1.3.6.1.4.1.1466.115.121.1.15 X-ORIGIN 'IPA v3' )
+attributeTypes: (2.16.840.1.113730.3.8.11.49 NAME 'ipaPermTarget' DESC 'IPA permission target' EQUALITY caseIgnoreMatch SYNTAX 1.3.6.1.4.1.1466.115.121.1.15 X-ORIGIN 'IPA v3' )
 objectClasses: (2.16.840.1.113730.3.8.12.1 NAME 'ipaExternalGroup' SUP top STRUCTURAL MUST ( cn ) MAY ( ipaExternalMember $ memberOf $ description $ owner) X-ORIGIN 'IPA v3' )
 objectClasses: (2.16.840.1.113730.3.8.12.2 NAME 'ipaNTUserAttrs' SUP top AUXILIARY MUST ( ipaNTSecurityIdentifier ) MAY ( ipaNTHash $ ipaNTLogonScript $ ipaNTProfilePath $ ipaNTHomeDirectory $ ipaNTHomeDirectoryDrive ) X-ORIGIN 'IPA v3' )
 objectClasses: (2.16.840.1.113730.3.8.12.3 NAME 'ipaNTGroupAttrs' SUP top AUXILIARY MUST ( ipaNTSecurityIdentifier ) X-ORIGIN 'IPA v3' )
@@ -55,3 +63,4 @@ dn: cn=schema
 objectClasses: (2.16.840.1.113730.3.8.12.17 NAME 'ipaTrustedADDomainRange' SUP ipaIDrange STRUCTURAL MUST ( ipaBaseRID $ ipaNTTrustedDomainSID ) X-ORIGIN 'IPA v3' )
 objectClasses: (2.16.840.1.113730.3.8.12.19 NAME 'ipaUserAuthTypeClass' SUP top AUXILIARY DESC 'Class for authentication methods definition' MAY ipaUserAuthType X-ORIGIN 'IPA v3')
 objectClasses: (2.16.840.1.113730.3.8.12.20 NAME 'ipaUser' AUXILIARY MUST ( uid ) MAY ( userClass ) X-ORIGIN 'IPA v3' )
+objectClasses: (2.16.840.1.113730.3.8.12.21 NAME 'ipaPermissionV2' DESC 'IPA Permission objectclass, version 2' SUP ipaPermission AUXILIARY MAY ( ipaPermDefaultAttr $ ipaPermAllowedAttr $ ipaPermExcludedAttr $ ipaPermBindRuleType $ ipaPermLocation $ ipaPermRight $ ipaPermTargetFilter $ ipaPermTarget ) X-ORIGIN 'IPA v3' )
-- 
1.8.3.1

From af63b97f7cc1d40b74f4784b081deee2a5a84ab0 Mon Sep 17 00:00:00 2001
From: Petr Viktorin <pviktori redhat com>
Date: Wed, 13 Nov 2013 16:31:58 +0100
Subject: [PATCH] Rewrite the Permission plugin

Ticket: https://fedorahosted.org/freeipa/ticket/3566
Design: http://www.freeipa.org/page/V3/Permissions_V2
---
 API.txt                                        |  110 +-
 VERSION                                        |    2 +-
 ipalib/capabilities.py                         |    6 +-
 ipalib/plugins/dns.py                          |    2 +-
 ipalib/plugins/permission.py                   | 1393 +++++++++++++++---------
 ipatests/test_xmlrpc/objectclasses.py          |    1 +
 ipatests/test_xmlrpc/test_dns_plugin.py        |    3 +
 ipatests/test_xmlrpc/test_permission_plugin.py |  857 +++++++++++++--
 ipatests/test_xmlrpc/test_privilege_plugin.py  |   26 +-
 9 files changed, 1705 insertions(+), 695 deletions(-)
 rewrite ipalib/plugins/permission.py (70%)

diff --git a/API.txt b/API.txt
index f1103e0ff671ac82fd71302633db808be5249c00..a4886bc2fb9387873234357978d310627a6a5509 100644
--- a/API.txt
+++ b/API.txt
@@ -2228,27 +2228,33 @@ command: passwd
 output: Output('summary', (<type 'unicode'>, <type 'NoneType'>), None)
 output: Output('value', <type 'unicode'>, None)
 command: permission_add
-args: 1,13,3
-arg: Str('cn', attribute=True, cli_name='name', multivalue=False, pattern='^[-_ a-zA-Z0-9]+$', primary_key=True, required=True)
+args: 1,19,3
+arg: Str('cn', attribute=True, cli_name='name', multivalue=False, pattern='^[-_ a-zA-Z0-9.]+$', primary_key=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', alwaysask=True, attribute=True, autofill=False, cli_name='attrs', csv=True, multivalue=True, query=False, required=False)
-option: Str('filter', alwaysask=True, attribute=True, autofill=False, cli_name='filter', multivalue=False, query=False, required=False)
-option: Str('memberof', alwaysask=True, attribute=True, autofill=False, cli_name='memberof', multivalue=False, query=False, required=False)
+option: Str('attrs', attribute=False, cli_name='attrs', multivalue=True, required=False)
+option: Str('filter', attribute=False, cli_name='filter', multivalue=True, required=False)
+option: Str('ipapermallowedattr', attribute=True, cli_name='attrs', multivalue=True, required=False)
+option: StrEnum('ipapermbindruletype', attribute=True, autofill=True, cli_name='bindtype', default=u'permission', multivalue=False, required=True, values=(u'permission',))
+option: DNOrURL('ipapermlocation', alwaysask=True, attribute=True, autofill=False, cli_name='subtree', multivalue=False, query=False, required=False)
+option: StrEnum('ipapermright', attribute=True, cli_name='permissions', multivalue=True, required=False, values=(u'read', u'search', u'compare', u'write', u'add', u'delete', u'all'))
+option: DNParam('ipapermtarget', attribute=True, cli_name='target', multivalue=False, required=False)
+option: Str('ipapermtargetfilter', attribute=True, cli_name='filter', multivalue=False, required=False)
+option: Str('memberof', alwaysask=True, attribute=False, autofill=False, cli_name='memberof', multivalue=False, query=False, required=False)
 option: Flag('no_members', autofill=True, default=False, exclude='webui')
-option: Str('permissions', attribute=True, cli_name='permissions', csv=True, multivalue=True, required=True)
+option: Str('permissions', attribute=False, cli_name='permissions', multivalue=True, required=False)
 option: Flag('raw', autofill=True, cli_name='raw', default=False, exclude='webui')
 option: Str('setattr*', cli_name='setattr', exclude='webui')
-option: Str('subtree', alwaysask=True, attribute=True, autofill=False, cli_name='subtree', multivalue=False, query=False, required=False)
-option: Str('targetgroup', alwaysask=True, attribute=True, autofill=False, cli_name='targetgroup', multivalue=False, query=False, required=False)
-option: StrEnum('type', alwaysask=True, attribute=True, autofill=False, cli_name='type', multivalue=False, query=False, required=False, values=(u'user', u'group', u'host', u'service', u'hostgroup', u'netgroup', u'dnsrecord'))
+option: Str('subtree', attribute=False, cli_name='subtree', multivalue=True, required=False)
+option: Str('targetgroup', alwaysask=True, attribute=False, autofill=False, cli_name='targetgroup', multivalue=False, query=False, required=False)
+option: StrEnum('type', alwaysask=True, attribute=False, autofill=False, cli_name='type', multivalue=False, query=False, required=False, values=(u'user', u'group', u'host', u'service', u'hostgroup', u'netgroup', u'dnsrecord'))
 option: Str('version?', exclude='webui')
 output: Entry('result', <type 'dict'>, Gettext('A dictionary representing an LDAP entry', domain='ipa', localedir=None))
 output: Output('summary', (<type 'unicode'>, <type 'NoneType'>), None)
 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)
@@ -2258,19 +2264,32 @@ command: permission_add_member
 output: Output('failed', <type 'dict'>, None)
 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', cli_name='name', multivalue=False, pattern=None, primary_key=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: StrEnum('permissiontype?', values=(u'SYSTEM',))
-option: Flag('raw', autofill=True, cli_name='raw', default=False, exclude='webui')
-option: Str('version?', exclude='webui')
+args: 1,18,3
+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=False)
+option: Str('attrs', attribute=False, cli_name='attrs', multivalue=True, required=False)
+option: Str('filter', attribute=False, cli_name='filter', multivalue=True, required=False)
+option: Str('ipapermallowedattr', attribute=True, cli_name='attrs', multivalue=True, required=False)
+option: StrEnum('ipapermbindruletype', attribute=True, autofill=True, cli_name='bindtype', default=u'permission', multivalue=False, required=False, values=(u'permission',))
+option: Str('ipapermissiontype', cli_name='ipapermissiontype', multivalue=True, required=True)
+option: DNOrURL('ipapermlocation', alwaysask=True, attribute=True, autofill=False, cli_name='subtree', multivalue=False, query=False, required=False)
+option: StrEnum('ipapermright', attribute=True, cli_name='permissions', multivalue=True, required=False, values=(u'read', u'search', u'compare', u'write', u'add', u'delete', u'all'))
+option: DNParam('ipapermtarget', attribute=True, cli_name='target', multivalue=False, required=False)
+option: Str('ipapermtargetfilter', attribute=True, cli_name='filter', multivalue=False, required=False)
+option: Str('memberof', alwaysask=True, attribute=False, autofill=False, cli_name='memberof', multivalue=False, query=False, required=False)
+option: Flag('no_members', autofill=True, cli_name='no_members', default=False, exclude='webui', multivalue=False, required=False)
+option: Str('permissions', attribute=False, cli_name='permissions', multivalue=True, required=False)
+option: Flag('raw', autofill=True, cli_name='raw', default=False, exclude='webui', multivalue=False, required=False)
+option: Str('subtree', attribute=False, cli_name='subtree', multivalue=True, required=False)
+option: Str('targetgroup', alwaysask=True, attribute=False, autofill=False, cli_name='targetgroup', multivalue=False, query=False, required=False)
+option: StrEnum('type', alwaysask=True, attribute=False, autofill=False, cli_name='type', multivalue=False, query=False, required=False, values=(u'user', u'group', u'host', u'service', u'hostgroup', u'netgroup', u'dnsrecord'))
+option: Str('version', cli_name='version', exclude='webui', multivalue=False, required=False)
 output: Entry('result', <type 'dict'>, Gettext('A dictionary representing an LDAP entry', domain='ipa', localedir=None))
 output: Output('summary', (<type 'unicode'>, <type 'NoneType'>), None)
 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')
@@ -2278,52 +2297,64 @@ command: permission_del
 output: Output('summary', (<type 'unicode'>, <type 'NoneType'>), None)
 output: Output('value', <type 'unicode'>, None)
 command: permission_find
-args: 1,15,4
+args: 1,21,4
 arg: Str('criteria?', noextrawhitespace=False)
 option: Flag('all', autofill=True, cli_name='all', default=False, exclude='webui')
-option: Str('attrs', attribute=True, autofill=False, cli_name='attrs', csv=True, 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('filter', attribute=True, autofill=False, cli_name='filter', multivalue=False, query=True, required=False)
-option: Str('memberof', attribute=True, autofill=False, cli_name='memberof', multivalue=False, query=True, required=False)
+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('filter', attribute=False, autofill=False, cli_name='filter', multivalue=True, query=True, required=False)
+option: Str('ipapermallowedattr', attribute=True, autofill=False, cli_name='attrs', 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',))
+option: DNOrURL('ipapermlocation', attribute=True, autofill=False, cli_name='subtree', multivalue=False, query=True, required=False)
+option: StrEnum('ipapermright', attribute=True, autofill=False, cli_name='permissions', multivalue=True, query=True, required=False, values=(u'read', u'search', u'compare', u'write', u'add', u'delete', u'all'))
+option: DNParam('ipapermtarget', attribute=True, autofill=False, cli_name='target', multivalue=False, query=True, required=False)
+option: Str('ipapermtargetfilter', attribute=True, autofill=False, cli_name='filter', multivalue=False, query=True, required=False)
+option: Str('memberof', attribute=False, autofill=False, cli_name='memberof', multivalue=False, query=True, required=False)
 option: Flag('no_members', autofill=True, default=False, exclude='webui')
-option: Str('permissions', attribute=True, autofill=False, cli_name='permissions', csv=True, multivalue=True, query=True, required=False)
+option: Str('permissions', attribute=False, autofill=False, cli_name='permissions', multivalue=True, query=True, required=False)
 option: Flag('pkey_only?', autofill=True, default=False)
 option: Flag('raw', autofill=True, cli_name='raw', default=False, exclude='webui')
 option: Int('sizelimit?', autofill=False, minvalue=0)
-option: Str('subtree', attribute=True, autofill=False, cli_name='subtree', multivalue=False, query=True, required=False)
-option: Str('targetgroup', attribute=True, autofill=False, cli_name='targetgroup', multivalue=False, query=True, required=False)
+option: Str('subtree', attribute=False, autofill=False, cli_name='subtree', multivalue=True, query=True, required=False)
+option: Str('targetgroup', attribute=False, autofill=False, cli_name='targetgroup', multivalue=False, query=True, required=False)
 option: Int('timelimit?', autofill=False, minvalue=0)
-option: StrEnum('type', attribute=True, autofill=False, cli_name='type', multivalue=False, query=True, required=False, values=(u'user', u'group', u'host', u'service', u'hostgroup', u'netgroup', u'dnsrecord'))
+option: StrEnum('type', attribute=False, autofill=False, cli_name='type', multivalue=False, query=True, required=False, values=(u'user', u'group', u'host', u'service', u'hostgroup', u'netgroup', u'dnsrecord'))
 option: Str('version?', exclude='webui')
 output: Output('count', <type 'int'>, None)
 output: ListOfEntries('result', (<type 'list'>, <type 'tuple'>), Gettext('A list of LDAP entries', domain='ipa', localedir=None))
 output: Output('summary', (<type 'unicode'>, <type 'NoneType'>), None)
 output: Output('truncated', <type 'bool'>, None)
 command: permission_mod
-args: 1,16,3
-arg: Str('cn', attribute=True, cli_name='name', multivalue=False, pattern='^[-_ a-zA-Z0-9]+$', primary_key=True, query=True, required=True)
+args: 1,22,3
+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=True, autofill=False, cli_name='attrs', csv=True, multivalue=True, required=False)
+option: Str('attrs', attribute=False, autofill=False, cli_name='attrs', multivalue=True, required=False)
 option: Str('delattr*', cli_name='delattr', exclude='webui')
-option: Str('filter', attribute=True, autofill=False, cli_name='filter', multivalue=False, required=False)
-option: Str('memberof', attribute=True, autofill=False, cli_name='memberof', multivalue=False, required=False)
+option: Str('filter', attribute=False, autofill=False, cli_name='filter', multivalue=True, required=False)
+option: Str('ipapermallowedattr', attribute=True, autofill=False, cli_name='attrs', multivalue=True, required=False)
+option: StrEnum('ipapermbindruletype', attribute=True, autofill=False, cli_name='bindtype', default=u'permission', multivalue=False, required=False, values=(u'permission',))
+option: DNOrURL('ipapermlocation', attribute=True, autofill=False, cli_name='subtree', multivalue=False, required=False)
+option: StrEnum('ipapermright', attribute=True, autofill=False, cli_name='permissions', multivalue=True, required=False, values=(u'read', u'search', u'compare', u'write', u'add', u'delete', u'all'))
+option: DNParam('ipapermtarget', attribute=True, autofill=False, cli_name='target', multivalue=False, required=False)
+option: Str('ipapermtargetfilter', attribute=True, autofill=False, cli_name='filter', multivalue=False, required=False)
+option: Str('memberof', attribute=False, autofill=False, cli_name='memberof', multivalue=False, required=False)
 option: Flag('no_members', autofill=True, default=False, exclude='webui')
-option: Str('permissions', attribute=True, autofill=False, cli_name='permissions', csv=True, multivalue=True, required=False)
+option: Str('permissions', attribute=False, autofill=False, cli_name='permissions', multivalue=True, required=False)
 option: Flag('raw', autofill=True, cli_name='raw', default=False, exclude='webui')
-option: Str('rename', cli_name='rename', multivalue=False, pattern='^[-_ a-zA-Z0-9]+$', primary_key=True, required=False)
+option: Str('rename', cli_name='rename', multivalue=False, pattern='^[-_ a-zA-Z0-9.]+$', primary_key=True, required=False)
 option: Flag('rights', autofill=True, default=False)
 option: Str('setattr*', cli_name='setattr', exclude='webui')
-option: Str('subtree', attribute=True, autofill=False, cli_name='subtree', multivalue=False, required=False)
-option: Str('targetgroup', attribute=True, autofill=False, cli_name='targetgroup', multivalue=False, required=False)
-option: StrEnum('type', attribute=True, autofill=False, cli_name='type', multivalue=False, required=False, values=(u'user', u'group', u'host', u'service', u'hostgroup', u'netgroup', u'dnsrecord'))
+option: Str('subtree', attribute=False, autofill=False, cli_name='subtree', multivalue=True, required=False)
+option: Str('targetgroup', attribute=False, autofill=False, cli_name='targetgroup', multivalue=False, required=False)
+option: StrEnum('type', attribute=False, autofill=False, cli_name='type', multivalue=False, required=False, values=(u'user', u'group', u'host', u'service', u'hostgroup', u'netgroup', u'dnsrecord'))
 option: Str('version?', exclude='webui')
 output: Entry('result', <type 'dict'>, Gettext('A dictionary representing an LDAP entry', domain='ipa', localedir=None))
 output: Output('summary', (<type 'unicode'>, <type 'NoneType'>), None)
 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)
@@ -2334,7 +2365,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')
@@ -3871,3 +3902,4 @@ command: user_unlock
 output: Output('value', <type 'unicode'>, None)
 capability: messages 2.52
 capability: optional_uid_params 2.54
+capability: permissions2 2.69
diff --git a/VERSION b/VERSION
index e7d7bc3eab38c57cd9c5b24f13c27a234b9c6f03..6ead76c68b5973496811604ac1793e7539432009 100644
--- a/VERSION
+++ b/VERSION
@@ -89,4 +89,4 @@ IPA_DATA_VERSION=20100614120000
 #                                                      #
 ########################################################
 IPA_API_VERSION_MAJOR=2
-IPA_API_VERSION_MINOR=70
+IPA_API_VERSION_MINOR=71
diff --git a/ipalib/capabilities.py b/ipalib/capabilities.py
index 6fcc436d5e00a58430c76aac6b3ebaf976ea23a8..b60a570cf422c9f167d7776e6f8450ca802f3b6d 100644
--- a/ipalib/capabilities.py
+++ b/ipalib/capabilities.py
@@ -40,7 +40,11 @@
     # a user with UID=999. With the capability, these parameters are optional
     # and 999 really means 999.
     # https://fedorahosted.org/freeipa/ticket/2886
-    optional_uid_params=u'2.54'
+    optional_uid_params=u'2.54',
+
+    # permissions2: Reworked permission system
+    # http://www.freeipa.org/page/V3/Permissions_V2
+    permissions2=u'2.69',
 )
 
 
diff --git a/ipalib/plugins/dns.py b/ipalib/plugins/dns.py
index 07523dc72466892f0e7d5fdd9261024d0e898548..19811d7f2eaef4e54481d316268bb7ad66acf7a8 100644
--- a/ipalib/plugins/dns.py
+++ b/ipalib/plugins/dns.py
@@ -2030,7 +2030,7 @@ def execute(self, *keys, **options):
 
         permission_name = self.obj.permission_name(keys[-1])
         permission = api.Command['permission_add_noaci'](permission_name,
-                         permissiontype=u'SYSTEM'
+                         ipapermissiontype=u'SYSTEM'
                      )['result']
 
         update = {}
diff --git a/ipalib/plugins/permission.py b/ipalib/plugins/permission.py
dissimilarity index 70%
index 0f1fe667f6a8b550b8af9d865224efca408621f1..c365dd08261213be3b0395bde012337f27eea4c0 100644
--- a/ipalib/plugins/permission.py
+++ b/ipalib/plugins/permission.py
@@ -1,527 +1,866 @@
-# Authors:
-#   Rob Crittenden <rcritten redhat com>
-#
-# Copyright (C) 2010  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.plugins.baseldap import *
-from ipalib import api, _, ngettext
-from ipalib import Flag, Str, StrEnum
-from ipalib.plugable import Registry
-from ipalib.request import context
-from ipalib import errors
-from ipapython.dn import DN, EditableDN
-
-__doc__ = _("""
-Permissions
-
-A permission enables fine-grained delegation of rights. A permission is
-a human-readable form of a 389-ds Access Control Rule, or instruction (ACI).
-A permission grants the right to perform a specific task such as adding a
-user, modifying a group, etc.
-
-A permission may not contain other permissions.
-
-* A permission grants access to read, write, add or delete.
-* A privilege combines similar permissions (for example all the permissions
-  needed to add a user).
-* A role grants a set of privileges to users, groups, hosts or hostgroups.
-
-A permission is made up of a number of different parts:
-
-1. The name of the permission.
-2. The target of the permission.
-3. The rights granted by the permission.
-
-Rights define what operations are allowed, and may be one or more
-of the following:
-1. write - write one or more attributes
-2. read - read one or more attributes
-3. add - add a new entry to the tree
-4. delete - delete an existing entry
-5. all - all permissions are granted
-
-Read permission is granted for most attributes by default so the read
-permission is not expected to be used very often.
-
-Note the distinction between attributes and entries. The permissions are
-independent, so being able to add a user does not mean that the user will
-be editable.
-
-There are a number of allowed targets:
-1. type: a type of object (user, group, etc).
-2. memberof: a member of a group or hostgroup
-3. filter: an LDAP filter
-4. subtree: an LDAP filter specifying part of the LDAP DIT. This is a
-   super-set of the "type" target.
-5. targetgroup: grant access to modify a specific group (such as granting
-   the rights to manage group membership)
-
-EXAMPLES:
-
- Add a permission that grants the creation of users:
-   ipa permission-add --type=user --permissions=add "Add Users"
-
- Add a permission that grants the ability to manage group membership:
-   ipa permission-add --attrs=member --permissions=write --type=group "Manage Group Members"
-""")
-
-ACI_PREFIX=u"permission"
-
-register = Registry()
-
-output_params = (
-    Str('ipapermissiontype',
-        label=_('Permission Type'),
-    ),
-    Str('aci',
-        label=_('ACI'),
-    ),
-)
-
-def filter_options(options, keys):
-    """Return a dict that includes entries from `options` that are in `keys`
-
-    example:
-    >>> filtered = filter_options({'a': 1, 'b': 2, 'c': 3}, ['a', 'c'])
-    >>> filtered == {'a': 1, 'c': 3}
-    True
-    """
-    return dict((k, options[k]) for k in keys if k in options)
-
-
- register()
-class permission(LDAPObject):
-    """
-    Permission object.
-    """
-    container_dn = api.env.container_permission
-    object_name = _('permission')
-    object_name_plural = _('permissions')
-    object_class = ['groupofnames', 'ipapermission']
-    default_attributes = ['cn', 'member', 'memberof',
-        'memberindirect', 'ipapermissiontype',
-    ]
-    aci_attributes = ['aci', 'group', 'permissions', 'attrs', 'type',
-        'filter', 'subtree', 'targetgroup', 'memberof',
-    ]
-    attribute_members = {
-        'member': ['privilege'],
-        'memberindirect': ['role'],
-    }
-    rdn_is_primary_key = True
-
-    label = _('Permissions')
-    label_singular = _('Permission')
-
-    takes_params = (
-        Str('cn',
-            cli_name='name',
-            label=_('Permission name'),
-            primary_key=True,
-            pattern='^[-_ a-zA-Z0-9]+$',
-            pattern_errmsg="May only contain letters, numbers, -, _, and space",
-        ),
-        Str('permissions+',
-            cli_name='permissions',
-            label=_('Permissions'),
-            doc=_('Permissions to grant ' \
-                '(read, write, add, delete, all)'),
-            csv=True,
-        ),
-        Str('attrs*',
-            cli_name='attrs',
-            label=_('Attributes'),
-            doc=_('Attributes to which the permission applies'),
-            csv=True,
-            normalizer=lambda value: value.lower(),
-            flags=('ask_create'),
-        ),
-        StrEnum('type?',
-            cli_name='type',
-            label=_('Type'),
-            doc=_('Type of IPA object (user, group, host, hostgroup, service, netgroup, dns)'),
-            values=(u'user', u'group', u'host', u'service', u'hostgroup', u'netgroup', u'dnsrecord',),
-            flags=('ask_create'),
-        ),
-        Str('memberof?',
-            cli_name='memberof',
-            label=_('Member of group'),  # FIXME: Does this label make sense?
-            doc=_('Target members of a group'),
-            flags=('ask_create'),
-        ),
-        Str('filter?',
-            cli_name='filter',
-            label=_('Filter'),
-            doc=_('Legal LDAP filter (e.g. ou=Engineering)'),
-            flags=('ask_create'),
-        ),
-        Str('subtree?',
-            cli_name='subtree',
-            label=_('Subtree'),
-            doc=_('Subtree to apply permissions to'),
-            flags=('ask_create'),
-        ),
-        Str('targetgroup?',
-            cli_name='targetgroup',
-            label=_('Target group'),
-            doc=_('User group to apply permissions to'),
-            flags=('ask_create'),
-        ),
-    )
-
-    # Don't allow SYSTEM permissions to be modified or removed
-    def check_system(self, ldap, dn, *keys):
-        try:
-            (dn, entry_attrs) = ldap.get_entry(dn, ['ipapermissiontype'])
-        except errors.NotFound:
-            self.handle_not_found(*keys)
-        if 'ipapermissiontype' in entry_attrs:
-            if 'SYSTEM' in entry_attrs['ipapermissiontype']:
-                return False
-        return True
-
-    def filter_aci_attributes(self, options):
-        """Return option dictionary that only includes ACI attributes"""
-        return dict((k, v) for k, v in options.items() if
-            k in self.aci_attributes)
-
-
- register()
-class permission_add(LDAPCreate):
-    __doc__ = _('Add a new permission.')
-
-    msg_summary = _('Added permission "%(value)s"')
-    has_output_params = LDAPCreate.has_output_params + output_params
-
-    def pre_callback(self, ldap, dn, entry_attrs, attrs_list, *keys, **options):
-        assert isinstance(dn, DN)
-        # Test the ACI before going any further
-        opts = self.obj.filter_aci_attributes(options)
-        opts['test'] = True
-        opts['permission'] = keys[-1]
-        opts['aciprefix'] = ACI_PREFIX
-        self.api.Command.aci_add(keys[-1], **opts)
-
-        # Clear the aci attributes out of the permission entry
-        for o in options:
-            try:
-                if o not in ['objectclass']:
-                    del entry_attrs[o]
-            except:
-                pass
-        return dn
-
-    def post_callback(self, ldap, dn, entry_attrs, *keys, **options):
-        assert isinstance(dn, DN)
-        # Now actually add the aci.
-        opts = self.obj.filter_aci_attributes(options)
-        opts['test'] = False
-        opts['permission'] = keys[-1]
-        opts['aciprefix'] = ACI_PREFIX
-        try:
-            result = self.api.Command.aci_add(keys[-1], **opts)['result']
-            for attr in self.obj.aci_attributes:
-                if attr in result:
-                    entry_attrs[attr] = result[attr]
-        except errors.InvalidSyntax, e:
-            # A syntax error slipped past our attempt at validation, clean up
-            self.api.Command.permission_del(keys[-1])
-            raise e
-        except Exception, e:
-            # Something bad happened, clean up as much as we can and return
-            # that error
-            try:
-                self.api.Command.permission_del(keys[-1])
-            except Exception, ignore:
-                pass
-            try:
-                self.api.Command.aci_del(keys[-1], aciprefix=ACI_PREFIX)
-            except Exception, ignore:
-                pass
-            raise e
-        return dn
-
-
- register()
-class permission_add_noaci(LDAPCreate):
-    __doc__ = _('Add a system permission without an ACI')
-
-    msg_summary = _('Added permission "%(value)s"')
-    has_output_params = LDAPCreate.has_output_params + output_params
-    NO_CLI = True
-
-    takes_options = (
-        StrEnum('permissiontype?',
-            label=_('Permission type'),
-            values=(u'SYSTEM',),
-        ),
-    )
-
-    def get_args(self):
-        # do not validate system permission names
-        yield self.obj.primary_key.clone(pattern=None, pattern_errmsg=None)
-
-    def get_options(self):
-        for option in super(permission_add_noaci, self).get_options():
-            # filter out ACI options
-            if option.name in self.obj.aci_attributes:
-                continue
-            yield option
-
-    def pre_callback(self, ldap, dn, entry_attrs, attrs_list, *keys, **options):
-        assert isinstance(dn, DN)
-        permission_type = options.get('permissiontype')
-        if permission_type:
-            entry_attrs['ipapermissiontype'] = [ permission_type ]
-        return dn
-
-
- register()
-class permission_del(LDAPDelete):
-    __doc__ = _('Delete a permission.')
-
-    msg_summary = _('Deleted permission "%(value)s"')
-
-    takes_options = LDAPDelete.takes_options + (
-        Flag('force',
-             label=_('Force'),
-             flags=['no_option', 'no_output'],
-             doc=_('force delete of SYSTEM permissions'),
-        ),
-    )
-
-    def pre_callback(self, ldap, dn, *keys, **options):
-        assert isinstance(dn, DN)
-        if not options.get('force') and not self.obj.check_system(ldap, dn, *keys):
-            raise errors.ACIError(
-                info=_('A SYSTEM permission may not be removed'))
-        # remove permission even when the underlying ACI is missing
-        try:
-            self.api.Command.aci_del(keys[-1], aciprefix=ACI_PREFIX)
-        except errors.NotFound:
-            pass
-        return dn
-
-
- register()
-class permission_mod(LDAPUpdate):
-    __doc__ = _('Modify a permission.')
-
-    msg_summary = _('Modified permission "%(value)s"')
-    has_output_params = LDAPUpdate.has_output_params + output_params
-
-    def pre_callback(self, ldap, dn, entry_attrs, attrs_list, *keys, **options):
-        assert isinstance(dn, DN)
-        if not self.obj.check_system(ldap, dn, *keys):
-            raise errors.ACIError(
-                info=_('A SYSTEM permission may not be modified'))
-
-        # check if permission is in LDAP
-        try:
-            (dn, attrs) = ldap.get_entry(dn, attrs_list)
-        except errors.NotFound:
-            self.obj.handle_not_found(*keys)
-
-        # when renaming permission, check if the target permission does not
-        # exists already. Then, make changes to underlying ACI
-        if 'rename' in options:
-            if options['rename']:
-                try:
-                    try:
-                        new_dn = EditableDN(dn)
-                        new_dn[0]['cn'] # assure the first RDN has cn as it's type
-                    except (IndexError, KeyError), e:
-                        raise ValueError("expected dn starting with 'cn=' but got '%s'" % dn)
-                    new_dn[0].value = options['rename']
-                    entry = ldap.get_entry(new_dn, attrs_list)
-                    raise errors.DuplicateEntry()
-                except errors.NotFound:
-                    pass    # permission may be renamed, continue
-            else:
-                raise errors.ValidationError(
-                    name='rename', error=_('New name can not be empty'))
-
-        opts = self.obj.filter_aci_attributes(options)
-        setattr(context, 'aciupdate', False)
-        # If there are no options left we don't need to do anything to the
-        # underlying ACI.
-        if len(opts) > 0:
-            opts['permission'] = keys[-1]
-            opts['aciprefix'] = ACI_PREFIX
-            self.api.Command.aci_mod(keys[-1], **opts)
-            setattr(context, 'aciupdate', True)
-
-        # Clear the aci attributes out of the permission entry
-        for o in self.obj.aci_attributes:
-            try:
-                del entry_attrs[o]
-            except:
-                pass
-
-        return dn
-
-    def exc_callback(self, keys, options, exc, call_func, *call_args, **call_kwargs):
-        if call_func.func_name == 'update_entry':
-            if isinstance(exc, errors.EmptyModlist):
-                aciupdate = getattr(context, 'aciupdate')
-                if aciupdate:
-                    return
-        raise exc
-
-    def post_callback(self, ldap, dn, entry_attrs, *keys, **options):
-        assert isinstance(dn, DN)
-        # rename the underlying ACI after the change to permission
-        cn = keys[-1]
-
-        if 'rename' in options:
-            self.api.Command.aci_mod(cn,aciprefix=ACI_PREFIX,
-                        permission=options['rename'])
-
-            self.api.Command.aci_rename(cn, aciprefix=ACI_PREFIX,
-                        newname=options['rename'])
-
-            cn = options['rename']     # rename finished
-
-        # all common options to permission-mod and show need to be listed here
-        common_options = filter_options(options, ['all', 'raw', 'rights'])
-        result = self.api.Command.permission_show(cn, **common_options)['result']
-
-        for r in result:
-            if not r.startswith('member_'):
-                entry_attrs[r] = result[r]
-        return dn
-
-
- register()
-class permission_find(LDAPSearch):
-    __doc__ = _('Search for permissions.')
-
-    msg_summary = ngettext(
-        '%(count)d permission matched', '%(count)d permissions matched', 0
-    )
-    has_output_params = LDAPSearch.has_output_params + output_params
-
-    def post_callback(self, ldap, entries, truncated, *args, **options):
-
-        # There is an option/param overlap: "cn" must be passed as "aciname"
-        # to aci-find. Besides that we don't need cn anymore so pop it
-        aciname = options.pop('cn', None)
-
-        pkey_only = options.pop('pkey_only', False)
-        if not pkey_only:
-            for entry in entries:
-                (dn, attrs) = entry
-                try:
-                    common_options = filter_options(options, ['all', 'raw'])
-                    aci = self.api.Command.aci_show(attrs['cn'][0],
-                        aciprefix=ACI_PREFIX, **common_options)['result']
-
-                    # copy information from respective ACI to permission entry
-                    for attr in self.obj.aci_attributes:
-                        if attr in aci:
-                            attrs[attr] = aci[attr]
-                except errors.NotFound:
-                    self.debug('ACI not found for %s' % attrs['cn'][0])
-        if truncated:
-            # size/time limit met, no need to search acis
-            return truncated
-
-        if 'sizelimit' in options:
-            max_entries = options['sizelimit']
-        else:
-            config = ldap.get_ipa_config()[1]
-            max_entries = config['ipasearchrecordslimit']
-
-        # Now find all the ACIs that match. Once we find them, add any that
-        # aren't already in the list along with their permission info.
-
-        opts = self.obj.filter_aci_attributes(options)
-        if aciname:
-            opts['aciname'] = aciname
-        opts['aciprefix'] = ACI_PREFIX
-        # permission ACI attribute is needed
-        aciresults = self.api.Command.aci_find(*args, **opts)
-        truncated = truncated or aciresults['truncated']
-        results = aciresults['result']
-
-        for aci in results:
-            found = False
-            if 'permission' in aci:
-                for entry in entries:
-                    (dn, attrs) = entry
-                    if aci['permission'] == attrs['cn'][0]:
-                        found = True
-                        break
-                if not found:
-                    common_options = filter_options(options, ['all', 'raw'])
-                    permission = self.api.Command.permission_show(
-                        aci['permission'], **common_options)['result']
-                    dn = permission['dn']
-                    del permission['dn']
-                    if pkey_only:
-                        pk = self.obj.primary_key.name
-                        new_entry = ldap.make_entry(dn, {pk: permission[pk]})
-                    else:
-                        new_entry = ldap.make_entry(dn, permission)
-
-                    if (dn, permission) not in entries:
-                       if len(entries) < max_entries:
-                           entries.append(new_entry)
-                       else:
-                           truncated = True
-                           break
-        return truncated
-
-
- register()
-class permission_show(LDAPRetrieve):
-    __doc__ = _('Display information about a permission.')
-
-    has_output_params = LDAPRetrieve.has_output_params + output_params
-    def post_callback(self, ldap, dn, entry_attrs, *keys, **options):
-        assert isinstance(dn, DN)
-        try:
-            common_options = filter_options(options, ['all', 'raw'])
-            aci = self.api.Command.aci_show(keys[-1], aciprefix=ACI_PREFIX,
-                **common_options)['result']
-            for attr in self.obj.aci_attributes:
-                if attr in aci:
-                    entry_attrs[attr] = aci[attr]
-        except errors.NotFound:
-            self.debug('ACI not found for %s' % entry_attrs['cn'][0])
-        if options.get('rights', False) and options.get('all', False):
-            # The ACI attributes are just broken-out components of aci so
-            # the rights should all match it.
-            for attr in self.obj.aci_attributes:
-                entry_attrs['attributelevelrights'][attr] = entry_attrs['attributelevelrights']['aci']
-        return dn
-
-
- register()
-class permission_add_member(LDAPAddMember):
-    """
-    Add members to a permission.
-    """
-    NO_CLI = True
-
-
- register()
-class permission_remove_member(LDAPRemoveMember):
-    """
-    Remove members from a permission.
-    """
-    NO_CLI = True
+# Authors:
+#   Petr Viktorin <pviktori redhat com>
+#
+# Copyright (C) 2013  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/>.
+
+import re
+
+from ipalib.plugins import baseldap
+from ipalib import errors
+from ipalib.parameters import Str, StrEnum, DNParam, Flag
+from ipalib import api, _, ngettext
+from ipalib.plugable import Registry
+from ipalib.capabilities import client_has_capability
+from ipalib.aci import ACI
+from ipapython.dn import DN
+from ipalib.request import context
+
+__doc__ = _("""
+Permissions
+""" + """
+A permission enables fine-grained delegation of rights. A permission is
+a human-readable wrapper around a 389-ds Access Control Rule,
+or instruction (ACI).
+A permission grants the right to perform a specific task such as adding a
+user, modifying a group, etc.
+""" + """
+A permission may not contain other permissions.
+""" + """
+* A permission grants access to read, write, add, delete, read, search,
+  or compare.
+* A privilege combines similar permissions (for example all the permissions
+  needed to add a user).
+* A role grants a set of privileges to users, groups, hosts or hostgroups.
+""" + """
+A permission is made up of a number of different parts:
+
+1. The name of the permission.
+2. The target of the permission.
+3. The rights granted by the permission.
+""" + """
+Rights define what operations are allowed, and may be one or more
+of the following:
+1. write - write one or more attributes
+2. read - read one or more attributes
+3. search - search on one or more attributes
+4. compare - compare one or more attributes
+5. add - add a new entry to the tree
+6. delete - delete an existing entry
+7. all - all permissions are granted
+""" + """
+Note the distinction between attributes and entries. The permissions are
+independent, so being able to add a user does not mean that the user will
+be editable.
+""" + """
+There are a number of allowed targets:
+1. subtree: a DN; the permission applies to the subtree under this DN
+2. target filter: an LDAP filter
+3. target: DN with possible wildcards, specifies entries permission applies to
+""" + """
+Additionally, there are the following convenience options.
+Setting one of these options will set the corresponding attribute(s).
+1. type: a type of object (user, group, etc); sets subtree and target filter.
+2. memberof: apply to members of a group; sets target filter
+3. targetgroup: grant access to modify a specific group (such as granting
+   the rights to manage group membership); sets target.
+""" + """
+EXAMPLES:
+""" + """
+ Add a permission that grants the creation of users:
+   ipa permission-add --type=user --permissions=add "Add Users"
+""" + """
+ Add a permission that grants the ability to manage group membership:
+   ipa permission-add --attrs=member --permissions=write --type=group "Manage Group Members"
+""")
+
+register = Registry()
+
+VALID_OBJECT_TYPES = (u'user', u'group', u'host', u'service', u'hostgroup',
+                      u'netgroup', u'dnsrecord',)
+
+_DEPRECATED_OPTION_ALIASES = {
+    'permissions': 'ipapermright',
+    'attrs': 'ipapermallowedattr',
+    'filter': 'ipapermtargetfilter',
+    'subtree': 'ipapermlocation',
+}
+
+KNOWN_FLAGS = {'SYSTEM', 'V2'}
+
+output_params = (
+    Str('aci',
+        label=_('ACI'),
+    ),
+)
+
+
+def strip_ldap_prefix(uri):
+    prefix = 'ldap:///'
+    if not uri.startswith(prefix):
+        raise ValueError('%r does not start with %r' % (uri, prefix))
+    return uri[len(prefix):]
+
+
+class DNOrURL(DNParam):
+    """DN parameter that allows, and strips, a "ldap:///"; prefix on input
+
+    Used for ``subtree`` to maintain backward compatibility.
+    """
+
+    def _convert_scalar(self, value, index=None):
+        if isinstance(value, basestring) and value.startswith('ldap:///'):
+            value = strip_ldap_prefix(value)
+        return super(DNOrURL, self)._convert_scalar(value, index=index)
+
+
+ register()
+class permission(baseldap.LDAPObject):
+    """
+    Permission object.
+    """
+    container_dn = api.env.container_permission
+    object_name = _('permission')
+    object_name_plural = _('permissions')
+    object_class = ['groupofnames', 'ipapermission', 'ipapermissionv2']
+    default_attributes = ['cn', 'member', 'memberof',
+        'memberindirect', 'ipapermissiontype', 'objectclass',
+        'ipapermdefaultattr', 'ipapermallowedattr', 'ipapermexcludedattr',
+        'ipapermbindruletype', 'ipapermlocation', 'ipapermright',
+        'ipapermtargetfilter', 'ipapermtarget'
+    ]
+    attribute_members = {
+        'member': ['privilege'],
+        'memberindirect': ['role'],
+    }
+    rdn_is_primary_key = True
+
+    label = _('Permissions')
+    label_singular = _('Permission')
+
+    takes_params = (
+        Str('cn',
+            cli_name='name',
+            label=_('Permission name'),
+            primary_key=True,
+            pattern='^[-_ a-zA-Z0-9.]+$',
+            pattern_errmsg="May only contain letters, numbers, -, _, ., and space",
+        ),
+        StrEnum(
+            'ipapermright*',
+            cli_name='permissions',
+            label=_('Permissions'),
+            doc=_('Rights to grant '
+                  '(read, search, compare, write, add, delete, all)'),
+            values=(u'read', u'search', u'compare',
+                    u'write', u'add', u'delete', u'all'),
+        ),
+        Str('ipapermallowedattr*',
+            cli_name='attrs',
+            label=_('Attributes'),
+            doc=_('Attributes to which the permission applies'),
+        ),
+        StrEnum(
+            'ipapermbindruletype',
+            cli_name='bindtype',
+            label=_('Bind rule type'),
+            doc=_('Bind rule type'),
+            autofill=True,
+            values=(u'permission',),
+            default=u'permission',
+        ),
+        DNOrURL(
+            'ipapermlocation?',
+            cli_name='subtree',
+            label=_('Subtree'),
+            doc=_('Subtree to apply permissions to'),
+            flags={'ask_create'},
+        ),
+        Str(
+            'ipapermtargetfilter?',
+            cli_name='filter',
+            label=_('ACI target filter'),
+            doc=_('ACI target filter'),
+        ),
+
+        DNParam(
+            'ipapermtarget?',
+            cli_name='target',
+            label=_('ACI target DN'),
+            flags={'no_option'}
+        ),
+
+        Str('memberof?',
+            label=_('Member of group'),  # FIXME: Does this label make sense?
+            doc=_('Target members of a group (sets targetfilter)'),
+            flags={'ask_create', 'virtual_attribute'},
+        ),
+        Str('targetgroup?',
+            label=_('Target group'),
+            doc=_('User group to apply permissions to (sets target)'),
+            flags={'ask_create', 'virtual_attribute'},
+        ),
+        StrEnum(
+            'type?',
+            label=_('Type'),
+            doc=_('Type of IPA object (sets subtree and filter)'),
+            values=VALID_OBJECT_TYPES,
+            flags={'ask_create', 'virtual_attribute'},
+        ),
+    ) + tuple(
+        Str(old_name + '*',
+            doc=_('Deprecated; use %s' % new_name),
+            flags={'no_option', 'virtual_attribute'})
+        for old_name, new_name in _DEPRECATED_OPTION_ALIASES.items()
+    )
+
+    def reject_system(self, entry):
+        """Raise if permission entry has unknown flags, or is a SYSTEM perm"""
+        flags = entry.get('ipapermissiontype', [])
+        for flag in flags:
+            if flag not in KNOWN_FLAGS:
+                raise errors.ACIError(
+                    info=_('Permission with unknown flag %s may not be '
+                            'modified or removed') % flag)
+        if list(flags) == [u'SYSTEM']:
+            raise errors.ACIError(
+                info=_('A SYSTEM permission may not be modified or removed'))
+
+    def postprocess_result(self, entry, options):
+        """Update a permission entry for output (in place)
+
+        :param entry: The entry to update
+        :param options:
+            Command options. Contains keys such as ``raw``, ``all``,
+            ``pkey_only``, ``version``.
+        """
+        if not options.get('raw') and not options.get('pkey_only'):
+            ipapermtargetfilter = entry.single_value.get('ipapermtargetfilter',
+                                                         '')
+            ipapermtarget = entry.single_value.get('ipapermtarget')
+            ipapermlocation = entry.single_value.get('ipapermlocation')
+
+            # memberof
+            match = re.match('^\(memberof=(.*)\)$', ipapermtargetfilter, re.I)
+            if match:
+                dn = DN(match.group(1))
+                if dn[1:] == DN(self.api.Object.group.container_dn,
+                                self.api.env.basedn)[:] and dn[0].attr == 'cn':
+                    entry.single_value['memberof'] = dn[0].value
+
+            # targetgroup
+            if ipapermtarget:
+                dn = DN(ipapermtarget)
+                if (dn[1:] == DN(self.api.Object.group.container_dn,
+                                self.api.env.basedn)[:] and
+                        dn[0].attr == 'cn' and dn[0].value != '*'):
+                    entry.single_value['targetgroup'] = dn[0].value
+
+            # type
+            if ipapermtarget and ipapermlocation:
+                for objname in VALID_OBJECT_TYPES:
+                    obj = self.api.Object[objname]
+                    wantdn = DN(obj.container_dn, self.api.env.basedn)
+                    if DN(ipapermlocation) == wantdn:
+                        targetdn = DN(
+                            (obj.rdn_attribute or obj.primary_key.name, '*'),
+                            obj.container_dn,
+                            self.api.env.basedn)
+                        if ipapermtarget == targetdn:
+                            entry.single_value['type'] = objname
+                        break
+
+            # old output names
+            if not client_has_capability(options['version'], 'permissions2'):
+                for old_name, new_name in _DEPRECATED_OPTION_ALIASES.items():
+                    if new_name in entry:
+                        entry[old_name] = entry[new_name]
+                        del entry[new_name]
+
+        rights = entry.get('attributelevelrights')
+        if rights:
+            rights['memberof'] = rights['ipapermtargetfilter']
+            rights['targetgroup'] = rights['ipapermtarget']
+
+            type_rights = set(rights['ipapermtarget'])
+            type_rights.intersection_update(rights['ipapermlocation'])
+            rights['type'] = ''.join(sorted(type_rights,
+                                            key=rights['ipapermtarget'].index))
+
+            if not client_has_capability(options['version'], 'permissions2'):
+                for old_name, new_name in _DEPRECATED_OPTION_ALIASES.items():
+                    if new_name in entry:
+                        rights[old_name] = rights[new_name]
+                        del rights[new_name]
+
+        if options.get('raw'):
+            # Retreive the ACI from LDAP to ensure we get the real thing
+            acientry, acistring = self._get_aci_entry_and_string(entry)
+            entry.single_value['aci'] = acistring
+
+        if not client_has_capability(options['version'], 'permissions2'):
+            # Legacy clients expect some attributes as a single value
+            for attr in 'type', 'targetgroup', 'memberof', 'aci':
+                if attr in entry:
+                    entry[attr] = entry.single_value[attr]
+            if 'subtree' in entry:
+                # Legacy clients expect subtree as a URL
+                dn = entry.single_value['subtree']
+                entry['subtree'] = u'ldap:///%s' % dn
+            if 'filter' in entry:
+                # Legacy clients expect filter without parentheses
+                new_filter = []
+                for flt in entry['filter']:
+                    assert flt[0] == '(' and flt[-1] == ')'
+                    new_filter.append(flt[1:-1])
+                entry['filter'] = new_filter
+
+    def make_aci(self, entry):
+        """Make an ACI string from the given permission entry"""
+
+        aci = ACI()
+        name = entry.single_value['cn']
+        aci.name = 'permission:%s' % name
+        ipapermtarget = entry.single_value.get('ipapermtarget')
+        if ipapermtarget:
+            aci.set_target('ldap:///%s' % ipapermtarget)
+        ipapermtargetfilter = entry.single_value.get('ipapermtargetfilter')
+        if ipapermtargetfilter:
+            aci.set_target_filter(ipapermtargetfilter)
+
+        ipapermbindruletype = entry.single_value.get('ipapermbindruletype',
+                                                     'permission')
+        if ipapermbindruletype == 'permission':
+            dn = DN(('cn', name), self.container_dn, self.api.env.basedn)
+            aci.set_bindrule('groupdn = "ldap:///%s";' % dn)
+        elif ipapermbindruletype == 'all':
+            aci.set_bindrule('userdn = "ldap:///all";')
+        elif ipapermbindruletype == 'anonymous':
+            aci.set_bindrule('userdn = "ldap:///anyone";')
+        else:
+            raise ValueError(ipapermbindruletype)
+        aci.permissions = entry['ipapermright']
+        aci.set_target_attr(entry.get('ipapermallowedattr', []))
+
+        return aci.export_to_string()
+
+    def add_aci(self, permission_entry):
+        """Add the ACI coresponding to the given permission entry"""
+        ldap = self.api.Backend.ldap2
+        acistring = self.make_aci(permission_entry)
+        location = permission_entry.single_value.get('ipapermlocation',
+                                                     self.api.env.basedn)
+
+        self.log.debug('Adding ACI %r to %s' % (acistring, location))
+        entry = ldap.get_entry(location, ['aci'])
+        entry.setdefault('aci', []).append(acistring)
+        ldap.update_entry(entry)
+
+    def remove_aci(self, permission_entry):
+        """Remove the ACI corresponding to the given permission entry"""
+        self._replace_aci(permission_entry)
+
+    def update_aci(self, permission_entry, old_name=None):
+        """Update the ACI corresponding to the given permission entry"""
+        new_acistring = self.make_aci(permission_entry)
+        self._replace_aci(permission_entry, old_name, new_acistring)
+
+    def _replace_aci(self, permission_entry, old_name=None, new_acistring=None):
+        """Replace ACI corresponding to permission_entry
+
+        :param old_name: the old name of the permission, if different from new
+        :param new_acistring: new ACI string; if None the ACI is just deleted
+        """
+        ldap = self.api.Backend.ldap2
+        acientry, acistring = self._get_aci_entry_and_string(
+            permission_entry, old_name, notfound_ok=True)
+
+        # (pylint thinks `acientry` is just a dict, but it's an LDAPEntry)
+        acidn = acientry.dn  # pylint: disable=E1103
+
+        if acistring is not None:
+            self.log.debug('Removing ACI %r from %s' % (acistring, acidn))
+            acientry['aci'].remove(acistring)
+        if new_acistring:
+            self.log.debug('Adding ACI %r to %s' % (new_acistring, acidn))
+            acientry['aci'].append(new_acistring)
+        try:
+            ldap.update_entry(acientry)
+        except errors.EmptyModlist:
+            self.log.info('No changes to ACI')
+
+    def _get_aci_entry_and_string(self, permission_entry, name=None,
+                                  notfound_ok=False):
+        """Get the entry and ACI corresponding to the permission entry
+
+        :param name: The name of the permission, or None for the cn
+        :param notfound_ok:
+            If true, (acientry, None) will be returned on missing ACI, rather
+            than raising exception
+        """
+        ldap = self.api.Backend.ldap2
+        if name is None:
+            name = permission_entry.single_value['cn']
+        location = permission_entry.single_value.get('ipapermlocation',
+                                                     self.api.env.basedn)
+        wanted_aciname = 'permission:%s' % name
+
+        try:
+            acientry = ldap.get_entry(location, ['aci'])
+        except errors.NotFound:
+            acientry = {}
+        acis = acientry.get('aci', ())
+        for acistring in acis:
+            aci = ACI(acistring)
+            if aci.name == wanted_aciname:
+                return acientry, acistring
+        else:
+            if notfound_ok:
+                return acientry, None
+            raise errors.NotFound(
+                reason=_('The ACI for permission %(name)s was not found '
+                         'in %(dn)s ') % {'name': name, 'dn': location})
+
+    def upgrade_permission(self, entry, target_entry=None,
+                           output_only=False):
+        """Upgrade the given permission entry to V2, in-place
+
+        The entry is only upgraded if it is a plain old-style permission,
+        that is, it has no flags set.
+
+        :param target_entry:
+            If given, ``target_entry`` is filled from information taken
+            from the ACI corresponding to ``entry``.
+            If None, ``entry`` itself is filled
+        :param output_only:
+            If true, the flags are not updated to V2.
+            Used for the -find and -show commands.
+        """
+        if entry.get('ipapermissiontype'):
+            # Only convert old-style, non-SYSTEM permissions -- i.e. no flags
+            return
+        base, acistring = self._get_aci_entry_and_string(entry)
+
+        if not target_entry:
+            target_entry = entry
+
+        # The DN of old permissions is always basedn
+        # (pylint thinks `base` is just a dict, but it's an LDAPEntry)
+        assert base.dn == self.api.env.basedn, base  # pylint: disable=E1103
+
+        aci = ACI(acistring)
+
+        if 'targetattr' in aci.target:
+            target_entry['ipapermallowedattr'] = (
+                aci.target['targetattr']['expression'])
+        if 'target' in aci.target:
+            target_entry.single_value['ipapermtarget'] = DN(strip_ldap_prefix(
+                aci.target['target']['expression']))
+        if 'targetfilter' in aci.target:
+            target_entry.single_value['ipapermtargetfilter'] = unicode(
+                aci.target['targetfilter']['expression'])
+        if aci.bindrule['expression'] == 'ldap:///all':
+            target_entry.single_value['ipapermbindruletype'] = u'all'
+        elif aci.bindrule['expression'] == 'ldap:///anyone':
+            target_entry.single_value['ipapermbindruletype'] = u'anonymous'
+        else:
+            target_entry.single_value['ipapermbindruletype'] = u'permission'
+        target_entry['ipapermright'] = aci.permissions
+        if 'targetattr' in aci.target:
+            target_entry['ipapermallowedattr'] = [
+                unicode(a) for a in aci.target['targetattr']['expression']]
+
+        if not output_only:
+            target_entry['ipapermissiontype'] = ['SYSTEM', 'V2']
+
+        if 'ipapermissionv2' not in entry['objectclass']:
+            target_entry['objectclass'] = list(entry['objectclass']) + [
+                u'ipapermissionv2']
+
+        # Make sure we're not losing *any info* by the upgrade
+        new_acistring = self.make_aci(target_entry)
+        if not ACI(new_acistring).isequal(aci):
+            raise ValueError('Cannot convert ACI, %r != %r' % (new_acistring,
+                                                               acistring))
+
+    def preprocess_options(self, options):
+        """Preprocess options (in-place)"""
+
+        if 'subtree' in options:
+            if isinstance(options['subtree'], (list, tuple)):
+                [options['subtree']] = options['subtree']
+            try:
+                options['subtree'] = strip_ldap_prefix(options['subtree'])
+            except ValueError:
+                raise errors.ValidationError(
+                    name='subtree',
+                    error='does not start with "ldap:///";')
+
+        # Handle old options
+        for old_name, new_name in _DEPRECATED_OPTION_ALIASES.items():
+            if old_name in options:
+                if client_has_capability(options['version'], 'permissions2'):
+                    raise errors.ValidationError(
+                        name=old_name,
+                        error=_('option was renamed; use %s') % new_name)
+                if new_name in options:
+                    raise errors.ValidationError(
+                        name=old_name,
+                        error=(_('Cannot use %(old_name)s with %(new_name)s') %
+                                {'old_name': old_name, 'new_name': new_name}))
+                options[new_name] = options[old_name]
+                del options[old_name]
+
+        # memberof
+        if 'memberof' in options:
+            memberof = options.pop('memberof')
+            if memberof:
+                if 'ipapermtargetfilter' in options:
+                    raise errors.ValidationError(
+                        name='ipapermtargetfilter',
+                        error=_('filter and memberof are mutually exclusive'))
+                try:
+                    groupdn = self.api.Object.group.get_dn_if_exists(memberof)
+                except errors.NotFound:
+                    raise errors.NotFound(
+                        reason=_('%s: group not found') % memberof)
+                options['ipapermtargetfilter'] = u'(memberOf=%s)' % groupdn
+            else:
+                if 'ipapermtargetfilter' not in options:
+                    options['ipapermtargetfilter'] = None
+
+        # targetgroup
+        if 'targetgroup' in options:
+            targetgroup = options.pop('targetgroup')
+            if targetgroup:
+                if 'ipapermtarget' in options:
+                    raise errors.ValidationError(
+                        name='ipapermtarget',
+                        error=_('target and targetgroup are mutually exclusive'))
+                try:
+                    groupdn = self.api.Object.group.get_dn_if_exists(targetgroup)
+                except errors.NotFound:
+                    raise errors.NotFound(
+                        reason=_('%s: group not found') % targetgroup)
+                options['ipapermtarget'] = groupdn
+            else:
+                if 'ipapermtarget' not in options:
+                    options['ipapermtarget'] = None
+
+        # type
+        if 'type' in options:
+            objtype = options.pop('type')
+            if objtype:
+                if 'ipapermlocation' in options:
+                    raise errors.ValidationError(
+                        name='ipapermlocation',
+                        error=_('subtree and type are mutually exclusive'))
+                if 'ipapermtarget' in options:
+                    raise errors.ValidationError(
+                        name='ipapermtarget',
+                        error=_('target and type are mutually exclusive'))
+                obj = self.api.Object[objtype.lower()]
+                container_dn = DN(obj.container_dn, self.api.env.basedn)
+                options['ipapermtarget'] = DN(
+                    (obj.rdn_attribute or obj.primary_key.name, '*'),
+                    container_dn)
+                options['ipapermlocation'] = container_dn
+            else:
+                if 'ipapermtarget' not in options:
+                    options['ipapermtarget'] = None
+                if 'ipapermlocation' not in options:
+                    options['ipapermlocation'] = None
+
+        if self.env.in_server:
+            ldap = self.Backend.ldap2
+
+            # Rough filter validation by a search
+            if 'ipapermtargetfilter' in options:
+                try:
+                    ldap.find_entries(
+                        filter=options['ipapermtargetfilter'],
+                        base_dn=self.env.basedn,
+                        scope=ldap.SCOPE_BASE,
+                        size_limit=1)
+                except errors.NotFound:
+                    pass
+                except errors.BadSearchFilter:
+                    raise errors.ValidationError(
+                        name='ipapermtargetfilter',
+                        error=_('Bad search filter'))
+
+
+ register()
+class permission_add_noaci(baseldap.LDAPCreate):
+    __doc__ = _('Add a system permission without an ACI (internal command)')
+
+    msg_summary = _('Added permission "%(value)s"')
+    NO_CLI = True
+    has_output_params = baseldap.LDAPCreate.has_output_params + output_params
+
+    takes_options = (
+        Str('ipapermissiontype+',
+            label=_('Permission flags'),
+        ),
+    )
+
+    def get_options(self):
+        for option in super(permission_add_noaci, self).get_options():
+            # Only cn and ipapermissiontype are required
+            if option.name == 'ipapermissiontype':
+                yield option.clone()
+            else:
+                yield option.clone(required=False)
+
+    def pre_callback(self, ldap, dn, entry, attrs_list, *keys, **options):
+        entry['ipapermissiontype'] = list(options['ipapermissiontype'])
+        return dn
+
+
+ register()
+class permission_add(baseldap.LDAPCreate):
+    __doc__ = _('Add a new permission.')
+
+    msg_summary = _('Added permission "%(value)s"')
+    has_output_params = baseldap.LDAPCreate.has_output_params + output_params
+
+    # Need to override args_options_2_params so that processed options apply to
+    # the whole command, not just the callbacks
+    def args_options_2_params(self, *args, **options):
+        if self.env.in_server:
+            self.obj.preprocess_options(options)
+
+        return super(permission_add, self).args_options_2_params(
+            *args, **options)
+
+    def pre_callback(self, ldap, dn, entry, attrs_list, *keys, **options):
+        entry['ipapermissiontype'] = ['SYSTEM', 'V2']
+        entry['cn'] = list(keys)
+        return dn
+
+    def post_callback(self, ldap, dn, entry, *keys, **options):
+        self.obj.add_aci(entry)
+        self.obj.postprocess_result(entry, options)
+        return dn
+
+
+ register()
+class permission_del(baseldap.LDAPDelete):
+    __doc__ = _('Delete a permission.')
+
+    msg_summary = _('Deleted permission "%(value)s"')
+
+    takes_options = baseldap.LDAPDelete.takes_options + (
+        Flag('force',
+             label=_('Force'),
+             flags={'no_option', 'no_output'},
+             doc=_('force delete of SYSTEM permissions'),
+        ),
+    )
+
+    def pre_callback(self, ldap, dn, *keys, **options):
+        try:
+            entry = ldap.get_entry(dn, attrs_list=self.obj.default_attributes)
+        except errors.NotFound:
+            self.obj.handle_not_found(*keys)
+
+        if not options.get('force'):
+            self.obj.reject_system(entry)
+
+        try:
+            self.obj.remove_aci(entry)
+        except errors.NotFound:
+            errors.NotFound('ACI of permission %s was not found' % keys[0])
+
+        return dn
+
+
+ register()
+class permission_mod(baseldap.LDAPUpdate):
+    __doc__ = _('Modify a permission.')
+
+    msg_summary = _('Modified permission "%(value)s"')
+    has_output_params = baseldap.LDAPUpdate.has_output_params + output_params
+
+    def args_options_2_params(self, *args, **options):
+        if self.env.in_server:
+            self.obj.preprocess_options(options)
+
+        return super(permission_mod, self).args_options_2_params(
+            *args, **options)
+
+    def pre_callback(self, ldap, dn, entry, attrs_list, *keys, **options):
+        if 'rename' in options and not options['rename']:
+            raise errors.ValidationError(name='rename',
+                                         error='New name can not be empty')
+
+        try:
+            attrs_list = self.obj.default_attributes
+            old_entry = ldap.get_entry(dn, attrs_list=attrs_list)
+        except errors.NotFound:
+            self.obj.handle_not_found(*keys)
+
+        self.obj.reject_system(old_entry)
+        self.obj.upgrade_permission(old_entry)
+
+        # Since `entry` only contains the attributes we are currently changing,
+        # it cannot be used directly to generate an ACI.
+        # First we need to copy the original data into it.
+        for key, value in old_entry.iteritems():
+            if key not in options and key != 'cn':
+                entry.setdefault(key, value)
+
+        old_location = old_entry.single_value.get('ipapermlocation',
+                                                  self.api.env.basedn)
+        if old_location == options.get('ipapermlocation', old_location):
+            context.permision_moving_aci = False
+        else:
+            context.permision_moving_aci = True
+            try:
+                self.obj.remove_aci(old_entry)
+            except errors.NotFound, e:
+                self.log.error('permission ACI not found: %s' % e)
+
+        # To pass data to postcallback, we currently need to use the context
+        context.old_entry = old_entry
+
+        return dn
+
+    def post_callback(self, ldap, dn, entry, *keys, **options):
+        old_entry = context.old_entry
+
+        if context.permision_moving_aci:
+            self.obj.add_aci(entry)
+        else:
+            self.obj.update_aci(entry, old_entry.single_value['cn'])
+        self.obj.postprocess_result(entry, options)
+        entry['dn'] = entry.dn
+        return dn
+
+
+ register()
+class permission_find(baseldap.LDAPSearch):
+    __doc__ = _('Search for permissions.')
+
+    msg_summary = ngettext(
+        '%(count)d permission matched', '%(count)d permissions matched', 0)
+    has_output_params = baseldap.LDAPSearch.has_output_params + output_params
+
+    def args_options_2_params(self, *args, **options):
+        if self.env.in_server:
+            self.obj.preprocess_options(options)
+
+        return super(permission_find, self).args_options_2_params(
+            *args, **options)
+
+    def post_callback(self, ldap, entries, truncated, *args, **options):
+        attribute_options = [o for o in options
+                             if (o in self.options and
+                                 self.options[o].attribute)]
+
+        if not options.get('pkey_only'):
+            for entry in entries:
+                # Old-style permissions might have matched (e.g. by name)
+                self.obj.upgrade_permission(entry, output_only=True)
+
+        if not truncated:
+            if 'sizelimit' in options:
+                max_entries = options['sizelimit']
+            else:
+                config = ldap.get_ipa_config()[1]
+                max_entries = int(config.single_value['ipasearchrecordslimit'])
+
+            filters = ['(objectclass=ipaPermission)',
+                       '(!(ipaPermissionType=V2))']
+            if args:
+                filters.append(ldap.make_filter_from_attr('cn', args[0],
+                                                          exact=False))
+            attrs_list = list(self.obj.default_attributes)
+            attrs_list += list(self.obj.attribute_members)
+            if options.get('all'):
+                attrs_list.append('*')
+            try:
+                legacy_entries = ldap.get_entries(
+                    base_dn=DN(self.obj.container_dn, self.api.env.basedn),
+                    filter=ldap.combine_filters(filters, rules=ldap.MATCH_ALL),
+                    attrs_list=attrs_list)
+            except errors.NotFound:
+                legacy_entries = ()
+            self.log.debug('potential legacy entries: %s', len(legacy_entries))
+            nonlegacy_names = {e.single_value['cn'] for e in entries}
+            for entry in legacy_entries:
+                if entry.single_value['cn'] in nonlegacy_names:
+                    continue
+                if len(entries) > max_entries:
+                    # We've over the limit, pop the last entry and set
+                    # truncated flag
+                    # (this is easier to do than checking before adding
+                    # the entry to results)
+                    entries.pop()
+                    truncated = True
+                    break
+                self.obj.upgrade_permission(entry, output_only=True)
+                cn = entry.single_value['cn']
+                if any(a.lower() in cn.lower() for a in args if a):
+                    entries.append(entry)
+                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
+                    else:
+                        entries.append(entry)
+
+        for entry in entries:
+            if options.get('pkey_only'):
+                for opt_name in entry.keys():
+                    if opt_name != self.obj.primary_key.name:
+                        del entry[opt_name]
+            else:
+                self.obj.postprocess_result(entry, options)
+
+        return truncated
+
+
+ register()
+class permission_show(baseldap.LDAPRetrieve):
+    __doc__ = _('Display information about a permission.')
+    has_output_params = baseldap.LDAPRetrieve.has_output_params + output_params
+
+    def post_callback(self, ldap, dn, entry, *keys, **options):
+        self.obj.upgrade_permission(entry, output_only=True)
+        self.obj.postprocess_result(entry, options)
+        return dn
+
+
+ register()
+class permission_add_member(baseldap.LDAPAddMember):
+    """Add members to a permission."""
+    NO_CLI = True
+
+
+ register()
+class permission_remove_member(baseldap.LDAPRemoveMember):
+    """Remove members from a permission."""
+    NO_CLI = True
diff --git a/ipatests/test_xmlrpc/objectclasses.py b/ipatests/test_xmlrpc/objectclasses.py
index 089ee69a38b37f11de539e60b9ecacdec7a7de0b..3d680dd20bee104142f9faf2c74294af7d51f042 100644
--- a/ipatests/test_xmlrpc/objectclasses.py
+++ b/ipatests/test_xmlrpc/objectclasses.py
@@ -80,6 +80,7 @@
 permission = [
     u'groupofnames',
     u'ipapermission',
+    u'ipapermissionv2',
     u'top'
 ]
 
diff --git a/ipatests/test_xmlrpc/test_dns_plugin.py b/ipatests/test_xmlrpc/test_dns_plugin.py
index 81e8e4ed2ed249e80bdb59bfeb61c09dd08cef2c..b30cec66b1a65b26e7f431ffec4283e93c3dbfef 100644
--- a/ipatests/test_xmlrpc/test_dns_plugin.py
+++ b/ipatests/test_xmlrpc/test_dns_plugin.py
@@ -1361,7 +1361,10 @@ def setUpClass(cls):
                 result={
                     'dn': dnszone1_permission_dn,
                     'cn': [dnszone1_permission],
+                    'objectclass': objectclasses.permission,
                     'ipapermissiontype': [u'SYSTEM'],
+                    'ipapermbindruletype': [u'permission'],
+
                 },
             ),
         ),
diff --git a/ipatests/test_xmlrpc/test_permission_plugin.py b/ipatests/test_xmlrpc/test_permission_plugin.py
index a3913a85856279a5fba096c9901ea68488719698..36cf4ba54daf1c89590014f300bfdbdbc9aad6e2 100644
--- a/ipatests/test_xmlrpc/test_permission_plugin.py
+++ b/ipatests/test_xmlrpc/test_permission_plugin.py
@@ -24,7 +24,7 @@
 
 from ipalib import api, errors
 from ipatests.test_xmlrpc import objectclasses
-from xmlrpc_test import Declarative, fuzzy_digits, fuzzy_uuid
+from xmlrpc_test import Declarative
 from ipapython.dn import DN
 
 permission1 = u'testperm'
@@ -57,18 +57,22 @@
                                     'objectclass': u'rscwo',
                                     'memberof': u'rscwo',
                                     'aci': u'rscwo',
-                                    'subtree': u'rscwo',
+                                    'ipapermlocation': u'rscwo',
                                     'o': u'rscwo',
-                                    'filter': u'rscwo',
-                                    'attrs': u'rscwo',
+                                    'ipapermallowedattr': u'rscwo',
+                                    'ipapermdefaultattr': u'rscwo',
+                                    'ipapermexcludedattr': u'rscwo',
                                     'owner': u'rscwo',
-                                    'group': u'rscwo',
                                     'ou': u'rscwo',
-                                    'targetgroup': u'rscwo',
-                                    'type': u'rscwo',
-                                    'permissions': u'rscwo',
+                                    'ipapermright': u'rscwo',
                                     'nsaccountlock': u'rscwo',
                                     'description': u'rscwo',
+                                    'ipapermtargetfilter': u'rscwo',
+                                    'ipapermbindruletype': u'rscwo',
+                                    'ipapermlocation': u'rscwo',
+                                    'ipapermtarget': u'rscwo',
+                                    'type': u'rscwo',
+                                    'targetgroup': u'rscwo',
                                    }
 
 privilege1 = u'testpriv1'
@@ -78,8 +82,12 @@
 invalid_permission1 = u'bad;perm'
 
 
+users_dn = DN(api.env.container_user, api.env.basedn)
+groups_dn = DN(api.env.container_group, api.env.basedn)
+
+
 class test_permission(Declarative):
-
+    """Misc. tests for the permission plugin"""
     cleanup_commands = [
         ('permission_del', [permission1], {'force': True}),
         ('permission_del', [permission2], {'force': True}),
@@ -101,7 +109,7 @@ class test_permission(Declarative):
 
         dict(
             desc='Try to update non-existent %r' % permission1,
-            command=('permission_mod', [permission1], dict(permissions=u'all')),
+            command=('permission_mod', [permission1], dict(ipapermright=u'all')),
             expected=errors.NotFound(
                 reason=u'%s: permission not found' % permission1),
         ),
@@ -132,7 +140,8 @@ class test_permission(Declarative):
             command=(
                 'permission_add', [permission1], dict(
                     type=u'user',
-                    permissions=u'write',
+                    ipapermright=[u'write'],
+                    ipapermallowedattr=[u'sn'],
                 )
             ),
             expected=dict(
@@ -142,8 +151,13 @@ class test_permission(Declarative):
                     dn=permission1_dn,
                     cn=[permission1],
                     objectclass=objectclasses.permission,
-                    type=u'user',
-                    permissions=[u'write'],
+                    type=[u'user'],
+                    ipapermright=[u'write'],
+                    ipapermallowedattr=[u'sn'],
+                    ipapermbindruletype=[u'permission'],
+                    ipapermissiontype=[u'SYSTEM', u'V2'],
+                    ipapermlocation=[users_dn],
+                    ipapermtarget=[DN(('uid', '*'), users_dn)],
                 ),
             ),
         ),
@@ -154,10 +168,12 @@ class test_permission(Declarative):
             command=(
                 'permission_add', [permission1], dict(
                     type=u'user',
-                    permissions=u'write',
+                    ipapermright=[u'write'],
+                    ipapermallowedattr=[u'sn'],
                 ),
             ),
-            expected=errors.DuplicateEntry(),
+            expected=errors.DuplicateEntry(
+                message='permission with name "%s" already exists' % permission1),
         ),
 
 
@@ -211,9 +227,15 @@ class test_permission(Declarative):
                 result={
                     'dn': permission1_dn,
                     'cn': [permission1],
+                    'objectclass': objectclasses.permission,
                     'member_privilege': [privilege1],
-                    'type': u'user',
-                    'permissions': [u'write'],
+                    'type': [u'user'],
+                    'ipapermright': [u'write'],
+                    'ipapermallowedattr': [u'sn'],
+                    'ipapermbindruletype': [u'permission'],
+                    'ipapermissiontype': [u'SYSTEM', u'V2'],
+                    'ipapermlocation': [users_dn],
+                    'ipapermtarget': [DN(('uid', '*'), users_dn)],
                 },
             ),
         ),
@@ -221,17 +243,28 @@ class test_permission(Declarative):
 
         dict(
             desc='Retrieve %r with --raw' % permission1,
-            command=('permission_show', [permission1], {'raw' : True}),
+            command=('permission_show', [permission1], {'raw': True}),
             expected=dict(
                 value=permission1,
                 summary=None,
                 result={
                     'dn': permission1_dn,
                     'cn': [permission1],
+                    'objectclass': objectclasses.permission,
                     'member': [privilege1_dn],
-                    'aci': u'(target = "ldap:///%s";)(version 3.0;acl "permission:testperm";allow (write) groupdn = "ldap:///%s";;)' % \
-                        (DN(('uid', '*'), ('cn', 'users'), ('cn', 'accounts'), api.env.basedn),
-                         DN(('cn', 'testperm'), ('cn', 'permissions'), ('cn', 'pbac'), api.env.basedn))
+                    'ipapermallowedattr': [u'sn'],
+                    'ipapermbindruletype': [u'permission'],
+                    'ipapermright': [u'write'],
+                    'ipapermissiontype': [u'SYSTEM', u'V2'],
+                    'ipapermlocation': [users_dn],
+                    'ipapermtarget': [DN(('uid', '*'), users_dn)],
+                    'aci': ['(targetattr = "sn")'
+                            '(target = "ldap:///%(tdn)s")'
+                            '(version 3.0;acl "permission:%(name)s";'
+                            'allow (write) groupdn = "ldap:///%(pdn)s";)' %
+                            {'tdn': DN(('uid', '*'), users_dn),
+                             'name': permission1,
+                             'pdn': permission1_dn}],
                 },
             ),
         ),
@@ -248,9 +281,15 @@ class test_permission(Declarative):
                     {
                         'dn': permission1_dn,
                         'cn': [permission1],
+                        'objectclass': objectclasses.permission,
                         'member_privilege': [privilege1],
-                        'type': u'user',
-                        'permissions': [u'write'],
+                        'type': [u'user'],
+                        'ipapermright': [u'write'],
+                        'ipapermallowedattr': [u'sn'],
+                        'ipapermbindruletype': [u'permission'],
+                        'ipapermissiontype': [u'SYSTEM', u'V2'],
+                        'ipapermlocation': [users_dn],
+                        'ipapermtarget': [DN(('uid', '*'), users_dn)],
                     },
                 ],
             ),
@@ -268,9 +307,15 @@ class test_permission(Declarative):
                     {
                         'dn': permission1_dn,
                         'cn': [permission1],
+                        'objectclass': objectclasses.permission,
                         'member_privilege': [privilege1],
-                        'type': u'user',
-                        'permissions': [u'write'],
+                        'type': [u'user'],
+                        'ipapermright': [u'write'],
+                        'ipapermallowedattr': [u'sn'],
+                        'ipapermbindruletype': [u'permission'],
+                        'ipapermissiontype': [u'SYSTEM', u'V2'],
+                        'ipapermlocation': [users_dn],
+                        'ipapermtarget': [DN(('uid', '*'), users_dn)],
                     },
                 ],
             ),
@@ -278,7 +323,7 @@ class test_permission(Declarative):
 
 
         dict(
-            desc='Search for non-existence permission using --name',
+            desc='Search for non-existent permission using --name',
             command=('permission_find', [], {'cn': u'notfound'}),
             expected=dict(
                 count=0,
@@ -300,9 +345,15 @@ class test_permission(Declarative):
                     {
                         'dn': permission1_dn,
                         'cn': [permission1],
+                        'objectclass': objectclasses.permission,
                         'member_privilege': [privilege1],
-                        'type': u'user',
-                        'permissions': [u'write'],
+                        'type': [u'user'],
+                        'ipapermright': [u'write'],
+                        'ipapermallowedattr': [u'sn'],
+                        'ipapermbindruletype': [u'permission'],
+                        'ipapermissiontype': [u'SYSTEM', u'V2'],
+                        'ipapermlocation': [users_dn],
+                        'ipapermtarget': [DN(('uid', '*'), users_dn)],
                     },
                 ],
             ),
@@ -311,7 +362,7 @@ class test_permission(Declarative):
 
         dict(
             desc='Search for %r with --raw' % permission1,
-            command=('permission_find', [permission1], {'raw' : True}),
+            command=('permission_find', [permission1], {'raw': True}),
             expected=dict(
                 count=1,
                 truncated=False,
@@ -320,10 +371,21 @@ class test_permission(Declarative):
                     {
                         'dn': permission1_dn,
                         'cn': [permission1],
+                        'objectclass': objectclasses.permission,
                         'member': [privilege1_dn],
-                        'aci': u'(target = "ldap:///%s";)(version 3.0;acl "permission:testperm";allow (write) groupdn = "ldap:///%s";;)' % \
-                            (DN(('uid', '*'), ('cn', 'users'), ('cn', 'accounts'), api.env.basedn),
-                             DN(('cn', 'testperm'), ('cn', 'permissions'), ('cn', 'pbac'), api.env.basedn)),
+                        'ipapermallowedattr': [u'sn'],
+                        'ipapermbindruletype': [u'permission'],
+                        'ipapermright': [u'write'],
+                        'ipapermissiontype': [u'SYSTEM', u'V2'],
+                        'ipapermlocation': [users_dn],
+                        'ipapermtarget': [DN(('uid', '*'), users_dn)],
+                        'aci': ['(targetattr = "sn")'
+                                '(target = "ldap:///%(tdn)s")'
+                                '(version 3.0;acl "permission:%(name)s";'
+                                'allow (write) groupdn = "ldap:///%(pdn)s";)' %
+                                {'tdn': DN(('uid', '*'), users_dn),
+                                 'name': permission1,
+                                 'pdn': permission1_dn}],
                     },
                 ],
             ),
@@ -335,9 +397,10 @@ class test_permission(Declarative):
             command=(
                 'permission_add', [permission2], dict(
                     type=u'user',
-                    permissions=u'write',
+                    ipapermright=u'write',
                     setattr=u'owner=cn=test',
                     addattr=u'owner=cn=test2',
+                    ipapermallowedattr=[u'cn'],
                 )
             ),
             expected=dict(
@@ -347,9 +410,14 @@ class test_permission(Declarative):
                     dn=permission2_dn,
                     cn=[permission2],
                     objectclass=objectclasses.permission,
-                    type=u'user',
-                    permissions=[u'write'],
+                    type=[u'user'],
+                    ipapermright=[u'write'],
                     owner=[u'cn=test', u'cn=test2'],
+                    ipapermallowedattr=[u'cn'],
+                    ipapermbindruletype=[u'permission'],
+                    ipapermissiontype=[u'SYSTEM', u'V2'],
+                    ipapermlocation=[users_dn],
+                    ipapermtarget=[DN(('uid', '*'), users_dn)],
                 ),
             ),
         ),
@@ -366,15 +434,27 @@ class test_permission(Declarative):
                     {
                         'dn': permission1_dn,
                         'cn': [permission1],
+                        'objectclass': objectclasses.permission,
                         'member_privilege': [privilege1],
-                        'type': u'user',
-                        'permissions': [u'write'],
+                        'type': [u'user'],
+                        'ipapermright': [u'write'],
+                        'ipapermallowedattr': [u'sn'],
+                        'ipapermbindruletype': [u'permission'],
+                        'ipapermissiontype': [u'SYSTEM', u'V2'],
+                        'ipapermlocation': [users_dn],
+                        'ipapermtarget': [DN(('uid', '*'), users_dn)],
                     },
                     {
                         'dn': permission2_dn,
                         'cn': [permission2],
-                        'type': u'user',
-                        'permissions': [u'write'],
+                        'objectclass': objectclasses.permission,
+                        'type': [u'user'],
+                        'ipapermright': [u'write'],
+                        'ipapermallowedattr': [u'cn'],
+                        'ipapermbindruletype': [u'permission'],
+                        'ipapermissiontype': [u'SYSTEM', u'V2'],
+                        'ipapermlocation': [users_dn],
+                        'ipapermtarget': [DN(('uid', '*'), users_dn)],
                     },
                 ],
             ),
@@ -405,7 +485,7 @@ class test_permission(Declarative):
         dict(
             desc='Search by ACI attribute with --pkey-only',
             command=('permission_find', [], {'pkey_only': True,
-                                             'attrs': [u'krbminpwdlife']}),
+                                             'ipapermallowedattr': [u'krbminpwdlife']}),
             expected=dict(
                 count=1,
                 truncated=False,
@@ -451,9 +531,15 @@ class test_permission(Declarative):
                     {
                         'dn': permission1_dn,
                         'cn': [permission1],
+                        'objectclass': objectclasses.permission,
                         'member_privilege': [privilege1],
-                        'type': u'user',
-                        'permissions': [u'write'],
+                        'type': [u'user'],
+                        'ipapermright': [u'write'],
+                        'ipapermallowedattr': [u'sn'],
+                        'ipapermbindruletype': [u'permission'],
+                        'ipapermissiontype': [u'SYSTEM', u'V2'],
+                        'ipapermlocation': [users_dn],
+                        'ipapermtarget': [DN(('uid', '*'), users_dn)],
                     },
                 ],
             ),
@@ -471,15 +557,27 @@ class test_permission(Declarative):
                     {
                         'dn': permission1_dn,
                         'cn': [permission1],
+                        'objectclass': objectclasses.permission,
+                        'type': [u'user'],
+                        'ipapermright': [u'write'],
+                        'ipapermallowedattr': [u'sn'],
+                        'ipapermbindruletype': [u'permission'],
+                        'ipapermissiontype': [u'SYSTEM', u'V2'],
+                        'ipapermlocation': [users_dn],
+                        'ipapermtarget': [DN(('uid', '*'), users_dn)],
                         'member_privilege': [privilege1],
-                        'type': u'user',
-                        'permissions': [u'write'],
                     },
                     {
                         'dn': permission2_dn,
                         'cn': [permission2],
-                        'type': u'user',
-                        'permissions': [u'write'],
+                        'objectclass': objectclasses.permission,
+                        'type': [u'user'],
+                        'ipapermright': [u'write'],
+                        'ipapermallowedattr': [u'cn'],
+                        'ipapermbindruletype': [u'permission'],
+                        'ipapermissiontype': [u'SYSTEM', u'V2'],
+                        'ipapermlocation': [users_dn],
+                        'ipapermtarget': [DN(('uid', '*'), users_dn)],
                     },
                 ],
             ),
@@ -492,7 +590,7 @@ class test_permission(Declarative):
         # to change.
         dict(
             desc='Search for permissions by attr with a limit of 1 (truncated)',
-            command=('permission_find', [], dict(attrs=u'ipaenabledflag',
+            command=('permission_find', [], dict(ipapermallowedattr=u'ipaenabledflag',
                                                  sizelimit=1)),
             expected=dict(
                 count=1,
@@ -503,11 +601,13 @@ class test_permission(Declarative):
                         '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'],
-                        'subtree' : u'ldap:///%s' % DN(('ipauniqueid', '*'), ('cn', 'hbac'), api.env.basedn),
+                        'ipapermright' : [u'write'],
+                        'ipapermallowedattr': [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'],
                     },
                 ],
             ),
@@ -518,7 +618,7 @@ class test_permission(Declarative):
             desc='Update %r' % permission1,
             command=(
                 'permission_mod', [permission1], dict(
-                    permissions=u'read',
+                    ipapermright=u'read',
                     memberof=u'ipausers',
                     setattr=u'owner=cn=other-test',
                     addattr=u'owner=cn=other-test2',
@@ -530,11 +630,19 @@ class test_permission(Declarative):
                 result=dict(
                     dn=permission1_dn,
                     cn=[permission1],
+                    objectclass=objectclasses.permission,
                     member_privilege=[privilege1],
-                    type=u'user',
-                    permissions=[u'read'],
-                    memberof=u'ipausers',
+                    type=[u'user'],
+                    ipapermright=[u'read'],
+                    memberof=[u'ipausers'],
                     owner=[u'cn=other-test', u'cn=other-test2'],
+                    ipapermallowedattr=[u'sn'],
+                    ipapermtargetfilter=[u'(memberOf=%s)' % DN('cn=ipausers',
+                                                               groups_dn)],
+                    ipapermbindruletype=[u'permission'],
+                    ipapermissiontype=[u'SYSTEM', u'V2'],
+                    ipapermlocation=[users_dn],
+                    ipapermtarget=[DN(('uid', '*'), users_dn)],
                 ),
             ),
         ),
@@ -549,10 +657,18 @@ class test_permission(Declarative):
                 result={
                     'dn': permission1_dn,
                     'cn': [permission1],
+                    'objectclass': objectclasses.permission,
                     'member_privilege': [privilege1],
-                    'type': u'user',
-                    'permissions': [u'read'],
-                    'memberof': u'ipausers',
+                    'type': [u'user'],
+                    'ipapermright': [u'read'],
+                    'memberof': [u'ipausers'],
+                    'ipapermallowedattr': [u'sn'],
+                    'ipapermtargetfilter': [u'(memberOf=%s)' % DN('cn=ipausers',
+                                                                  groups_dn)],
+                    'ipapermbindruletype': [u'permission'],
+                    'ipapermissiontype': [u'SYSTEM', u'V2'],
+                    'ipapermlocation': [users_dn],
+                    'ipapermtarget': [DN(('uid', '*'), users_dn)],
                 },
             ),
         ),
@@ -564,7 +680,7 @@ class test_permission(Declarative):
                                                                  permission2),
             command=(
                 'permission_mod', [permission1], dict(rename=permission2,
-                                                      permissions=u'all',)
+                                                      ipapermright=u'all',)
             ),
             expected=errors.DuplicateEntry(),
         ),
@@ -574,7 +690,7 @@ class test_permission(Declarative):
             desc='Try to rename %r to empty name' % (permission1),
             command=(
                 'permission_mod', [permission1], dict(rename=u'',
-                                                      permissions=u'all',)
+                                                      ipapermright=u'all',)
             ),
             expected=errors.ValidationError(name='rename',
                                     error=u'New name can not be empty'),
@@ -590,10 +706,18 @@ class test_permission(Declarative):
                 result={
                     'dn': permission1_dn,
                     'cn': [permission1],
+                    'objectclass': objectclasses.permission,
                     'member_privilege': [privilege1],
-                    'type': u'user',
-                    'permissions': [u'read'],
-                    'memberof': u'ipausers',
+                    'type': [u'user'],
+                    'ipapermright': [u'read'],
+                    'memberof': [u'ipausers'],
+                    'ipapermallowedattr': [u'sn'],
+                    'ipapermtargetfilter': [u'(memberOf=%s)' % DN('cn=ipausers',
+                                                                  groups_dn)],
+                    'ipapermbindruletype': [u'permission'],
+                    'ipapermissiontype': [u'SYSTEM', u'V2'],
+                    'ipapermlocation': [users_dn],
+                    'ipapermtarget': [DN(('uid', '*'), users_dn)],
                 },
             ),
         ),
@@ -604,7 +728,7 @@ class test_permission(Declarative):
                                                  permission1_renamed),
             command=(
                 'permission_mod', [permission1], dict(rename=permission1_renamed,
-                                                      permissions= u'all',)
+                                                      ipapermright= u'all',)
             ),
             expected=dict(
                 value=permission1,
@@ -612,10 +736,18 @@ class test_permission(Declarative):
                 result={
                     'dn': permission1_renamed_dn,
                     'cn': [permission1_renamed],
+                    'objectclass': objectclasses.permission,
                     'member_privilege': [privilege1],
-                    'type': u'user',
-                    'permissions': [u'all'],
-                    'memberof': u'ipausers',
+                    'type': [u'user'],
+                    'ipapermright': [u'all'],
+                    'memberof': [u'ipausers'],
+                    'ipapermallowedattr': [u'sn'],
+                    'ipapermtargetfilter': [u'(memberOf=%s)' % DN('cn=ipausers',
+                                                                  groups_dn)],
+                    'ipapermbindruletype': [u'permission'],
+                    'ipapermissiontype': [u'SYSTEM', u'V2'],
+                    'ipapermlocation': [users_dn],
+                    'ipapermtarget': [DN(('uid', '*'), users_dn)],
                 },
             ),
         ),
@@ -626,7 +758,7 @@ class test_permission(Declarative):
                                                  permission1_renamed_ucase),
             command=(
                 'permission_mod', [permission1_renamed], dict(rename=permission1_renamed_ucase,
-                                                      permissions= u'write',)
+                                                      ipapermright= u'write',)
             ),
             expected=dict(
                 value=permission1_renamed,
@@ -634,10 +766,18 @@ class test_permission(Declarative):
                 result={
                     'dn': permission1_renamed_ucase_dn,
                     'cn': [permission1_renamed_ucase],
+                    'objectclass': objectclasses.permission,
                     'member_privilege': [privilege1],
-                    'type': u'user',
-                    'permissions': [u'write'],
-                    'memberof': u'ipausers',
+                    'type': [u'user'],
+                    'ipapermright': [u'write'],
+                    'memberof': [u'ipausers'],
+                    'ipapermallowedattr': [u'sn'],
+                    'ipapermtargetfilter': [u'(memberOf=%s)' % DN('cn=ipausers',
+                                                                  groups_dn)],
+                    'ipapermbindruletype': [u'permission'],
+                    'ipapermissiontype': [u'SYSTEM', u'V2'],
+                    'ipapermlocation': [users_dn],
+                    'ipapermtarget': [DN(('uid', '*'), users_dn)],
                 },
             ),
         ),
@@ -647,8 +787,7 @@ class test_permission(Declarative):
             desc='Change %r to a subtree type' % permission1_renamed_ucase,
             command=(
                 'permission_mod', [permission1_renamed_ucase],
-                dict(subtree=u'ldap:///%s' % DN(('cn', '*'), ('cn', 'test'), ('cn', 'accounts'), api.env.basedn),
-                     type=None)
+                dict(ipapermlocation=users_dn, type=None)
             ),
             expected=dict(
                 value=permission1_renamed_ucase,
@@ -656,19 +795,45 @@ class test_permission(Declarative):
                 result=dict(
                     dn=permission1_renamed_ucase_dn,
                     cn=[permission1_renamed_ucase],
+                    objectclass=objectclasses.permission,
                     member_privilege=[privilege1],
-                    subtree=u'ldap:///%s' % DN(('cn', '*'), ('cn', 'test'), ('cn', 'accounts'), api.env.basedn),
-                    permissions=[u'write'],
-                    memberof=u'ipausers',
+                    ipapermlocation=[users_dn],
+                    ipapermright=[u'write'],
+                    memberof=[u'ipausers'],
+                    ipapermallowedattr=[u'sn'],
+                    ipapermtargetfilter=[u'(memberOf=%s)' % DN('cn=ipausers',
+                                                               groups_dn)],
+                    ipapermbindruletype=[u'permission'],
+                    ipapermissiontype=[u'SYSTEM', u'V2'],
                 ),
             ),
         ),
 
+        dict(
+            desc='Unset --subtree from %r' % permission2,
+            command=(
+                'permission_mod', [permission2], dict(ipapermlocation=None)
+            ),
+            expected=dict(
+                value=permission2,
+                summary=u'Modified permission "%s"' % permission2,
+                result={
+                    'dn': permission2_dn,
+                    'cn': [permission2],
+                    'objectclass': objectclasses.permission,
+                    'ipapermright': [u'write'],
+                    'ipapermallowedattr': [u'cn'],
+                    'ipapermbindruletype': [u'permission'],
+                    'ipapermissiontype': [u'SYSTEM', u'V2'],
+                    'ipapermtarget': [DN(('uid', '*'), users_dn)],
+                },
+            ),
+        ),
 
         dict(
             desc='Search for %r using --subtree' % permission1,
             command=('permission_find', [],
-                     {'subtree': u'ldap:///%s' % DN(('cn', '*'), ('cn', 'test'), ('cn', 'accounts'), api.env.basedn)}),
+                     {'ipapermlocation': u'ldap:///%s' % users_dn}),
             expected=dict(
                 count=1,
                 truncated=False,
@@ -677,10 +842,17 @@ class test_permission(Declarative):
                     {
                         'dn':permission1_renamed_ucase_dn,
                         'cn':[permission1_renamed_ucase],
+                        'objectclass': objectclasses.permission,
                         'member_privilege':[privilege1],
-                        'subtree':u'ldap:///%s' % DN(('cn', '*'), ('cn', 'test'), ('cn', 'accounts'), api.env.basedn),
-                        'permissions':[u'write'],
-                        'memberof':u'ipausers',
+                        'ipapermlocation': [users_dn],
+                        'ipapermright':[u'write'],
+                        'memberof':[u'ipausers'],
+                        'ipapermallowedattr': [u'sn'],
+                        'ipapermtargetfilter': [u'(memberOf=%s)' % DN(
+                            'cn=ipausers', groups_dn)],
+                        'ipapermbindruletype': [u'permission'],
+                        'ipapermissiontype': [u'SYSTEM', u'V2'],
+                        'ipapermlocation': [users_dn],
                     },
                 ],
             ),
@@ -689,13 +861,9 @@ class test_permission(Declarative):
 
         dict(
             desc='Search using nonexistent --subtree',
-            command=('permission_find', [], {'subtree': u'foo'}),
-            expected=dict(
-                count=0,
-                truncated=False,
-                summary=u'0 permissions matched',
-                result=[],
-            ),
+            command=('permission_find', [], {'ipapermlocation': u'foo'}),
+            expected=errors.ConversionError(
+                name='subtree', error='malformed RDN string = "foo"'),
         ),
 
 
@@ -711,11 +879,16 @@ class test_permission(Declarative):
                         'dn': DN(('cn','Add user to default group'),
                                  api.env.container_permission, api.env.basedn),
                         'cn': [u'Add user to default group'],
+                        'objectclass': objectclasses.permission,
                         'member_privilege': [u'User Administrators'],
-                        'attrs': [u'member'],
-                        'targetgroup': u'ipausers',
+                        'ipapermallowedattr': [u'member'],
+                        'targetgroup': [u'ipausers'],
                         'memberindirect_role': [u'User Administrator'],
-                        'permissions': [u'write']
+                        'ipapermright': [u'write'],
+                        'ipapermbindruletype': [u'permission'],
+                        'ipapermtarget': [DN(
+                            'cn=ipausers', api.env.container_group,
+                            api.env.basedn)],
                     }
                 ],
             ),
@@ -795,7 +968,8 @@ class test_permission(Declarative):
             command=(
                 'permission_add', [permission1], dict(
                     memberof=u'nonexisting',
-                    permissions=u'write',
+                    ipapermright=u'write',
+                    ipapermallowedattr=[u'cn'],
                 )
             ),
             expected=errors.NotFound(reason=u'nonexisting: group not found'),
@@ -806,8 +980,9 @@ class test_permission(Declarative):
             command=(
                 'permission_add', [permission1], dict(
                     memberof=u'editors',
-                    permissions=u'write',
+                    ipapermright=u'write',
                     type=u'user',
+                    ipapermallowedattr=[u'sn'],
                 )
             ),
             expected=dict(
@@ -817,9 +992,16 @@ class test_permission(Declarative):
                     dn=permission1_dn,
                     cn=[permission1],
                     objectclass=objectclasses.permission,
-                    memberof=u'editors',
-                    permissions=[u'write'],
-                    type=u'user',
+                    memberof=[u'editors'],
+                    ipapermright=[u'write'],
+                    type=[u'user'],
+                    ipapermallowedattr=[u'sn'],
+                    ipapermtargetfilter=[u'(memberOf=%s)' % DN(('cn', 'editors'),
+                                                               groups_dn)],
+                    ipapermbindruletype=[u'permission'],
+                    ipapermissiontype=[u'SYSTEM', u'V2'],
+                    ipapermlocation=[users_dn],
+                    ipapermtarget=[DN(('uid', '*'), users_dn)],
                 ),
             ),
         ),
@@ -844,9 +1026,17 @@ class test_permission(Declarative):
                 result=dict(
                     dn=permission1_dn,
                     cn=[permission1],
-                    memberof=u'admins',
-                    permissions=[u'write'],
-                    type=u'user',
+                    objectclass=objectclasses.permission,
+                    memberof=[u'admins'],
+                    ipapermright=[u'write'],
+                    type=[u'user'],
+                    ipapermallowedattr=[u'sn'],
+                    ipapermtargetfilter=[u'(memberOf=%s)' % DN(('cn', 'admins'),
+                                                               groups_dn)],
+                    ipapermbindruletype=[u'permission'],
+                    ipapermissiontype=[u'SYSTEM', u'V2'],
+                    ipapermlocation=[users_dn],
+                    ipapermtarget=[DN(('uid', '*'), users_dn)],
                 ),
             ),
         ),
@@ -864,8 +1054,14 @@ class test_permission(Declarative):
                 result=dict(
                     dn=permission1_dn,
                     cn=[permission1],
-                    permissions=[u'write'],
-                    type=u'user',
+                    objectclass=objectclasses.permission,
+                    ipapermright=[u'write'],
+                    type=[u'user'],
+                    ipapermallowedattr=[u'sn'],
+                    ipapermbindruletype=[u'permission'],
+                    ipapermissiontype=[u'SYSTEM', u'V2'],
+                    ipapermlocation=[users_dn],
+                    ipapermtarget=[DN(('uid', '*'), users_dn)],
                 ),
             ),
         ),
@@ -887,7 +1083,8 @@ class test_permission(Declarative):
             command=(
                 'permission_add', [permission1], dict(
                     targetgroup=u'editors',
-                    permissions=u'write',
+                    ipapermright=u'write',
+                    ipapermallowedattr=[u'sn'],
                 )
             ),
             expected=dict(
@@ -897,8 +1094,12 @@ class test_permission(Declarative):
                     dn=permission1_dn,
                     cn=[permission1],
                     objectclass=objectclasses.permission,
-                    targetgroup=u'editors',
-                    permissions=[u'write'],
+                    targetgroup=[u'editors'],
+                    ipapermright=[u'write'],
+                    ipapermallowedattr=[u'sn'],
+                    ipapermbindruletype=[u'permission'],
+                    ipapermtarget=[DN(('cn', 'editors'), groups_dn)],
+                    ipapermissiontype=[u'SYSTEM', u'V2'],
                 ),
             ),
         ),
@@ -907,10 +1108,10 @@ class test_permission(Declarative):
             desc='Try to create invalid %r' % invalid_permission1,
             command=('permission_add', [invalid_permission1], dict(
                     type=u'user',
-                    permissions=u'write',
+                    ipapermright=u'write',
                 )),
             expected=errors.ValidationError(name='name',
-                error='May only contain letters, numbers, -, _, and space'),
+                error='May only contain letters, numbers, -, _, ., and space'),
         ),
 
         dict(
@@ -918,8 +1119,8 @@ class test_permission(Declarative):
             command=(
                 'permission_add', [permission3], dict(
                     type=u'user',
-                    permissions=u'write',
-                    attrs=[u'cn']
+                    ipapermright=u'write',
+                    ipapermallowedattr=[u'cn']
                 )
             ),
             expected=dict(
@@ -929,9 +1130,13 @@ class test_permission(Declarative):
                     dn=permission3_dn,
                     cn=[permission3],
                     objectclass=objectclasses.permission,
-                    type=u'user',
-                    permissions=[u'write'],
-                    attrs=(u'cn',),
+                    type=[u'user'],
+                    ipapermright=[u'write'],
+                    ipapermallowedattr=(u'cn',),
+                    ipapermbindruletype=[u'permission'],
+                    ipapermtarget=[DN(('uid', '*'), users_dn)],
+                    ipapermissiontype=[u'SYSTEM', u'V2'],
+                    ipapermlocation=[users_dn],
                 ),
             ),
         ),
@@ -946,17 +1151,23 @@ class test_permission(Declarative):
                     dn=permission3_dn,
                     cn=[permission3],
                     objectclass=objectclasses.permission,
-                    type=u'user',
-                    attrs=(u'cn',),
-                    permissions=[u'write'],
-                    attributelevelrights=permission3_attributelevelrights
+                    type=[u'user'],
+                    ipapermallowedattr=(u'cn',),
+                    ipapermright=[u'write'],
+                    attributelevelrights=permission3_attributelevelrights,
+                    ipapermbindruletype=[u'permission'],
+                    ipapermtarget=[DN(('uid', '*'),users_dn)],
+                    ipapermissiontype=[u'SYSTEM', u'V2'],
+                    ipapermlocation=[users_dn],
                 ),
             ),
         ),
 
         dict(
-            desc='Modify %r with --all -rights' % permission3,
-            command=('permission_mod', [permission3], {'all' : True, 'rights': True, 'attrs':[u'cn',u'uid']}),
+            desc='Modify %r with --all --rights' % permission3,
+            command=('permission_mod', [permission3], {
+                'all': True, 'rights': True,
+                'ipapermallowedattr': [u'cn', u'uid']}),
             expected=dict(
                 value=permission3,
                 summary=u'Modified permission "%s"' % permission3,
@@ -964,11 +1175,421 @@ class test_permission(Declarative):
                     dn=permission3_dn,
                     cn=[permission3],
                     objectclass=objectclasses.permission,
-                    type=u'user',
-                    attrs=(u'cn',u'uid'),
-                    permissions=[u'write'],
+                    type=[u'user'],
+                    ipapermallowedattr=(u'cn',u'uid'),
+                    ipapermright=[u'write'],
                     attributelevelrights=permission3_attributelevelrights,
+                    ipapermbindruletype=[u'permission'],
+                    ipapermtarget=[DN(('uid', '*'), users_dn)],
+                    ipapermissiontype=[u'SYSTEM', u'V2'],
+                    ipapermlocation=[users_dn],
                 ),
             ),
         ),
+
+        dict(
+            desc='Try to modify %r with invalid targetfilter' % permission1,
+            command=('permission_mod', [permission1],
+                     {'ipapermtargetfilter': u"ceci n'est pas un filtre"}),
+            expected=errors.ValidationError(
+                name='ipapermtargetfilter',
+                error='Bad search filter'),
+        ),
+    ]
+
+
+class test_permission_sync_attributes(Declarative):
+    """Test the effects of setting permission attributes"""
+    cleanup_commands = [
+        ('permission_del', [permission1], {'force': True}),
+    ]
+
+    tests = [
+        dict(
+            desc='Create %r' % permission1,
+            command=(
+                'permission_add', [permission1], dict(
+                    ipapermlocation=users_dn,
+                    ipapermright=u'write',
+                    ipapermallowedattr=u'sn',
+                    ipapermtargetfilter=u'(memberOf=%s)' % DN(('cn', 'admins'),
+                                                              groups_dn),
+                    ipapermtarget=DN(('uid', '*'), users_dn),
+                )
+            ),
+            expected=dict(
+                value=permission1,
+                summary=u'Added permission "%s"' % permission1,
+                result=dict(
+                    dn=permission1_dn,
+                    cn=[permission1],
+                    objectclass=objectclasses.permission,
+                    type=[u'user'],
+                    ipapermright=[u'write'],
+                    ipapermallowedattr=[u'sn'],
+                    ipapermbindruletype=[u'permission'],
+                    ipapermissiontype=[u'SYSTEM', u'V2'],
+                    ipapermlocation=[users_dn],
+                    ipapermtarget=[DN(('uid', '*'), users_dn)],
+                    ipapermtargetfilter=[u'(memberOf=%s)' % DN(('cn', 'admins'),
+                                                              groups_dn)],
+                    memberof=[u'admins'],
+                ),
+            ),
+        ),
+
+        dict(
+            desc='Unset location on %r, verify type is gone' % permission1,
+            command=(
+                'permission_mod', [permission1], dict(
+                    ipapermlocation=None,
+                )
+            ),
+            expected=dict(
+                value=permission1,
+                summary=u'Modified permission "%s"' % permission1,
+                result=dict(
+                    dn=permission1_dn,
+                    cn=[permission1],
+                    objectclass=objectclasses.permission,
+                    ipapermright=[u'write'],
+                    ipapermallowedattr=[u'sn'],
+                    ipapermbindruletype=[u'permission'],
+                    ipapermissiontype=[u'SYSTEM', u'V2'],
+                    ipapermtarget=[DN(('uid', '*'), users_dn)],
+                    ipapermtargetfilter=[u'(memberOf=%s)' % DN(('cn', 'admins'),
+                                                              groups_dn)],
+                    memberof=[u'admins'],
+                ),
+            ),
+        ),
+
+        dict(
+            desc='Reset location on %r' % permission1,
+            command=(
+                'permission_mod', [permission1], dict(
+                    ipapermlocation=users_dn,
+                )
+            ),
+            expected=dict(
+                value=permission1,
+                summary=u'Modified permission "%s"' % permission1,
+                result=dict(
+                    dn=permission1_dn,
+                    cn=[permission1],
+                    objectclass=objectclasses.permission,
+                    type=[u'user'],
+                    ipapermright=[u'write'],
+                    ipapermallowedattr=[u'sn'],
+                    ipapermbindruletype=[u'permission'],
+                    ipapermissiontype=[u'SYSTEM', u'V2'],
+                    ipapermlocation=[users_dn],
+                    ipapermtarget=[DN(('uid', '*'), users_dn)],
+                    ipapermtargetfilter=[u'(memberOf=%s)' % DN(('cn', 'admins'),
+                                                              groups_dn)],
+                    memberof=[u'admins'],
+                ),
+            ),
+        ),
+
+        dict(
+            desc='Unset target on %r, verify type is gone' % permission1,
+            command=(
+                'permission_mod', [permission1], dict(
+                    ipapermtarget=None,
+                )
+            ),
+            expected=dict(
+                value=permission1,
+                summary=u'Modified permission "%s"' % permission1,
+                result=dict(
+                    dn=permission1_dn,
+                    cn=[permission1],
+                    objectclass=objectclasses.permission,
+                    ipapermright=[u'write'],
+                    ipapermallowedattr=[u'sn'],
+                    ipapermbindruletype=[u'permission'],
+                    ipapermissiontype=[u'SYSTEM', u'V2'],
+                    ipapermlocation=[users_dn],
+                    ipapermtargetfilter=[u'(memberOf=%s)' % DN(('cn', 'admins'),
+                                                              groups_dn)],
+                    memberof=[u'admins'],
+                ),
+            ),
+        ),
+
+        dict(
+            desc='Unset targetfilter on %r, verify memberof is gone' % permission1,
+            command=(
+                'permission_mod', [permission1], dict(
+                    ipapermtargetfilter=None,
+                )
+            ),
+            expected=dict(
+                value=permission1,
+                summary=u'Modified permission "%s"' % permission1,
+                result=dict(
+                    dn=permission1_dn,
+                    cn=[permission1],
+                    objectclass=objectclasses.permission,
+                    ipapermright=[u'write'],
+                    ipapermallowedattr=[u'sn'],
+                    ipapermbindruletype=[u'permission'],
+                    ipapermissiontype=[u'SYSTEM', u'V2'],
+                    ipapermlocation=[users_dn],
+                ),
+            ),
+        ),
+
+        dict(
+            desc='Set type of %r to group' % permission1,
+            command=(
+                'permission_mod', [permission1], dict(
+                    type=u'group',
+                )
+            ),
+            expected=dict(
+                value=permission1,
+                summary=u'Modified permission "%s"' % permission1,
+                result=dict(
+                    dn=permission1_dn,
+                    cn=[permission1],
+                    objectclass=objectclasses.permission,
+                    type=[u'group'],
+                    ipapermright=[u'write'],
+                    ipapermallowedattr=[u'sn'],
+                    ipapermbindruletype=[u'permission'],
+                    ipapermissiontype=[u'SYSTEM', u'V2'],
+                    ipapermlocation=[groups_dn],
+                    ipapermtarget=[DN(('cn', '*'), groups_dn)],
+                ),
+            ),
+        ),
+
+        dict(
+            desc='Set target on %r, verify targetgroup is set' % permission1,
+            command=(
+                'permission_mod', [permission1], dict(
+                    ipapermtarget=DN('cn=editors', groups_dn),
+                )
+            ),
+            expected=dict(
+                value=permission1,
+                summary=u'Modified permission "%s"' % permission1,
+                result=dict(
+                    dn=permission1_dn,
+                    cn=[permission1],
+                    objectclass=objectclasses.permission,
+                    ipapermright=[u'write'],
+                    ipapermallowedattr=[u'sn'],
+                    ipapermbindruletype=[u'permission'],
+                    ipapermissiontype=[u'SYSTEM', u'V2'],
+                    ipapermtarget=[DN('cn=editors', groups_dn)],
+                    ipapermlocation=[groups_dn],
+                    targetgroup=[u'editors'],
+                ),
+            ),
+        ),
+    ]
+
+
+class test_permission_sync_nice(Declarative):
+    """Test the effects of setting convenience options on permissions"""
+    cleanup_commands = [
+        ('permission_del', [permission1], {'force': True}),
+    ]
+
+    tests = [
+        dict(
+            desc='Create %r' % permission1,
+            command=(
+                'permission_add', [permission1], dict(
+                    type=u'user',
+                    ipapermright=u'write',
+                    ipapermallowedattr=u'sn',
+                    memberof=u'admins',
+                )
+            ),
+            expected=dict(
+                value=permission1,
+                summary=u'Added permission "%s"' % permission1,
+                result=dict(
+                    dn=permission1_dn,
+                    cn=[permission1],
+                    objectclass=objectclasses.permission,
+                    type=[u'user'],
+                    ipapermright=[u'write'],
+                    ipapermallowedattr=[u'sn'],
+                    ipapermbindruletype=[u'permission'],
+                    ipapermissiontype=[u'SYSTEM', u'V2'],
+                    ipapermlocation=[users_dn],
+                    ipapermtarget=[DN(('uid', '*'), users_dn)],
+                    ipapermtargetfilter=[u'(memberOf=%s)' % DN(('cn', 'admins'),
+                                                              groups_dn)],
+                    memberof=[u'admins'],
+                ),
+            ),
+        ),
+
+        dict(
+            desc='Unset type on %r, verify target & location are gone' % permission1,
+            command=(
+                'permission_mod', [permission1], dict(
+                    type=None,
+                )
+            ),
+            expected=dict(
+                value=permission1,
+                summary=u'Modified permission "%s"' % permission1,
+                result=dict(
+                    dn=permission1_dn,
+                    cn=[permission1],
+                    objectclass=objectclasses.permission,
+                    ipapermright=[u'write'],
+                    ipapermallowedattr=[u'sn'],
+                    ipapermbindruletype=[u'permission'],
+                    ipapermissiontype=[u'SYSTEM', u'V2'],
+                    ipapermtargetfilter=[u'(memberOf=%s)' % DN(('cn', 'admins'),
+                                                              groups_dn)],
+                    memberof=[u'admins'],
+                ),
+            ),
+        ),
+
+        dict(
+            desc='Unset memberof on %r, verify targetfilter is gone' % permission1,
+            command=(
+                'permission_mod', [permission1], dict(
+                    memberof=None,
+                )
+            ),
+            expected=dict(
+                value=permission1,
+                summary=u'Modified permission "%s"' % permission1,
+                result=dict(
+                    dn=permission1_dn,
+                    cn=[permission1],
+                    objectclass=objectclasses.permission,
+                    ipapermright=[u'write'],
+                    ipapermallowedattr=[u'sn'],
+                    ipapermbindruletype=[u'permission'],
+                    ipapermissiontype=[u'SYSTEM', u'V2'],
+                ),
+            ),
+        ),
+
+        dict(
+            desc='Set type of %r to group' % permission1,
+            command=(
+                'permission_mod', [permission1], dict(
+                    type=u'group',
+                )
+            ),
+            expected=dict(
+                value=permission1,
+                summary=u'Modified permission "%s"' % permission1,
+                result=dict(
+                    dn=permission1_dn,
+                    cn=[permission1],
+                    objectclass=objectclasses.permission,
+                    type=[u'group'],
+                    ipapermright=[u'write'],
+                    ipapermallowedattr=[u'sn'],
+                    ipapermbindruletype=[u'permission'],
+                    ipapermissiontype=[u'SYSTEM', u'V2'],
+                    ipapermlocation=[groups_dn],
+                    ipapermtarget=[DN(('cn', '*'), groups_dn)],
+                ),
+            ),
+        ),
+
+        dict(
+            desc='Set targetgroup on %r, verify target is set' % permission1,
+            command=(
+                'permission_mod', [permission1], dict(
+                    targetgroup=u'editors',
+                )
+            ),
+            expected=dict(
+                value=permission1,
+                summary=u'Modified permission "%s"' % permission1,
+                result=dict(
+                    dn=permission1_dn,
+                    cn=[permission1],
+                    objectclass=objectclasses.permission,
+                    ipapermright=[u'write'],
+                    ipapermallowedattr=[u'sn'],
+                    ipapermbindruletype=[u'permission'],
+                    ipapermissiontype=[u'SYSTEM', u'V2'],
+                    ipapermtarget=[DN('cn=editors', groups_dn)],
+                    ipapermlocation=[groups_dn],
+                    targetgroup=[u'editors'],
+                ),
+            ),
+        ),
+    ]
+
+
+def _make_permission_flag_tests(flags, expected_message):
+    return [
+
+        dict(
+            desc='Create %r with flags %s' % (permission1, flags),
+            command=(
+                'permission_add_noaci', [permission1], dict(
+                    ipapermissiontype=flags,
+                )
+            ),
+            expected=dict(
+                value=permission1,
+                summary=u'Added permission "%s"' % permission1,
+                result=dict(
+                    dn=permission1_dn,
+                    cn=[permission1],
+                    objectclass=objectclasses.permission,
+                    ipapermbindruletype=[u'permission'],
+                    ipapermissiontype=flags,
+                ),
+            ),
+        ),
+
+        dict(
+            desc='Try to modify %r' % permission1,
+            command=('permission_mod', [permission1], {'type': u'user'}),
+            expected=errors.ACIError(info=expected_message),
+        ),
+
+        dict(
+            desc='Try to delete %r' % permission1,
+            command=('permission_del', [permission1], {}),
+            expected=errors.ACIError(info=expected_message),
+        ),
+
+        dict(
+            desc='Delete %r with --force' % permission1,
+            command=('permission_del', [permission1], {'force': True}),
+            expected=dict(
+                result=dict(failed=u''),
+                value=permission1,
+                summary=u'Deleted permission "%s"' % permission1,
+            ),
+        ),
+    ]
+
+
+class test_permission_flags(Declarative):
+    """Test that permission flags are handled correctly"""
+    cleanup_commands = [
+        ('permission_del', [permission1], {'force': True}),
     ]
+
+    tests = (
+        _make_permission_flag_tests(
+            [u'SYSTEM'],
+            'A SYSTEM permission may not be modified or removed') +
+        _make_permission_flag_tests(
+            [u'??'],
+            'Permission with unknown flag ?? may not be modified or removed') +
+        _make_permission_flag_tests(
+            [u'SYSTEM', u'??'],
+            'Permission with unknown flag ?? may not be modified or removed'))
diff --git a/ipatests/test_xmlrpc/test_privilege_plugin.py b/ipatests/test_xmlrpc/test_privilege_plugin.py
index 741590dd0123cd7c293a145806d5f6f8003b86e9..b76c87c7136ffe890f3e9bdf753ff68c17021a59 100644
--- a/ipatests/test_xmlrpc/test_privilege_plugin.py
+++ b/ipatests/test_xmlrpc/test_privilege_plugin.py
@@ -38,6 +38,8 @@
 privilege1_dn = DN(('cn',privilege1),
                    api.env.container_privilege,api.env.basedn)
 
+users_dn = DN(api.env.container_user, api.env.basedn)
+
 
 class test_privilege(Declarative):
 
@@ -89,8 +91,8 @@ class test_privilege(Declarative):
             desc='Create %r' % permission1,
             command=(
                 'permission_add', [permission1], dict(
-                     type=u'user',
-                     permissions=[u'add', u'delete'],
+                    type=u'user',
+                    ipapermright=[u'add', u'delete'],
                 )
             ),
             expected=dict(
@@ -100,8 +102,12 @@ class test_privilege(Declarative):
                     dn=permission1_dn,
                     cn=[permission1],
                     objectclass=objectclasses.permission,
-                    type=u'user',
-                    permissions=[u'add', u'delete'],
+                    type=[u'user'],
+                    ipapermright=[u'add', u'delete'],
+                    ipapermbindruletype=[u'permission'],
+                    ipapermissiontype=[u'SYSTEM', u'V2'],
+                    ipapermlocation=[users_dn],
+                    ipapermtarget=[DN('uid=*', users_dn)],
                 ),
             ),
         ),
@@ -206,8 +212,8 @@ class test_privilege(Declarative):
             desc='Create %r' % permission2,
             command=(
                 'permission_add', [permission2], dict(
-                     type=u'user',
-                     permissions=u'write',
+                    type=u'user',
+                    ipapermright=u'write',
                 )
             ),
             expected=dict(
@@ -217,8 +223,12 @@ class test_privilege(Declarative):
                     dn=permission2_dn,
                     cn=[permission2],
                     objectclass=objectclasses.permission,
-                    type=u'user',
-                    permissions=[u'write'],
+                    type=[u'user'],
+                    ipapermright=[u'write'],
+                    ipapermbindruletype=[u'permission'],
+                    ipapermissiontype=[u'SYSTEM', u'V2'],
+                    ipapermlocation=[users_dn],
+                    ipapermtarget=[DN('uid=*', users_dn)],
                 ),
             ),
         ),
-- 
1.8.3.1

From cc89d7b619e882ebce2783a2ec490072e4847782 Mon Sep 17 00:00:00 2001
From: Petr Viktorin <pviktori redhat com>
Date: Fri, 29 Nov 2013 12:57:30 +0100
Subject: [PATCH] Verify ACIs are added correctly in tests

To double-check the ACIs are correct, this uses different code
than the new permission plugin: the aci_show command.
A new option, location, is added to the command to support
these checks.
---
 API.txt                                        |   3 +-
 ipalib/plugins/aci.py                          |  14 +-
 ipatests/test_xmlrpc/test_permission_plugin.py | 255 ++++++++++++++++++++++++-
 3 files changed, 266 insertions(+), 6 deletions(-)

diff --git a/API.txt b/API.txt
index a4886bc2fb9387873234357978d310627a6a5509..659a944916bbb0f03de34ed84ba344568ff2af79 100644
--- a/API.txt
+++ b/API.txt
@@ -92,10 +92,11 @@ command: aci_rename
 output: Output('summary', (<type 'unicode'>, <type 'NoneType'>), None)
 output: Output('value', <type 'unicode'>, None)
 command: aci_show
-args: 1,4,3
+args: 1,5,3
 arg: Str('aciname', attribute=True, cli_name='name', multivalue=False, primary_key=True, query=True, required=True)
 option: StrEnum('aciprefix', cli_name='prefix', values=(u'permission', u'delegation', u'selfservice', u'none'))
 option: Flag('all', autofill=True, cli_name='all', default=False, exclude='webui')
+option: DNParam('location?')
 option: Flag('raw', autofill=True, cli_name='raw', default=False, exclude='webui')
 option: Str('version?', exclude='webui')
 output: Entry('result', <type 'dict'>, Gettext('A dictionary representing an LDAP entry', domain='ipa', localedir=None))
diff --git a/ipalib/plugins/aci.py b/ipalib/plugins/aci.py
index 328effcbc1aebf5ba2c7b895a2e0d18944630579..a4c3866541dff10e4ee8042e8a860ceb3b5d4d35 100644
--- a/ipalib/plugins/aci.py
+++ b/ipalib/plugins/aci.py
@@ -120,8 +120,8 @@
 from copy import deepcopy
 
 from ipalib import api, crud, errors
-from ipalib import Object, Command
-from ipalib import Flag, Int, Str, StrEnum
+from ipalib import Object
+from ipalib import Flag, Str, StrEnum, DNParam
 from ipalib.aci import ACI
 from ipalib import output
 from ipalib import _, ngettext
@@ -892,7 +892,12 @@ class aci_show(crud.Retrieve):
         ),
     )
 
-    takes_options = (_prefix_option,)
+    takes_options = (
+        _prefix_option,
+        DNParam('location?',
+            label=_('Location of the ACI'),
+        )
+    )
 
     def execute(self, aciname, **kw):
         """
@@ -905,7 +910,8 @@ def execute(self, aciname, **kw):
         """
         ldap = self.api.Backend.ldap2
 
-        entry = ldap.get_entry(self.api.env.basedn, ['aci'])
+        dn = kw.get('location', self.api.env.basedn)
+        entry = ldap.get_entry(dn, ['aci'])
 
         acis = _convert_strings_to_acis(entry.get('aci', []))
 
diff --git a/ipatests/test_xmlrpc/test_permission_plugin.py b/ipatests/test_xmlrpc/test_permission_plugin.py
index 36cf4ba54daf1c89590014f300bfdbdbc9aad6e2..71ac2344b42c9ebe1d0c4ca9944571360869c037 100644
--- a/ipatests/test_xmlrpc/test_permission_plugin.py
+++ b/ipatests/test_xmlrpc/test_permission_plugin.py
@@ -22,10 +22,13 @@
 Test the `ipalib/plugins/permission.py` module.
 """
 
+import os
+
 from ipalib import api, errors
 from ipatests.test_xmlrpc import objectclasses
 from xmlrpc_test import Declarative
 from ipapython.dn import DN
+import inspect
 
 permission1 = u'testperm'
 permission1_dn = DN(('cn',permission1),
@@ -86,6 +89,44 @@
 groups_dn = DN(api.env.container_group, api.env.basedn)
 
 
+def verify_permission_aci(name, dn, acistring):
+    """Return test dict that verifies the ACI at the given location"""
+    return dict(
+        desc="Verify ACI of %s #(%s)" % (name, lineinfo(2)),
+        command=('aci_show', [name], dict(
+            aciprefix=u'permission', location=dn, raw=True)),
+        expected=dict(
+            result=dict(aci=acistring),
+            summary=None,
+            value=name,
+        ),
+    )
+
+
+def verify_permission_aci_missing(name, dn):
+    """Return test dict that checks the ACI at the given location is missing"""
+    return dict(
+        desc="Verify ACI of %s is missing #(%s)" % (name, lineinfo(2)),
+        command=('aci_show', [name], dict(
+            aciprefix=u'permission', location=dn, raw=True)),
+        expected=errors.NotFound(
+            reason='ACI with name "%s" not found' % name),
+    )
+
+
+def lineinfo(level):
+    """Return "filename:lineno" for `level`-th caller"""
+    # Declarative tests hide tracebacks.
+    # Including this info in the test name makes it possible
+    # to locate failing tests.
+    frame = inspect.currentframe()
+    for i in range(level):
+        frame = frame.f_back
+    lineno = frame.f_lineno
+    filename = os.path.basename(frame.f_code.co_filename)
+    return '%s:%s' % (filename, lineno)
+
+
 class test_permission(Declarative):
     """Misc. tests for the permission plugin"""
     cleanup_commands = [
@@ -106,7 +147,6 @@ class test_permission(Declarative):
                 reason=u'%s: permission not found' % permission1),
         ),
 
-
         dict(
             desc='Try to update non-existent %r' % permission1,
             command=('permission_mod', [permission1], dict(ipapermright=u'all')),
@@ -162,6 +202,13 @@ class test_permission(Declarative):
             ),
         ),
 
+        verify_permission_aci(
+            permission1, users_dn,
+            '(targetattr = "sn")' +
+            '(target = "ldap:///%s";)' % DN(('uid', '*'), users_dn) +
+            '(version 3.0;acl "permission:%s";' % permission1 +
+            'allow (write) groupdn = "ldap:///%s";;)' % permission1_dn,
+        ),
 
         dict(
             desc='Try to create duplicate %r' % permission1,
@@ -422,6 +469,14 @@ class test_permission(Declarative):
             ),
         ),
 
+        verify_permission_aci(
+            permission2, users_dn,
+            '(targetattr = "cn")' +
+            '(target = "ldap:///%s";)' % DN(('uid', '*'), users_dn) +
+            '(version 3.0;acl "permission:%s";' % permission2 +
+            'allow (write) groupdn = "ldap:///%s";;)' % permission2_dn,
+        ),
+
 
         dict(
             desc='Search for %r' % permission1,
@@ -647,6 +702,15 @@ class test_permission(Declarative):
             ),
         ),
 
+        verify_permission_aci(
+            permission1, users_dn,
+            '(targetattr = "sn")' +
+            '(target = "ldap:///%s";)' % DN(('uid', '*'), users_dn) +
+            '(targetfilter = "(memberOf=%s)")' % DN('cn=ipausers', groups_dn) +
+            '(version 3.0;acl "permission:%s";' % permission1 +
+            'allow (read) groupdn = "ldap:///%s";;)' % permission1_dn,
+        ),
+
 
         dict(
             desc='Retrieve %r to verify update' % permission1,
@@ -752,6 +816,17 @@ class test_permission(Declarative):
             ),
         ),
 
+        verify_permission_aci_missing(permission1, users_dn),
+
+        verify_permission_aci(
+            permission1_renamed, users_dn,
+            '(targetattr = "sn")' +
+            '(target = "ldap:///%s";)' % DN(('uid', '*'), users_dn) +
+            '(targetfilter = "(memberOf=%s)")' % DN('cn=ipausers', groups_dn) +
+            '(version 3.0;acl "permission:%s";' % permission1_renamed +
+            'allow (all) groupdn = "ldap:///%s";;)' % permission1_renamed_dn,
+        ),
+
 
         dict(
             desc='Rename %r to permission %r' % (permission1_renamed,
@@ -782,6 +857,17 @@ class test_permission(Declarative):
             ),
         ),
 
+        verify_permission_aci_missing(permission1_renamed, users_dn),
+
+        verify_permission_aci(
+            permission1_renamed_ucase, users_dn,
+            '(targetattr = "sn")' +
+            '(target = "ldap:///%s";)' % DN(('uid', '*'), users_dn) +
+            '(targetfilter = "(memberOf=%s)")' % DN('cn=ipausers', groups_dn) +
+            '(version 3.0;acl "permission:%s";' % permission1_renamed_ucase +
+            'allow (write) groupdn = "ldap:///%s";;)' %
+                permission1_renamed_ucase_dn,
+        ),
 
         dict(
             desc='Change %r to a subtree type' % permission1_renamed_ucase,
@@ -809,6 +895,15 @@ class test_permission(Declarative):
             ),
         ),
 
+        verify_permission_aci(
+            permission1_renamed_ucase, users_dn,
+            '(targetattr = "sn")' +
+            '(targetfilter = "(memberOf=%s)")' % DN('cn=ipausers', groups_dn) +
+            '(version 3.0;acl "permission:%s";' % permission1_renamed_ucase +
+            'allow (write) groupdn = "ldap:///%s";;)' %
+                permission1_renamed_ucase_dn,
+        ),
+
         dict(
             desc='Unset --subtree from %r' % permission2,
             command=(
@@ -830,6 +925,14 @@ class test_permission(Declarative):
             ),
         ),
 
+        verify_permission_aci(
+            permission2, api.env.basedn,
+            '(targetattr = "cn")' +
+            '(target = "ldap:///%s";)' % DN(('uid', '*'), users_dn) +
+            '(version 3.0;acl "permission:%s";' % permission2 +
+            'allow (write) groupdn = "ldap:///%s";;)' % permission2_dn,
+        ),
+
         dict(
             desc='Search for %r using --subtree' % permission1,
             command=('permission_find', [],
@@ -905,6 +1008,7 @@ class test_permission(Declarative):
             )
         ),
 
+        verify_permission_aci_missing(permission1_renamed_ucase, users_dn),
 
         dict(
             desc='Try to delete non-existent %r' % permission1,
@@ -940,6 +1044,7 @@ class test_permission(Declarative):
             )
         ),
 
+        verify_permission_aci_missing(permission2, users_dn),
 
         dict(
             desc='Search for %r' % permission1,
@@ -1006,6 +1111,15 @@ class test_permission(Declarative):
             ),
         ),
 
+        verify_permission_aci(
+            permission1, users_dn,
+            '(targetattr = "sn")' +
+            '(target = "ldap:///%s";)' % DN(('uid', '*'), users_dn) +
+            '(targetfilter = "(memberOf=%s)")' % DN('cn=editors', groups_dn) +
+            '(version 3.0;acl "permission:%s";' % permission1 +
+            'allow (write) groupdn = "ldap:///%s";;)' % permission1_dn,
+        ),
+
         dict(
             desc='Try to update non-existent memberof of %r' % permission1,
             command=('permission_mod', [permission1], dict(
@@ -1041,6 +1155,15 @@ class test_permission(Declarative):
             ),
         ),
 
+        verify_permission_aci(
+            permission1, users_dn,
+            '(targetattr = "sn")' +
+            '(target = "ldap:///%s";)' % DN(('uid', '*'), users_dn) +
+            '(targetfilter = "(memberOf=%s)")' % DN('cn=admins', groups_dn) +
+            '(version 3.0;acl "permission:%s";' % permission1 +
+            'allow (write) groupdn = "ldap:///%s";;)' % permission1_dn,
+        ),
+
         dict(
             desc='Unset memberof of permission %r' % permission1,
             command=(
@@ -1066,6 +1189,13 @@ class test_permission(Declarative):
             ),
         ),
 
+        verify_permission_aci(
+            permission1, users_dn,
+            '(targetattr = "sn")' +
+            '(target = "ldap:///%s";)' % DN(('uid', '*'), users_dn) +
+            '(version 3.0;acl "permission:%s";' % permission1 +
+            'allow (write) groupdn = "ldap:///%s";;)' % permission1_dn,
+        ),
 
         dict(
             desc='Delete %r' % permission1,
@@ -1077,6 +1207,7 @@ class test_permission(Declarative):
             )
         ),
 
+        verify_permission_aci_missing(permission1, users_dn),
 
         dict(
             desc='Create targetgroup permission %r' % permission1,
@@ -1104,6 +1235,14 @@ class test_permission(Declarative):
             ),
         ),
 
+        verify_permission_aci(
+            permission1, api.env.basedn,
+            '(targetattr = "sn")' +
+            '(target = "ldap:///%s";)' % DN('cn=editors', groups_dn) +
+            '(version 3.0;acl "permission:%s";' % permission1 +
+            'allow (write) groupdn = "ldap:///%s";;)' % permission1_dn,
+        ),
+
         dict(
             desc='Try to create invalid %r' % invalid_permission1,
             command=('permission_add', [invalid_permission1], dict(
@@ -1141,6 +1280,14 @@ class test_permission(Declarative):
             ),
         ),
 
+        verify_permission_aci(
+            permission3, users_dn,
+            '(targetattr = "cn")' +
+            '(target = "ldap:///%s";)' % DN(('uid', '*'), users_dn) +
+            '(version 3.0;acl "permission:%s";' % permission3 +
+            'allow (write) groupdn = "ldap:///%s";;)' % permission3_dn,
+        ),
+
         dict(
             desc='Retrieve %r with --all --rights' % permission3,
             command=('permission_show', [permission3], {'all' : True, 'rights' : True}),
@@ -1187,6 +1334,14 @@ class test_permission(Declarative):
             ),
         ),
 
+        verify_permission_aci(
+            permission3, users_dn,
+            '(targetattr = "cn || uid")' +
+            '(target = "ldap:///%s";)' % DN(('uid', '*'), users_dn) +
+            '(version 3.0;acl "permission:%s";' % permission3 +
+            'allow (write) groupdn = "ldap:///%s";;)' % permission3_dn,
+        ),
+
         dict(
             desc='Try to modify %r with invalid targetfilter' % permission1,
             command=('permission_mod', [permission1],
@@ -1238,6 +1393,15 @@ class test_permission_sync_attributes(Declarative):
             ),
         ),
 
+        verify_permission_aci(
+            permission1, users_dn,
+            '(targetattr = "sn")' +
+            '(target = "ldap:///%s";)' % DN(('uid', '*'), users_dn) +
+            '(targetfilter = "(memberOf=%s)")' % DN('cn=admins', groups_dn) +
+            '(version 3.0;acl "permission:%s";' % permission1 +
+            'allow (write) groupdn = "ldap:///%s";;)' % permission1_dn,
+        ),
+
         dict(
             desc='Unset location on %r, verify type is gone' % permission1,
             command=(
@@ -1264,6 +1428,15 @@ class test_permission_sync_attributes(Declarative):
             ),
         ),
 
+        verify_permission_aci(
+            permission1, api.env.basedn,
+            '(targetattr = "sn")' +
+            '(target = "ldap:///%s";)' % DN(('uid', '*'), users_dn) +
+            '(targetfilter = "(memberOf=%s)")' % DN('cn=admins', groups_dn) +
+            '(version 3.0;acl "permission:%s";' % permission1 +
+            'allow (write) groupdn = "ldap:///%s";;)' % permission1_dn,
+        ),
+
         dict(
             desc='Reset location on %r' % permission1,
             command=(
@@ -1292,6 +1465,15 @@ class test_permission_sync_attributes(Declarative):
             ),
         ),
 
+        verify_permission_aci(
+            permission1, users_dn,
+            '(targetattr = "sn")' +
+            '(target = "ldap:///%s";)' % DN(('uid', '*'), users_dn) +
+            '(targetfilter = "(memberOf=%s)")' % DN('cn=admins', groups_dn) +
+            '(version 3.0;acl "permission:%s";' % permission1 +
+            'allow (write) groupdn = "ldap:///%s";;)' % permission1_dn,
+        ),
+
         dict(
             desc='Unset target on %r, verify type is gone' % permission1,
             command=(
@@ -1318,6 +1500,14 @@ class test_permission_sync_attributes(Declarative):
             ),
         ),
 
+        verify_permission_aci(
+            permission1, users_dn,
+            '(targetattr = "sn")' +
+            '(targetfilter = "(memberOf=%s)")' % DN('cn=admins', groups_dn) +
+            '(version 3.0;acl "permission:%s";' % permission1 +
+            'allow (write) groupdn = "ldap:///%s";;)' % permission1_dn,
+        ),
+
         dict(
             desc='Unset targetfilter on %r, verify memberof is gone' % permission1,
             command=(
@@ -1341,6 +1531,13 @@ class test_permission_sync_attributes(Declarative):
             ),
         ),
 
+        verify_permission_aci(
+            permission1, users_dn,
+            '(targetattr = "sn")' +
+            '(version 3.0;acl "permission:%s";' % permission1 +
+            'allow (write) groupdn = "ldap:///%s";;)' % permission1_dn,
+        ),
+
         dict(
             desc='Set type of %r to group' % permission1,
             command=(
@@ -1366,6 +1563,14 @@ class test_permission_sync_attributes(Declarative):
             ),
         ),
 
+        verify_permission_aci(
+            permission1, groups_dn,
+            '(targetattr = "sn")' +
+            '(target = "ldap:///%s";)' % DN(('cn', '*'), groups_dn) +
+            '(version 3.0;acl "permission:%s";' % permission1 +
+            'allow (write) groupdn = "ldap:///%s";;)' % permission1_dn,
+        ),
+
         dict(
             desc='Set target on %r, verify targetgroup is set' % permission1,
             command=(
@@ -1390,6 +1595,14 @@ class test_permission_sync_attributes(Declarative):
                 ),
             ),
         ),
+
+        verify_permission_aci(
+            permission1, groups_dn,
+            '(targetattr = "sn")' +
+            '(target = "ldap:///%s";)' % DN(('cn', 'editors'), groups_dn) +
+            '(version 3.0;acl "permission:%s";' % permission1 +
+            'allow (write) groupdn = "ldap:///%s";;)' % permission1_dn,
+        ),
     ]
 
 
@@ -1431,6 +1644,15 @@ class test_permission_sync_nice(Declarative):
             ),
         ),
 
+        verify_permission_aci(
+            permission1, users_dn,
+            '(targetattr = "sn")' +
+            '(target = "ldap:///%s";)' % DN(('uid', '*'), users_dn) +
+            '(targetfilter = "(memberOf=%s)")' % DN('cn=admins', groups_dn) +
+            '(version 3.0;acl "permission:%s";' % permission1 +
+            'allow (write) groupdn = "ldap:///%s";;)' % permission1_dn,
+        ),
+
         dict(
             desc='Unset type on %r, verify target & location are gone' % permission1,
             command=(
@@ -1456,6 +1678,14 @@ class test_permission_sync_nice(Declarative):
             ),
         ),
 
+        verify_permission_aci(
+            permission1, api.env.basedn,
+            '(targetattr = "sn")' +
+            '(targetfilter = "(memberOf=%s)")' % DN('cn=admins', groups_dn) +
+            '(version 3.0;acl "permission:%s";' % permission1 +
+            'allow (write) groupdn = "ldap:///%s";;)' % permission1_dn,
+        ),
+
         dict(
             desc='Unset memberof on %r, verify targetfilter is gone' % permission1,
             command=(
@@ -1478,6 +1708,13 @@ class test_permission_sync_nice(Declarative):
             ),
         ),
 
+        verify_permission_aci(
+            permission1, api.env.basedn,
+            '(targetattr = "sn")' +
+            '(version 3.0;acl "permission:%s";' % permission1 +
+            'allow (write) groupdn = "ldap:///%s";;)' % permission1_dn,
+        ),
+
         dict(
             desc='Set type of %r to group' % permission1,
             command=(
@@ -1503,6 +1740,14 @@ class test_permission_sync_nice(Declarative):
             ),
         ),
 
+        verify_permission_aci(
+            permission1, groups_dn,
+            '(targetattr = "sn")' +
+            '(target = "ldap:///%s";)' % DN(('cn', '*'), groups_dn) +
+            '(version 3.0;acl "permission:%s";' % permission1 +
+            'allow (write) groupdn = "ldap:///%s";;)' % permission1_dn,
+        ),
+
         dict(
             desc='Set targetgroup on %r, verify target is set' % permission1,
             command=(
@@ -1527,6 +1772,14 @@ class test_permission_sync_nice(Declarative):
                 ),
             ),
         ),
+
+        verify_permission_aci(
+            permission1, groups_dn,
+            '(targetattr = "sn")' +
+            '(target = "ldap:///%s";)' % DN(('cn', 'editors'), groups_dn) +
+            '(version 3.0;acl "permission:%s";' % permission1 +
+            'allow (write) groupdn = "ldap:///%s";;)' % permission1_dn,
+        ),
     ]
 
 
-- 
1.8.3.1

From e5981d2715725787935fcc9e69d90b9c413f13f6 Mon Sep 17 00:00:00 2001
From: Petr Viktorin <pviktori redhat com>
Date: Thu, 5 Dec 2013 18:10:02 +0100
Subject: [PATCH] Roll back ACI changes on failed permission updates

---
 ipalib/plugins/permission.py                   |  63 ++++++++++++---
 ipatests/test_xmlrpc/test_permission_plugin.py | 101 +++++++++++++++++++++++++
 2 files changed, 153 insertions(+), 11 deletions(-)

diff --git a/ipalib/plugins/permission.py b/ipalib/plugins/permission.py
index c365dd08261213be3b0395bde012337f27eea4c0..1355e4cc6368fbc6609009205375d2d06d41f39e 100644
--- a/ipalib/plugins/permission.py
+++ b/ipalib/plugins/permission.py
@@ -18,6 +18,7 @@
 # along with this program.  If not, see <http://www.gnu.org/licenses/>.
 
 import re
+import traceback
 
 from ipalib.plugins import baseldap
 from ipalib import errors
@@ -365,24 +366,40 @@ def add_aci(self, permission_entry):
                                                      self.api.env.basedn)
 
         self.log.debug('Adding ACI %r to %s' % (acistring, location))
-        entry = ldap.get_entry(location, ['aci'])
+        try:
+            entry = ldap.get_entry(location, ['aci'])
+        except errors.NotFound:
+            raise errors.NotFound(reason=_('Entry %s not found') % location)
         entry.setdefault('aci', []).append(acistring)
         ldap.update_entry(entry)
 
     def remove_aci(self, permission_entry):
-        """Remove the ACI corresponding to the given permission entry"""
-        self._replace_aci(permission_entry)
+        """Remove the ACI corresponding to the given permission entry
+
+        :return: tuple:
+            - entry
+            - removed ACI string, or None if none existed previously
+        """
+        return self._replace_aci(permission_entry)
 
     def update_aci(self, permission_entry, old_name=None):
-        """Update the ACI corresponding to the given permission entry"""
+        """Update the ACI corresponding to the given permission entry
+
+        :return: tuple:
+            - entry
+            - removed ACI string, or None if none existed previously
+        """
         new_acistring = self.make_aci(permission_entry)
-        self._replace_aci(permission_entry, old_name, new_acistring)
+        return self._replace_aci(permission_entry, old_name, new_acistring)
 
     def _replace_aci(self, permission_entry, old_name=None, new_acistring=None):
         """Replace ACI corresponding to permission_entry
 
         :param old_name: the old name of the permission, if different from new
         :param new_acistring: new ACI string; if None the ACI is just deleted
+        :return: tuple:
+            - entry
+            - removed ACI string, or None if none existed previously
         """
         ldap = self.api.Backend.ldap2
         acientry, acistring = self._get_aci_entry_and_string(
@@ -401,6 +418,7 @@ def _replace_aci(self, permission_entry, old_name=None, new_acistring=None):
             ldap.update_entry(acientry)
         except errors.EmptyModlist:
             self.log.info('No changes to ACI')
+        return acientry, acistring
 
     def _get_aci_entry_and_string(self, permission_entry, name=None,
                                   notfound_ok=False):
@@ -421,7 +439,7 @@ def _get_aci_entry_and_string(self, permission_entry, name=None,
         try:
             acientry = ldap.get_entry(location, ['aci'])
         except errors.NotFound:
-            acientry = {}
+            acientry = ldap.make_entry(location)
         acis = acientry.get('aci', ())
         for acistring in acis:
             aci = ACI(acistring)
@@ -730,7 +748,7 @@ def pre_callback(self, ldap, dn, entry, attrs_list, *keys, **options):
         else:
             context.permision_moving_aci = True
             try:
-                self.obj.remove_aci(old_entry)
+                context.old_aci_info = self.obj.remove_aci(old_entry)
             except errors.NotFound, e:
                 self.log.error('permission ACI not found: %s' % e)
 
@@ -739,13 +757,36 @@ def pre_callback(self, ldap, dn, entry, attrs_list, *keys, **options):
 
         return dn
 
+    def exc_callback(self, keys, options, exc, call_func, *call_args, **call_kwargs):
+        if call_func.func_name == 'update_entry':
+            self._revert_aci()
+        raise exc
+
+    def _revert_aci(self):
+        old_aci_info = getattr(context, 'old_aci_info', None)
+        if old_aci_info:
+            # Try to roll back the old ACI
+            entry, old_aci_string = old_aci_info
+            if old_aci_string:
+                self.log.warn('Reverting ACI on %s to %s' % (entry.dn,
+                                                            old_aci_string))
+                entry['aci'].append(old_aci_string)
+                self.Backend.ldap2.update_entry(entry)
+
     def post_callback(self, ldap, dn, entry, *keys, **options):
         old_entry = context.old_entry
 
-        if context.permision_moving_aci:
-            self.obj.add_aci(entry)
-        else:
-            self.obj.update_aci(entry, old_entry.single_value['cn'])
+        try:
+            if context.permision_moving_aci:
+                self.obj.add_aci(entry)
+            else:
+                self.obj.update_aci(entry, old_entry.single_value['cn'])
+        except Exception:
+            self.log.error('Error updating ACI: %s' % traceback.format_exc())
+            self.log.warn('Reverting entry')
+            ldap.update_entry(old_entry)
+            self._revert_aci()
+            raise
         self.obj.postprocess_result(entry, options)
         entry['dn'] = entry.dn
         return dn
diff --git a/ipatests/test_xmlrpc/test_permission_plugin.py b/ipatests/test_xmlrpc/test_permission_plugin.py
index 71ac2344b42c9ebe1d0c4ca9944571360869c037..0ffc98103e4348d536d7bd57528345867f6c4e55 100644
--- a/ipatests/test_xmlrpc/test_permission_plugin.py
+++ b/ipatests/test_xmlrpc/test_permission_plugin.py
@@ -87,6 +87,7 @@
 
 users_dn = DN(api.env.container_user, api.env.basedn)
 groups_dn = DN(api.env.container_group, api.env.basedn)
+etc_dn = DN('cn=etc', api.env.basedn)
 
 
 def verify_permission_aci(name, dn, acistring):
@@ -1353,6 +1354,106 @@ class test_permission(Declarative):
     ]
 
 
+class test_permission_rollback(Declarative):
+    """Test rolling back changes after failed update"""
+    cleanup_commands = [
+        ('permission_del', [permission1], {'force': True}),
+    ]
+
+    _verifications = [
+        dict(
+            desc='Retrieve %r' % permission1,
+            command=('permission_show', [permission1], {}),
+            expected=dict(
+                value=permission1,
+                summary=None,
+                result={
+                    'dn': permission1_dn,
+                    'cn': [permission1],
+                    'objectclass': objectclasses.permission,
+                    'ipapermright': [u'write'],
+                    'ipapermallowedattr': [u'sn'],
+                    'ipapermbindruletype': [u'permission'],
+                    'ipapermissiontype': [u'SYSTEM', u'V2'],
+                    'ipapermlocation': [users_dn],
+                    'ipapermtarget': [DN(('uid', 'admin'), users_dn)],
+                },
+            ),
+        ),
+
+        verify_permission_aci(
+            permission1, users_dn,
+            '(targetattr = "sn")' +
+            '(target = "ldap:///%s";)' % DN(('uid', 'admin'), users_dn) +
+            '(version 3.0;acl "permission:%s";' % permission1 +
+            'allow (write) groupdn = "ldap:///%s";;)' % permission1_dn,
+        ),
+
+        verify_permission_aci_missing(permission1, etc_dn)
+    ]
+
+    tests = [
+        dict(
+            desc='Create %r' % permission1,
+            command=(
+                'permission_add', [permission1], dict(
+                    ipapermlocation=users_dn,
+                    ipapermtarget=DN('uid=admin', users_dn),
+                    ipapermright=[u'write'],
+                    ipapermallowedattr=[u'sn'],
+                )
+            ),
+            expected=dict(
+                value=permission1,
+                summary=u'Added permission "%s"' % permission1,
+                result=dict(
+                    dn=permission1_dn,
+                    cn=[permission1],
+                    objectclass=objectclasses.permission,
+                    ipapermright=[u'write'],
+                    ipapermallowedattr=[u'sn'],
+                    ipapermbindruletype=[u'permission'],
+                    ipapermissiontype=[u'SYSTEM', u'V2'],
+                    ipapermlocation=[users_dn],
+                    ipapermtarget=[DN(('uid', 'admin'), users_dn)],
+                ),
+            ),
+        ),
+
+    ] + _verifications + [
+
+        dict(
+            desc='Move %r to non-existent DN' % permission1,
+            command=(
+                'permission_mod', [permission1], dict(
+                    ipapermlocation=DN('foo=bar'),
+                )
+            ),
+            expected=errors.NotFound(reason='Entry foo=bar not found'),
+        ),
+
+    ] + _verifications + [
+
+        dict(
+            desc='Move %r to another DN' % permission1,
+            command=('permission_mod', [permission1],
+                     dict(ipapermlocation=etc_dn)
+            ),
+            expected=errors.InvalidSyntax(
+                attr=r'ACL Invalid Target Error(-8): '
+                    r'Target is beyond the scope of the ACL'
+                    r'(SCOPE:%(sdn)s) '
+                    r'(targetattr = \22sn\22)'
+                    r'(target = \22ldap:///%(tdn)s\22)'
+                    r'(version 3.0;acl \22permission:testperm\22;'
+                    r'allow (write) groupdn = \22ldap:///%(pdn)s\22;)' % dict(
+                        sdn=etc_dn,
+                        tdn=DN('uid=admin', users_dn),
+                        pdn=permission1_dn)),
+        ),
+
+    ] + _verifications
+
 class test_permission_sync_attributes(Declarative):
     """Test the effects of setting permission attributes"""
     cleanup_commands = [
-- 
1.8.3.1

From 119c4898c4ea434db2092b8151e9cccc682e9541 Mon Sep 17 00:00:00 2001
From: Petr Viktorin <pviktori redhat com>
Date: Thu, 5 Dec 2013 18:18:32 +0100
Subject: [PATCH] permission plugin: Ensure ipapermlocation (subtree) always
 exists

---
 ipalib/plugins/permission.py                       | 10 ++++++++++
 ipatests/test_xmlrpc/test_old_permission_plugin.py |  9 +++------
 ipatests/test_xmlrpc/test_permission_plugin.py     | 18 +++++++++++++++++-
 3 files changed, 30 insertions(+), 7 deletions(-)

diff --git a/ipalib/plugins/permission.py b/ipalib/plugins/permission.py
index 1355e4cc6368fbc6609009205375d2d06d41f39e..048f82f1ef3b635db5870460b2f27101f907b838 100644
--- a/ipalib/plugins/permission.py
+++ b/ipalib/plugins/permission.py
@@ -620,6 +620,16 @@ def preprocess_options(self, options):
                         name='ipapermtargetfilter',
                         error=_('Bad search filter'))
 
+            # Ensure location exists
+            if options.get('ipapermlocation'):
+                location = DN(options['ipapermlocation'])
+                try:
+                    ldap.get_entry(location, attrs_list=[])
+                except errors.NotFound:
+                    raise errors.ValidationError(
+                        name='ipapermlocation',
+                        error=_('Entry %s does not exist') % location)
+
 
 @register()
 class permission_add_noaci(baseldap.LDAPCreate):
diff --git a/ipatests/test_xmlrpc/test_old_permission_plugin.py b/ipatests/test_xmlrpc/test_old_permission_plugin.py
index cb4a62317116a606fc73127c65961b257628d2bd..d65cd38258367e17e701b19d2bbb4cb991b6e3b7 100644
--- a/ipatests/test_xmlrpc/test_old_permission_plugin.py
+++ b/ipatests/test_xmlrpc/test_old_permission_plugin.py
@@ -806,12 +806,9 @@ class test_old_permission(Declarative):
         dict(
             desc='Search using nonexistent --subtree',
             command=('permission_find', [], {'subtree': u'ldap:///foo=bar'}),
-            expected=dict(
-                count=0,
-                truncated=False,
-                summary=u'0 permissions matched',
-                result=[],
-            ),
+            expected=errors.ValidationError(
+                name='ipapermlocation',
+                error='Entry foo=bar does not exist')
         ),
 
 
diff --git a/ipatests/test_xmlrpc/test_permission_plugin.py b/ipatests/test_xmlrpc/test_permission_plugin.py
index 0ffc98103e4348d536d7bd57528345867f6c4e55..30dfadca1e0546da5cbadafa5e2044bfa7a2ed27 100644
--- a/ipatests/test_xmlrpc/test_permission_plugin.py
+++ b/ipatests/test_xmlrpc/test_permission_plugin.py
@@ -88,6 +88,7 @@
 users_dn = DN(api.env.container_user, api.env.basedn)
 groups_dn = DN(api.env.container_group, api.env.basedn)
 etc_dn = DN('cn=etc', api.env.basedn)
+nonexistent_dn = DN('cn=does not exist', api.env.basedn)
 
 
 def verify_permission_aci(name, dn, acistring):
@@ -1351,6 +1352,19 @@ class test_permission(Declarative):
                 name='ipapermtargetfilter',
                 error='Bad search filter'),
         ),
+
+
+        dict(
+            desc='Try setting nonexisting location on %r' % permission1,
+            command=(
+                'permission_mod', [permission1], dict(
+                    ipapermlocation=nonexistent_dn,
+                )
+            ),
+            expected=errors.ValidationError(
+                name='ipapermlocation',
+                error='Entry %s does not exist' % nonexistent_dn)
+        ),
     ]
 
 
@@ -1429,7 +1443,9 @@ class test_permission_rollback(Declarative):
                     ipapermlocation=DN('foo=bar'),
                 )
             ),
-            expected=errors.NotFound(reason='Entry foo=bar not found'),
+            expected=errors.ValidationError(
+                name='ipapermlocation',
+                error='Entry foo=bar does not exist'),
         ),
 
     ] + _verifications + [
-- 
1.8.3.1


[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]