Endi Sukma Dewata wrote:
On 1/4/2012 2:32 PM, Rob Crittenden wrote:
ipa permission-add test --permissions=all
--memberof=editors --targetgroup=ipausers

It generates the following ACI:

(targetfilter = "(memberOf=cn=editors,cn=groups,cn=accounts,
dc=example,dc=com)")
(target = "ldap:///cn=ipausers,cn=groups,cn=accounts,
dc=example,dc=com")
(version 3.0;acl "permission:test";allow (all)
groupdn = "ldap:///cn=test,cn=permissions,cn=pbac,
dc=example,dc=com";)

If I understand correctly this ACI gives members of cn=test full access
to members of cn=editors under the cn=ipausers subtree.

In this case there is no subtree, cn=ipausers is a group.

Right, specifying filter/memberof with targetgroup doesn't make sense
because there's no entries under the group. But it's still possible to
create useful ACI by specifying filter/memberof with type/subtree. For
example, the following permission targets users that are members of
editors only:

ipa permission-add test --permissions=all
--memberof=editors --type=user

It will generate the following ACI target:

(target = "ldap:///uid=*,cn=users,cn=accounts,dc=example,dc=com";)

Since target and targetfilter attributes can co-exist in the ACI, I
agree that we might want to relax the rules. So the permission target
can be defined with a subtree, or a filter, or both. With a subtree we
can specify either a generic subtree, a type, or a targetgroup. With a
filter we can specify either a generic filter or a memberof. Is this
correct?

There are a lot of things we CAN allow, the 389-ds acis are extremely
flexible. The question is do we need to? I'm all for providing lots of
rope but acis are very hard to get right and can be difficult to read
and debug which is why I tried to keep things as simple as I could. I
think its fine if we have some constraints.

Either one is OK for me.

For consistency & simplicity it might be better to combine the rules
such that filter, memberof, type, subtree, and targetgroup are mutually
exclusive. The memberof wasn't available in the UI before and the CLI
support wasn't complete either, so I'm not sure if anybody is relying on
this feature prior to this.

But if we want more flexibility to support scenario like above with
filter/memberof and type/subtree together, we will need to split the
target into subtree and filter as described earlier.


I guess I'm just not convinced this additional complexity would buy us anything.

Updated patch attached that fixes the memberof display and updates the tests trivially.

rob
>From de83d9b702a3ac37dfe68d94fb3d9de1f3e4c5da Mon Sep 17 00:00:00 2001
From: Rob Crittenden <rcrit...@redhat.com>
Date: Tue, 6 Dec 2011 18:15:41 -0500
Subject: [PATCH] Display the value of memberOf ACIs in permission plugin.

There were two problems:

1. memberof wasn't in the list of things we looked for in the return value
   from aci_show()
2. The value wasn't being translated into a group name.

Use the DN class to retrieve the group name from the memberof URI.

Note that I changed the parsing for targetgroup as well. We now save a lookup
and potentially returning a NotFound if an aci points to a group that no
longer exists.

https://fedorahosted.org/freeipa/ticket/2100
---
 ipalib/plugins/aci.py                       |   11 +++--
 ipalib/plugins/permission.py                |    4 +-
 tests/test_xmlrpc/test_permission_plugin.py |   62 ++++++++++++++++++++++++++-
 3 files changed, 70 insertions(+), 7 deletions(-)

diff --git a/ipalib/plugins/aci.py b/ipalib/plugins/aci.py
index 7ace05eb4a19b5bf9443a1f274c9e7e548fb062b..4b85bc93cbf309dd4ddcb0b0dfd5b1d45c807239 100644
--- a/ipalib/plugins/aci.py
+++ b/ipalib/plugins/aci.py
@@ -122,6 +122,7 @@ from ipalib import api, crud, errors
 from ipalib import Object, Command
 from ipalib import Flag, Int, Str, StrEnum
 from ipalib.aci import ACI
+from ipalib.dn import DN
 from ipalib import output
 from ipalib import _, ngettext
 if api.env.in_server and api.env.context in ['lite', 'server']:
@@ -312,8 +313,10 @@ def _aci_to_kw(ldap, a, test=False):
         kw['attrs'] = tuple(kw['attrs'])
     if 'targetfilter' in a.target:
         target = a.target['targetfilter']['expression']
-        if target.startswith('(memberOf') or target.startswith('memberOf'):
-            kw['memberof'] = unicode(target)
+        if target.startswith('(memberOf=') or target.startswith('memberOf='):
+            (junk, memberof) = target.split('memberOf=', 1)
+            memberof = DN(memberof)
+            kw['memberof'] = memberof['cn']
         else:
             kw['filter'] = unicode(target)
     if 'target' in a.target:
@@ -332,8 +335,8 @@ def _aci_to_kw(ldap, a, test=False):
                 # targetgroup attr, otherwise we consider it a subtree
                 if api.env.container_group in target:
                     targetdn = unicode(target.replace('ldap:///',''))
-                    (dn, entry_attrs) = ldap.get_entry(targetdn, ['cn'])
-                    kw['targetgroup'] = entry_attrs['cn'][0]
+                    target = DN(targetdn)
+                    kw['targetgroup'] = target['cn']
                 else:
                     kw['subtree'] = unicode(target)
 
diff --git a/ipalib/plugins/permission.py b/ipalib/plugins/permission.py
index c48979f9dd99fa103e1bd26e0f6d5c13800fe9b8..e4d11f0d8f09e5d94cbb4d3eb7e0f944ada558d6 100644
--- a/ipalib/plugins/permission.py
+++ b/ipalib/plugins/permission.py
@@ -98,7 +98,7 @@ class permission(LDAPObject):
         'memberindirect', 'ipapermissiontype',
     ]
     aci_attributes = ['group', 'permissions', 'attrs', 'type',
-        'filter', 'subtree', 'targetgroup',
+        'filter', 'subtree', 'targetgroup', 'memberof',
     ]
     attribute_members = {
         'member': ['privilege'],
@@ -338,7 +338,7 @@ class permission_mod(LDAPUpdate):
 
         result = self.api.Command.permission_show(cn, **options)['result']
         for r in result:
-            if not r.startswith('member'):
+            if not r.startswith('member_'):
                 entry_attrs[r] = result[r]
         return dn
 
diff --git a/tests/test_xmlrpc/test_permission_plugin.py b/tests/test_xmlrpc/test_permission_plugin.py
index a116a66ea29384859f729ed8a95889ba9c05095a..b0df800944005129a511c7016d6dfae88d737938 100644
--- a/tests/test_xmlrpc/test_permission_plugin.py
+++ b/tests/test_xmlrpc/test_permission_plugin.py
@@ -290,7 +290,7 @@ class test_permission(Declarative):
         dict(
             desc='Update %r' % permission1,
             command=(
-                'permission_mod', [permission1], dict(permissions=u'read')
+                'permission_mod', [permission1], dict(permissions=u'read', memberof=u'ipausers')
             ),
             expected=dict(
                 value=permission1,
@@ -301,6 +301,7 @@ class test_permission(Declarative):
                     member_privilege=[privilege1],
                     type=u'user',
                     permissions=[u'read'],
+                    memberof=u'ipausers',
                 ),
             ),
         ),
@@ -318,6 +319,7 @@ class test_permission(Declarative):
                     'member_privilege': [privilege1],
                     'type': u'user',
                     'permissions': [u'read'],
+                    'memberof': u'ipausers',
                 },
             ),
         ),
