This is the second and likely the next-to-last part of disabling extra
command options (after this it's just test fixes and turning the
checking on).
Part of the work for https://fedorahosted.org/freeipa/ticket/2509
--
Petr³
From f504ab30d10e9274ee67da00e4d8171263143db7 Mon Sep 17 00:00:00 2001
From: Petr Viktorin <pvikt...@redhat.com>
Date: Tue, 17 Apr 2012 12:48:33 -0400
Subject: [PATCH] Do not use extra command options in ACI, permission,
selfservice
Allowing Commands to be called with ignored unknown options opens the
door to problems, for example with misspelled option names.
Before we start rejecting them, we need to make sure IPA itself does
not use them when it calls commands internally.
This patch does that for ACI-related plugins.
Part of the work for https://fedorahosted.org/freeipa/ticket/2509
---
ipalib/plugins/aci.py | 26 +++++++++-----------
ipalib/plugins/permission.py | 33 +++++++++++---------------
ipalib/plugins/selfservice.py | 3 +--
tests/test_xmlrpc/test_selfservice_plugin.py | 3 ++-
4 files changed, 28 insertions(+), 37 deletions(-)
diff --git a/ipalib/plugins/aci.py b/ipalib/plugins/aci.py
index f0b81f48af1f9fbf8ab267a3d4b113c328ab1170..b0be26f5c44e51d91af3bdf7d0a94c0a14f8fe4a 100644
--- a/ipalib/plugins/aci.py
+++ b/ipalib/plugins/aci.py
@@ -565,21 +565,20 @@ class aci_del(crud.Delete):
takes_options = (_prefix_option,)
- def execute(self, aciname, **kw):
+ def execute(self, aciname, aciprefix):
"""
Execute the aci-delete operation.
:param aciname: The name of the ACI being deleted.
- :param kw: unused
+ :param aciprefix: The ACI prefix.
"""
- assert 'aciname' not in kw
ldap = self.api.Backend.ldap2
(dn, entry_attrs) = ldap.get_entry(self.api.env.basedn, ['aci'])
acistrs = entry_attrs.get('aci', [])
acis = _convert_strings_to_acis(acistrs)
- aci = _find_aci_by_name(acis, kw['aciprefix'], aciname)
+ aci = _find_aci_by_name(acis, aciprefix, aciname)
for a in acistrs:
candidate = ACI(a)
if aci.isequal(candidate):
@@ -614,45 +613,42 @@ class aci_mod(crud.Update):
msg_summary = _('Modified ACI "%(value)s"')
def execute(self, aciname, **kw):
+ aciprefix = kw['aciprefix']
ldap = self.api.Backend.ldap2
(dn, entry_attrs) = ldap.get_entry(self.api.env.basedn, ['aci'])
acis = _convert_strings_to_acis(entry_attrs.get('aci', []))
- aci = _find_aci_by_name(acis, kw['aciprefix'], aciname)
+ aci = _find_aci_by_name(acis, aciprefix, aciname)
# The strategy here is to convert the ACI we're updating back into
# a series of keywords. Then we replace any keywords that have been
# updated and convert that back into an ACI and write it out.
oldkw = _aci_to_kw(ldap, aci)
newkw = deepcopy(oldkw)
- if 'selfaci' in newkw and newkw['selfaci'] == True:
+ if newkw.get('selfaci', False):
# selfaci is set in aci_to_kw to True only if the target is self
kw['selfaci'] = True
- for k in kw.keys():
- newkw[k] = kw[k]
+ newkw.update(kw)
for acikw in (oldkw, newkw):
- try:
- del acikw['aciname']
- except KeyError:
- pass
+ acikw.pop('aciname', None)
# _make_aci is what is run in aci_add and validates the input.
# Do this before we delete the existing ACI.
newaci = _make_aci(ldap, None, aciname, newkw)
if aci.isequal(newaci):
raise errors.EmptyModlist()
- self.api.Command['aci_del'](aciname, **kw)
+ self.api.Command['aci_del'](aciname, aciprefix=aciprefix)
try:
result = self.api.Command['aci_add'](aciname, **newkw)['result']
except Exception, e:
# ACI could not be added, try to restore the old deleted ACI and
# report the ADD error back to user
try:
self.api.Command['aci_add'](aciname, **oldkw)
- except:
+ except Exception:
pass
raise e
@@ -949,7 +945,7 @@ def execute(self, aciname, **kw):
# Do this before we delete the existing ACI.
newaci = _make_aci(ldap, None, kw['newname'], newkw)
- self.api.Command['aci_del'](aciname, **kw)
+ self.api.Command['aci_del'](aciname, aciprefix=kw['aciprefix'])
result = self.api.Command['aci_add'](kw['newname'], **newkw)['result']
diff --git a/ipalib/plugins/permission.py b/ipalib/plugins/permission.py
index 2cf42bbc07c769044d31396d917734cb3c0e17b2..a8afd141e3a24d39cec0a39682bfb9aab331ddc9 100644
--- a/ipalib/plugins/permission.py
+++ b/ipalib/plugins/permission.py
@@ -194,10 +194,7 @@ def pre_callback(self, ldap, dn, entry_attrs, attrs_list, *keys, **options):
opts['test'] = True
opts['permission'] = keys[-1]
opts['aciprefix'] = ACI_PREFIX
- try:
- self.api.Command.aci_add(keys[-1], **opts)
- except Exception, e:
- raise e
+ self.api.Command.aci_add(keys[-1], **opts)
# Clear the aci attributes out of the permission entry
for o in options:
@@ -289,24 +286,20 @@ def pre_callback(self, ldap, dn, entry_attrs, attrs_list, *keys, **options):
except errors.NotFound:
pass # permission may be renamed, continue
else:
- raise errors.ValidationError(name='rename',error=_('New name can not be empty'))
+ raise errors.ValidationError(
+ name='rename',error=_('New name can not be empty'))
opts = copy.copy(options)
- for o in ['all', 'raw', 'rights', 'rename']:
- if o in opts:
- del opts[o]
+ for o in ['all', 'raw', 'rights', 'test', 'rename']:
+ opts.pop(o, None)
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['test'] = False
opts['permission'] = keys[-1]
opts['aciprefix'] = ACI_PREFIX
- try:
- self.api.Command.aci_mod(keys[-1], **opts)
- setattr(context, 'aciupdate', True)
- except Exception, e:
- raise e
+ 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:
@@ -341,11 +334,12 @@ def post_callback(self, ldap, dn, entry_attrs, *keys, **options):
permission=options['rename'])
self.api.Command.aci_rename(cn, aciprefix=ACI_PREFIX,
- newname=options['rename'], newprefix=ACI_PREFIX)
+ newname=options['rename'])
cn = options['rename'] # rename finished
- result = self.api.Command.permission_show(cn, **options)['result']
+ common_options = dict((k, options[k]) for k in ('all', 'raw') if k in options)
+ result = self.api.Command.permission_show(cn, **common_options)['result']
for r in result:
if not r.startswith('member_'):
entry_attrs[r] = result[r]
@@ -363,7 +357,7 @@ class permission_find(LDAPSearch):
has_output_params = LDAPSearch.has_output_params + output_params
def post_callback(self, ldap, entries, truncated, *args, **options):
- if options.get('pkey_only', False):
+ if options.pop('pkey_only', False):
return
for entry in entries:
(dn, attrs) = entry
@@ -379,9 +373,9 @@ def post_callback(self, ldap, entries, truncated, *args, **options):
# 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.
- options['aciprefix'] = ACI_PREFIX
opts = copy.copy(options)
+ opts['aciprefix'] = ACI_PREFIX
try:
# permission ACI attribute is needed
del opts['raw']
@@ -415,7 +409,8 @@ class permission_show(LDAPRetrieve):
has_output_params = LDAPRetrieve.has_output_params + output_params
def post_callback(self, ldap, dn, entry_attrs, *keys, **options):
try:
- aci = self.api.Command.aci_show(keys[-1], aciprefix=ACI_PREFIX, **options)['result']
+ common_options = dict((k, options[k]) for k in ('all', 'raw') if k in options)
+ 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]
diff --git a/ipalib/plugins/selfservice.py b/ipalib/plugins/selfservice.py
index a60475b7cc7b109fdebdfecdbf71408d6041a0ba..82f2a0cc0efae8d0fb6ea862228f7a70d70ddccc 100644
--- a/ipalib/plugins/selfservice.py
+++ b/ipalib/plugins/selfservice.py
@@ -149,8 +149,7 @@ class selfservice_del(crud.Delete):
msg_summary = _('Deleted selfservice "%(value)s"')
def execute(self, aciname, **kw):
- kw['aciprefix'] = ACI_PREFIX
- result = api.Command['aci_del'](aciname, **kw)
+ result = api.Command['aci_del'](aciname, aciprefix=ACI_PREFIX)
self.obj.postprocess_result(result)
return dict(
diff --git a/tests/test_xmlrpc/test_selfservice_plugin.py b/tests/test_xmlrpc/test_selfservice_plugin.py
index e60eb5d52373250d0f62e178e36f5fa602a0c311..fa67cbc2d8c730dfd3ec63507739d4363298c8d2 100644
--- a/tests/test_xmlrpc/test_selfservice_plugin.py
+++ b/tests/test_xmlrpc/test_selfservice_plugin.py
@@ -46,7 +46,8 @@ class test_selfservice(Declarative):
dict(
desc='Try to update non-existent %r' % selfservice1,
- command=('selfservice_mod', [selfservice1], dict(description=u'Foo')),
+ command=('selfservice_mod', [selfservice1],
+ dict(permissions=u'write')),
expected=errors.NotFound(
reason=u'ACI with name "%s" not found' % selfservice1),
),
--
1.7.10.1
_______________________________________________
Freeipa-devel mailing list
Freeipa-devel@redhat.com
https://www.redhat.com/mailman/listinfo/freeipa-devel
