On Thu, 2012-01-12 at 22:47 -0500, Rob Crittenden wrote: > Martin Kosek wrote: > > On Fri, 2011-12-09 at 19:33 -0600, Endi Sukma Dewata wrote: > >> On 12/9/2011 9:47 AM, Martin Kosek wrote: > >>> pkey-only functionality has to be implemented separately for these > >>> modules as they are based on crud.Search instead of standard > >>> LDAPSearch. > >>> > >>> Delegation commands were modified in the process to allow ACIs > >>> without 'memberof' as delegation ACIs. This check is no longer > >>> needed since delegation ACI prefixe ensures the ACI type. > >>> > >>> https://fedorahosted.org/freeipa/ticket/2092 > >> > >> From UI perspective this is ACKed. I'm sending a patch to enable paging > >> on these pages. > >> > > > > Thanks for the UI review Endi. If there are no objections from server > > people too we can push this. > > > > A rebased version for current master is attached. > > > > Martin > > The delegation tests fail with creation. Here is what it looks like from > the cli. > > # ipa delegation-add --attrs=street,c,l,st,postalcode --group=editors > --permissions=write --membergroup=admins delegation1 > ipa: ERROR: Delegation 'delegation1' not found > > rob
In this case, delegation was broken earlier in a recent patch "Display the value of memberOf ACIs in permission plugin". A new version of the patch #180 adding the --pkey-only option + fixing the delegation plugin is attached. Delegation plugin is pretty simple and straightforward after this patch. Martin
>From 776f15592fbf41387d37b7d89f2d756ab8da6688 Mon Sep 17 00:00:00 2001 From: Martin Kosek <mko...@redhat.com> Date: Mon, 16 Jan 2012 11:14:59 +0100 Subject: [PATCH] Add missing --pkey-only option for selfservice and delegation pkey-only functionality has to be implemented separately for these modules as they are based on crud.Search instead of standard LDAPSearch. Delegation moduled was also fixed to support new format of ACI's memberof attribute introduced in patch "Display the value of memberOf ACIs in permission plugin." https://fedorahosted.org/freeipa/ticket/2092 --- API.txt | 9 ++- VERSION | 2 +- ipalib/plugins/aci.py | 11 +++- ipalib/plugins/baseldap.py | 12 ++-- ipalib/plugins/delegation.py | 85 +++++++++----------------- ipalib/plugins/selfservice.py | 3 + tests/test_xmlrpc/test_delegation_plugin.py | 16 +++++ tests/test_xmlrpc/test_permission_plugin.py | 21 ++++++ tests/test_xmlrpc/test_selfservice_plugin.py | 15 +++++ 9 files changed, 106 insertions(+), 68 deletions(-) diff --git a/API.txt b/API.txt index 80ef42ba01728de400f413b790c35c51ae80d6ff..9048231bb1f349047f9790e5335778d4c3d637b0 100644 --- a/API.txt +++ b/API.txt @@ -27,7 +27,7 @@ output: Output('summary', (<type 'unicode'>, <type 'NoneType'>), None) output: Output('result', <type 'bool'>, None) output: Output('value', <type 'unicode'>, None) command: aci_find -args: 1,15,4 +args: 1,16,4 arg: Str('criteria?') option: Str('aciname', attribute=False, autofill=False, cli_name='name', multivalue=False, primary_key=True, query=True, required=False) option: Str('permission', attribute=False, autofill=False, cli_name='permission', multivalue=False, query=True, required=False) @@ -41,6 +41,7 @@ option: Str('subtree', attribute=False, autofill=False, cli_name='subtree', mult option: Str('targetgroup', attribute=False, autofill=False, cli_name='targetgroup', multivalue=False, query=True, required=False) option: Bool('selfaci', attribute=False, autofill=False, cli_name='self', default=False, multivalue=False, query=True, required=False) option: StrEnum('aciprefix?', cli_name='prefix', multivalue=False, required=False, values=(u'permission', u'delegation', u'selfservice', u'none')) +option: Flag('pkey_only?', autofill=True, default=False) option: Flag('all', autofill=True, cli_name='all', default=False, exclude='webui') option: Flag('raw', autofill=True, cli_name='raw', default=False, exclude='webui') option: Str('version?', exclude='webui') @@ -560,13 +561,14 @@ output: Output('summary', (<type 'unicode'>, <type 'NoneType'>), None) output: Output('result', <type 'bool'>, None) output: Output('value', <type 'unicode'>, None) command: delegation_find -args: 1,8,4 +args: 1,9,4 arg: Str('criteria?') option: Str('aciname', attribute=True, autofill=False, cli_name='name', multivalue=False, primary_key=True, query=True, required=False) option: Str('permissions', attribute=True, autofill=False, cli_name='permissions', csv=True, multivalue=True, query=True, required=False) option: Str('attrs', attribute=True, autofill=False, cli_name='attrs', csv=True, multivalue=True, query=True, required=False) option: Str('memberof', attribute=True, autofill=False, cli_name='membergroup', multivalue=False, query=True, required=False) option: Str('group', attribute=True, autofill=False, cli_name='group', multivalue=False, query=True, required=False) +option: Flag('pkey_only?', autofill=True, default=False) option: Flag('all', autofill=True, cli_name='all', default=False, exclude='webui') option: Flag('raw', autofill=True, cli_name='raw', default=False, exclude='webui') option: Str('version?', exclude='webui') @@ -2465,11 +2467,12 @@ output: Output('summary', (<type 'unicode'>, <type 'NoneType'>), None) output: Output('result', <type 'bool'>, None) output: Output('value', <type 'unicode'>, None) command: selfservice_find -args: 1,6,4 +args: 1,7,4 arg: Str('criteria?') option: Str('aciname', attribute=True, autofill=False, cli_name='name', multivalue=False, primary_key=True, query=True, required=False) option: Str('permissions', attribute=True, autofill=False, cli_name='permissions', csv=True, multivalue=True, query=True, required=False) option: Str('attrs', attribute=True, autofill=False, cli_name='attrs', csv=True, multivalue=True, query=True, required=False) +option: Flag('pkey_only?', autofill=True, default=False) option: Flag('all', autofill=True, cli_name='all', default=False, exclude='webui') option: Flag('raw', autofill=True, cli_name='raw', default=False, exclude='webui') option: Str('version?', exclude='webui') diff --git a/VERSION b/VERSION index dafb7d803da8dff6a1bcf800d6a13cb3961f1575..ff245b559f67f80f83119d15a154c30185ad0f55 100644 --- a/VERSION +++ b/VERSION @@ -79,4 +79,4 @@ IPA_DATA_VERSION=20100614120000 # # ######################################################## IPA_API_VERSION_MAJOR=2 -IPA_API_VERSION_MINOR=20 +IPA_API_VERSION_MINOR=21 diff --git a/ipalib/plugins/aci.py b/ipalib/plugins/aci.py index 8a10efccbb387044e8548ff6bd2d6bfcacf206d4..e4ef2ef17ffed7f47a88d0ff5e60ace942d60657 100644 --- a/ipalib/plugins/aci.py +++ b/ipalib/plugins/aci.py @@ -126,6 +126,7 @@ from ipalib.aci import ACI from ipalib.dn import DN from ipalib import output from ipalib import _, ngettext +from ipalib.plugins.baseldap import gen_pkey_only_option if api.env.in_server and api.env.context in ['lite', 'server']: from ldap import explode_dn from ipapython.ipa_log_manager import * @@ -297,7 +298,7 @@ def _make_aci(ldap, current, aciname, kw): return a -def _aci_to_kw(ldap, a, test=False): +def _aci_to_kw(ldap, a, test=False, pkey_only=False): """Convert an ACI into its equivalent keywords. This is used for the modify operation so we can merge the @@ -306,6 +307,8 @@ def _aci_to_kw(ldap, a, test=False): """ kw = {} kw['aciprefix'], kw['aciname'] = _parse_aci_name(a.name) + if pkey_only: + return kw kw['permissions'] = tuple(a.permissions) if 'targetattr' in a.target: kw['attrs'] = list(a.target['targetattr']['expression']) @@ -682,7 +685,8 @@ class aci_find(crud.Search): NO_CLI = True msg_summary = ngettext('%(count)d ACI matched', '%(count)d ACIs matched', 0) - takes_options = (_prefix_option.clone_rename("aciprefix?", required=False),) + takes_options = (_prefix_option.clone_rename("aciprefix?", required=False), + gen_pkey_only_option("name"),) def execute(self, term, **kw): ldap = self.api.Backend.ldap2 @@ -837,7 +841,8 @@ class aci_find(crud.Search): if kw.get('raw', False): aci = dict(aci=unicode(result)) else: - aci = _aci_to_kw(ldap, result) + aci = _aci_to_kw(ldap, result, + pkey_only=kw.get('pkey_only', False)) acis.append(aci) return dict( diff --git a/ipalib/plugins/baseldap.py b/ipalib/plugins/baseldap.py index 2fdcd2b744e1abc78189a1a81de78189e86f5f92..7c7053b8559d502a78a3d38e304b35d1c48c0f03 100644 --- a/ipalib/plugins/baseldap.py +++ b/ipalib/plugins/baseldap.py @@ -1540,6 +1540,12 @@ class LDAPRemoveMember(LDAPModMember): return +def gen_pkey_only_option(cli_name): + return Flag('pkey_only?', + label=_('Primary key only'), + doc=_('Results should contain primary key attribute only ("%s")') \ + % to_cli(cli_name),) + class LDAPSearch(BaseLDAPCommand, crud.Search): """ Retrieve all LDAP entries matching the given criteria. @@ -1582,11 +1588,7 @@ class LDAPSearch(BaseLDAPCommand, crud.Search): yield option if self.obj.primary_key and \ 'no_output' not in self.obj.primary_key.flags: - yield Flag('pkey_only?', - label=_('Primary key only'), - doc=_('Results should contain primary key attribute only ("%s")') \ - % to_cli(self.obj.primary_key.cli_name), - ) + yield gen_pkey_only_option(self.obj.primary_key.cli_name) for attr in self.member_attributes: for ldap_obj_name in self.obj.attribute_members[attr]: ldap_obj = self.api.Object[ldap_obj_name] diff --git a/ipalib/plugins/delegation.py b/ipalib/plugins/delegation.py index 5fe1511f2318af24c2a23279f0a3ab40fc4fae07..660425013b6622a1b4f21c0cb72d761537641bf8 100644 --- a/ipalib/plugins/delegation.py +++ b/ipalib/plugins/delegation.py @@ -1,5 +1,6 @@ # Authors: # Rob Crittenden <rcrit...@redhat.com> +# Martin Kosek <mko...@redhat.com> # # Copyright (C) 2010 Red Hat # see file 'COPYING' for use and warranty information @@ -24,6 +25,7 @@ from ipalib.request import context from ipalib import api, crud, errors from ipalib import output from ipalib import Object, Command +from ipalib.plugins.baseldap import gen_pkey_only_option __doc__ = _(""" Group to Group Delegation @@ -53,45 +55,6 @@ EXAMPLES: ACI_PREFIX=u"delegation" -def convert_delegation(ldap, aci): - """ - memberOf is in filter but we want to pull out the group for easier - displaying. - """ - filter = aci['memberof'] - st = filter.find('memberOf=') - if st == -1: - raise errors.NotFound(reason=_('Delegation \'%(permission)s\' not found') % dict(permission=aci['aciname'])) - en = filter.find(')', st) - membergroup = filter[st+9:en] - try: - (dn, entry_attrs) = ldap.get_entry(membergroup, ['cn']) - except Exception, e: - # Uh oh, the group we're granting access to has an error - msg = _('Error retrieving member group %(group)s: %(error)s') % (membergroup, str(e)) - raise errors.NonFatalError(reason=msg) - aci['memberof'] = entry_attrs['cn'][0] - - del aci['aciprefix'] # do not include prefix in result - - return aci - -def is_delegation(ldap, aciname): - """ - Determine if the ACI is a Delegation ACI and raise an exception if it - isn't. - - Return the result if it is a delegation ACI, adding a new attribute - membergroup. - """ - result = api.Command['aci_show'](aciname, aciprefix=ACI_PREFIX)['result'] - if 'memberof' in result: - result = convert_delegation(ldap, result) - else: - raise errors.NotFound(reason=_('Delegation \'%(permission)s\' not found') % dict(permission=aciname)) - return result - - class delegation(Object): """ Delegation object. @@ -163,8 +126,12 @@ class delegation_add(crud.Create): kw['permissions'] = (u'write',) kw['aciprefix'] = ACI_PREFIX result = api.Command['aci_add'](aciname, **kw)['result'] - if 'memberof' in result: - result = convert_delegation(ldap, result) + + # do not include prefix in result + try: + del result['aciprefix'] + except KeyError: + pass return dict( result=result, @@ -181,8 +148,6 @@ class delegation_del(crud.Delete): msg_summary = _('Deleted delegation "%(value)s"') def execute(self, aciname, **kw): - ldap = self.api.Backend.ldap2 - is_delegation(ldap, aciname) kw['aciprefix'] = ACI_PREFIX result = api.Command['aci_del'](aciname, **kw) return dict( @@ -199,12 +164,15 @@ class delegation_mod(crud.Update): msg_summary = _('Modified delegation "%(value)s"') def execute(self, aciname, **kw): - ldap = self.api.Backend.ldap2 - is_delegation(ldap, aciname) kw['aciprefix'] = ACI_PREFIX result = api.Command['aci_mod'](aciname, **kw)['result'] - if 'memberof' in result: - result = convert_delegation(ldap, result) + + # do not include prefix in result + try: + del result['aciprefix'] + except KeyError: + pass + return dict( result=result, value=aciname, @@ -220,17 +188,18 @@ class delegation_find(crud.Search): '%(count)d delegation matched', '%(count)d delegations matched', 0 ) + takes_options = (gen_pkey_only_option("name"),) + def execute(self, term, **kw): ldap = self.api.Backend.ldap2 kw['aciprefix'] = ACI_PREFIX - acis = api.Command['aci_find'](term, **kw)['result'] - results = [] - for aci in acis: + results = api.Command['aci_find'](term, **kw)['result'] + + for aci in results: + # do not include prefix in result try: - if 'memberof' in aci: - aci = convert_delegation(ldap, aci) - results.append(aci) - except errors.NotFound: + del aci['aciprefix'] + except KeyError: pass return dict( @@ -252,8 +221,12 @@ class delegation_show(crud.Retrieve): ) def execute(self, aciname, **kw): - ldap = self.api.Backend.ldap2 - result = is_delegation(ldap, aciname) + result = api.Command['aci_show'](aciname, aciprefix=ACI_PREFIX)['result'] + # do not include prefix in result + try: + del result['aciprefix'] + except KeyError: + pass return dict( result=result, value=aciname, diff --git a/ipalib/plugins/selfservice.py b/ipalib/plugins/selfservice.py index 902e16baf813508708c570b6cc7544612f4ab544..2db3764797aa2edbd4770498ed79e4773ddbe09f 100644 --- a/ipalib/plugins/selfservice.py +++ b/ipalib/plugins/selfservice.py @@ -24,6 +24,7 @@ from ipalib.request import context from ipalib import api, crud, errors from ipalib import output from ipalib import Object, Command +from ipalib.plugins.baseldap import gen_pkey_only_option __doc__ = _(""" Self-service Permissions @@ -182,6 +183,8 @@ class selfservice_find(crud.Search): '%(count)d selfservice matched', '%(count)d selfservices matched', 0 ) + takes_options = (gen_pkey_only_option("name"),) + def execute(self, term, **kw): kw['selfaci'] = True kw['aciprefix'] = ACI_PREFIX diff --git a/tests/test_xmlrpc/test_delegation_plugin.py b/tests/test_xmlrpc/test_delegation_plugin.py index 2131c5ad7796cdb3e2f420bc9f19533fbcc48110..dbfa5ff75fdc5ce9a7c1ff53a20da98f805db9c5 100644 --- a/tests/test_xmlrpc/test_delegation_plugin.py +++ b/tests/test_xmlrpc/test_delegation_plugin.py @@ -147,6 +147,22 @@ class test_delegation(Declarative): dict( + desc='Search for %r with --pkey-only' % delegation1, + command=('delegation_find', [delegation1], {'pkey_only' : True}), + expected=dict( + count=1, + truncated=False, + summary=u'1 delegation matched', + result=[ + { + 'aciname': delegation1, + }, + ], + ), + ), + + + dict( desc='Update %r' % delegation1, command=( 'delegation_mod', [delegation1], dict(permissions=u'read') diff --git a/tests/test_xmlrpc/test_permission_plugin.py b/tests/test_xmlrpc/test_permission_plugin.py index b0df800944005129a511c7016d6dfae88d737938..b71921174019294e632e3466bed6b6a6a7f287ef 100644 --- a/tests/test_xmlrpc/test_permission_plugin.py +++ b/tests/test_xmlrpc/test_permission_plugin.py @@ -269,6 +269,27 @@ class test_permission(Declarative): 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': lambda x: DN(x) == permission1_dn, + 'cn': [permission1], + }, + { + 'dn': lambda x: DN(x) == permission2_dn, + 'cn': [permission2], + }, + ], + ), + ), + + + dict( desc='Search for %r' % privilege1, command=('privilege_find', [privilege1], {}), expected=dict( diff --git a/tests/test_xmlrpc/test_selfservice_plugin.py b/tests/test_xmlrpc/test_selfservice_plugin.py index cb4b387decbfa4eda8128c3a786e78803a587dc1..e994bb32c4079d9179b16a58fc94c46bc49c13e1 100644 --- a/tests/test_xmlrpc/test_selfservice_plugin.py +++ b/tests/test_xmlrpc/test_selfservice_plugin.py @@ -137,6 +137,21 @@ class test_selfservice(Declarative): ), ), + dict( + desc='Search for %r with --pkey-only' % selfservice1, + command=('selfservice_find', [selfservice1], {'pkey_only' : True}), + expected=dict( + count=1, + truncated=False, + summary=u'1 selfservice matched', + result=[ + { + 'aciname': selfservice1, + }, + ], + ), + ), + dict( desc='Update %r' % selfservice1, -- 1.7.7.5
_______________________________________________ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