@@ -347,6 +349,7 @@ class test_permission(Declarative):
                     'member_privilege': [privilege1],
                     'type': u'user',
                     'permissions': [u'read'],
+                    'memberof': u'ipausers',
                 },
             ),
         ),
@@ -368,6 +371,7 @@ class test_permission(Declarative):
                     'member_privilege': [privilege1],
                     'type': u'user',
                     'permissions': [u'all'],
+                    'memberof': u'ipausers',
                 },
             ),
         ),
@@ -438,4 +442,60 @@ class test_permission(Declarative):
             )
         ),
 
+
+        dict(
+            desc='Create memberof permission %r' % permission1,
+            command=(
+                'permission_add', [permission1], dict(
+                     memberof=u'editors',
+                     permissions=u'write',
+                )
+            ),
+            expected=dict(
+                value=permission1,
+                summary=u'Added permission "%s"' % permission1,
+                result=dict(
+                    dn=lambda x: DN(x) == permission1_dn,
+                    cn=[permission1],
+                    objectclass=objectclasses.permission,
+                    memberof=u'editors',
+                    permissions=[u'write'],
+                ),
+            ),
+        ),
+
+
+        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=lambda x: DN(x) == permission1_dn,
+                    cn=[permission1],
+                    objectclass=objectclasses.permission,
+                    targetgroup=u'editors',
+                    permissions=[u'write'],
+                ),
+            ),
+        ),
+
+
     ]
-- 
1.7.6

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

Reply via email to