Martin Kosek wrote:
On Fri, 2012-05-11 at 16:34 -0400, Rob Crittenden wrote:
permission-find --subtree wasn't implemented so always returned all
entries (the option was ignored).
rob
I found the following 2 issues:
1) The following piece of code is over-complicated:
+found = False
+if kw['subtree'] == target:
+found = True
+if not found:
+try:
+results.remove(a)
+except ValueError:
+pass
This would simpler and more readable:
+if kw['subtree'] != target:
+try:
+results.remove(a)
+except ValueError:
+pass
2) I know we don't validate the subtree expression, but I wonder if we
shouldn't make the subtree comparing at least case insensitive as DN is
also not case sensitive.
Martin
Yeah, much simpler. Fixed both.
rob
From df3d0d26a4cbc534bb49338ca7eb8261b9fe2787 Mon Sep 17 00:00:00 2001
From: Rob Crittenden rcrit...@redhat.com
Date: Fri, 11 May 2012 16:15:58 -0400
Subject: [PATCH] Implement permission/aci find by subtree
https://fedorahosted.org/freeipa/ticket/2321
---
ipalib/plugins/aci.py | 13 -
tests/test_xmlrpc/test_permission_plugin.py | 41 +++
2 files changed, 53 insertions(+), 1 deletion(-)
diff --git a/ipalib/plugins/aci.py b/ipalib/plugins/aci.py
index f0b81f48af1f9fbf8ab267a3d4b113c328ab1170..51f6568ceb47408a1876e1f4dd03387fc2e9c9b3 100644
--- a/ipalib/plugins/aci.py
+++ b/ipalib/plugins/aci.py
@@ -840,7 +840,18 @@ class aci_find(crud.Search):
a.target['targetfilter']['expression'] != kw['filter']:
results.remove(a)
-# TODO: searching by: subtree
+if kw.get('subtree'):
+for a in acis:
+if 'target' in a.target:
+target = a.target['target']['expression']
+else:
+results.remove(a)
+continue
+if kw['subtree'].lower() != target.lower():
+try:
+results.remove(a)
+except ValueError:
+pass
acis = []
for result in results:
diff --git a/tests/test_xmlrpc/test_permission_plugin.py b/tests/test_xmlrpc/test_permission_plugin.py
index 14cfcbc784624011d067eaa8a1400b5c4f6fe963..f54e20462c3c26a0a1cd98a0a75a310bacfc62de 100644
--- a/tests/test_xmlrpc/test_permission_plugin.py
+++ b/tests/test_xmlrpc/test_permission_plugin.py
@@ -478,6 +478,47 @@ class test_permission(Declarative):
dict(
+desc='Change %r to a subtree type' % permission1_renamed_ucase,
+command=(
+'permission_mod', [permission1_renamed_ucase], dict(subtree=u'ldap:///cn=*,cn=test,cn=accounts,%s' % api.env.basedn, type=None)
+),
+expected=dict(
+value=permission1_renamed_ucase,
+summary=u'Modified permission %s' % permission1_renamed_ucase,
+result=dict(
+dn=lambda x: DN(x) == permission1_renamed_ucase_dn,
+cn=[permission1_renamed_ucase.lower()],
+member_privilege=[privilege1],
+subtree=u'ldap:///cn=*,cn=test,cn=accounts,%s' % api.env.basedn,
+permissions=[u'write'],
+memberof=u'ipausers',
+),
+),
+),
+
+
+dict(
+desc='Search for %r using --subtree' % permission1,
+command=('permission_find', [], {'subtree': 'ldap:///cn=*,cn=test,cn=accounts,%s' % api.env.basedn}),
+expected=dict(
+count=1,
+truncated=False,
+summary=u'1 permission matched',
+result=[
+{
+'dn':lambda x: DN(x) == permission1_renamed_ucase_dn,
+'cn':[permission1_renamed_ucase.lower()],
+'member_privilege':[privilege1],
+'subtree':u'ldap:///cn=*,cn=test,cn=accounts,%s' % api.env.basedn,
+'permissions':[u'write'],
+'memberof':u'ipausers',
+},
+],
+),
+),
+
+
+dict(
desc='Delete %r' % permission1_renamed_ucase,
command=('permission_del', [permission1_renamed_ucase], {}),
expected=dict(
--
1.7.10.1
___
Freeipa-devel mailing list
Freeipa-devel@redhat.com
https://www.redhat.com/mailman/listinfo/freeipa-devel
