mirror of
https://salsa.debian.org/freeipa-team/freeipa.git
synced 2024-12-23 23:50:03 -06:00
Defer conversion and validation until after --{add,del,set}attr are handled
--addattr & friends that modified attributes known to Python sometimes used converted and validated Python values instead of LDAP strings. This caused a problem for --delattr, which searched for a converted integer in a list of raw strings (ticket 2407). With this patch we work on raw strings, converting only when done. Deferring validation ensures the end result is valid, so proper errors are raised instead of failing later (ticket 2405). Tests included. Replaces previous fix for: https://fedorahosted.org/freeipa/ticket/2418 Fixes: https://fedorahosted.org/freeipa/ticket/2405 https://fedorahosted.org/freeipa/ticket/2407 https://fedorahosted.org/freeipa/ticket/2408
This commit is contained in:
parent
a5a4323946
commit
8a7d7aaf81
@ -794,8 +794,6 @@ last, after all sets and adds."""),
|
||||
Convert a string in the form of name/value pairs into a dictionary.
|
||||
The incoming attribute may be a string or a list.
|
||||
|
||||
Any attribute found that is also a param is validated.
|
||||
|
||||
:param attrs: A list of name/value pairs
|
||||
|
||||
:param append: controls whether this returns a list of values or a single
|
||||
@ -811,15 +809,6 @@ last, after all sets and adds."""),
|
||||
if len(value) == 0:
|
||||
# None means "delete this attribute"
|
||||
value = None
|
||||
if attr in self.params:
|
||||
try:
|
||||
value = self.params[attr](value)
|
||||
except errors.ValidationError, err:
|
||||
raise errors.ValidationError(name=attr, error=err.error)
|
||||
except errors.ConversionError, err:
|
||||
raise errors.ValidationError(name=attr, error=err.error)
|
||||
if self.api.env.in_server:
|
||||
value = self.params[attr].encode(value)
|
||||
if append and attr in newdict:
|
||||
if type(value) in (tuple,):
|
||||
newdict[attr] += list(value)
|
||||
@ -923,12 +912,41 @@ last, after all sets and adds."""),
|
||||
# normalize all values
|
||||
changedattrs = setattrs | addattrs | delattrs
|
||||
for attr in changedattrs:
|
||||
# remove duplicite and invalid values
|
||||
entry_attrs[attr] = list(set([val for val in entry_attrs[attr] if val]))
|
||||
if not entry_attrs[attr]:
|
||||
entry_attrs[attr] = None
|
||||
elif isinstance(entry_attrs[attr], (tuple, list)) and len(entry_attrs[attr]) == 1:
|
||||
entry_attrs[attr] = entry_attrs[attr][0]
|
||||
if attr in self.obj.params:
|
||||
# convert single-value params to scalars
|
||||
value = entry_attrs[attr]
|
||||
try:
|
||||
param = self.params[attr]
|
||||
except KeyError:
|
||||
# The CRUD classes filter their disallowed parameters out.
|
||||
# Yet {set,add,del}attr are powerful enough to change these
|
||||
# (e.g. Config's ipacertificatesubjectbase)
|
||||
# So, use the parent's attribute
|
||||
param = self.obj.params[attr]
|
||||
if not param.multivalue:
|
||||
if len(value) == 1:
|
||||
value = value[0]
|
||||
elif not value:
|
||||
value = None
|
||||
else:
|
||||
raise errors.OnlyOneValueAllowed(attr=attr)
|
||||
# validate, convert and encode params
|
||||
try:
|
||||
value = param(value)
|
||||
except errors.ValidationError, err:
|
||||
raise errors.ValidationError(name=attr, error=err.error)
|
||||
except errors.ConversionError, err:
|
||||
raise errors.ConversionError(name=attr, error=err.error)
|
||||
value = param.encode(value)
|
||||
entry_attrs[attr] = value
|
||||
else:
|
||||
# unknown attribute: remove duplicite and invalid values
|
||||
entry_attrs[attr] = list(set([val for val in entry_attrs[attr] if val]))
|
||||
if not entry_attrs[attr]:
|
||||
entry_attrs[attr] = None
|
||||
elif isinstance(entry_attrs[attr], (tuple, list)) and len(entry_attrs[attr]) == 1:
|
||||
entry_attrs[attr] = entry_attrs[attr][0]
|
||||
|
||||
|
||||
class LDAPCreate(BaseLDAPCommand, crud.Create):
|
||||
"""
|
||||
|
@ -402,4 +402,117 @@ class test_attr(Declarative):
|
||||
),
|
||||
),
|
||||
|
||||
dict(
|
||||
desc='Lock %r using setattr' % user1,
|
||||
command=(
|
||||
'user_mod', [user1], dict(setattr=u'nsaccountlock=TrUe')
|
||||
),
|
||||
expected=dict(
|
||||
result=dict(
|
||||
givenname=[u'Finkle'],
|
||||
homedirectory=[u'/home/tuser1'],
|
||||
loginshell=[u'/bin/sh'],
|
||||
sn=[u'User1'],
|
||||
uid=[user1],
|
||||
uidnumber=[fuzzy_digits],
|
||||
gidnumber=[fuzzy_digits],
|
||||
mail=[u'test@example.com', u'test2@example.com'],
|
||||
memberof_group=[u'ipausers'],
|
||||
telephonenumber=[u'202-888-9833'],
|
||||
nsaccountlock=True,
|
||||
has_keytab=False,
|
||||
has_password=False,
|
||||
),
|
||||
summary=u'Modified user "tuser1"',
|
||||
value=user1,
|
||||
),
|
||||
),
|
||||
|
||||
dict(
|
||||
desc='Unlock %r using addattr&delattr' % user1,
|
||||
command=(
|
||||
'user_mod', [user1], dict(
|
||||
addattr=u'nsaccountlock=FaLsE',
|
||||
delattr=u'nsaccountlock=True')
|
||||
),
|
||||
expected=dict(
|
||||
result=dict(
|
||||
givenname=[u'Finkle'],
|
||||
homedirectory=[u'/home/tuser1'],
|
||||
loginshell=[u'/bin/sh'],
|
||||
sn=[u'User1'],
|
||||
uid=[user1],
|
||||
uidnumber=[fuzzy_digits],
|
||||
gidnumber=[fuzzy_digits],
|
||||
mail=[u'test@example.com', u'test2@example.com'],
|
||||
memberof_group=[u'ipausers'],
|
||||
telephonenumber=[u'202-888-9833'],
|
||||
nsaccountlock=False,
|
||||
has_keytab=False,
|
||||
has_password=False,
|
||||
),
|
||||
summary=u'Modified user "tuser1"',
|
||||
value=user1,
|
||||
),
|
||||
),
|
||||
|
||||
dict(
|
||||
desc='Try adding a new group search fields config entry',
|
||||
command=(
|
||||
'config_mod', [], dict(addattr=u'ipagroupsearchfields=newattr')
|
||||
),
|
||||
expected=errors.OnlyOneValueAllowed(attr='ipagroupsearchfields'),
|
||||
),
|
||||
|
||||
dict(
|
||||
desc='Try adding a new cert subject base config entry',
|
||||
command=(
|
||||
'config_mod', [], dict(addattr=u'ipacertificatesubjectbase=0=DOMAIN.COM')
|
||||
),
|
||||
expected=errors.OnlyOneValueAllowed(attr='ipacertificatesubjectbase'),
|
||||
),
|
||||
|
||||
dict(
|
||||
desc='Try deleting a required config entry',
|
||||
command=(
|
||||
'config_mod', [], dict(delattr=u'ipasearchrecordslimit=100')
|
||||
),
|
||||
expected=errors.RequirementError(name='ipasearchrecordslimit'),
|
||||
),
|
||||
|
||||
dict(
|
||||
desc='Try setting nonexistent attribute',
|
||||
command=('config_mod', [], dict(setattr=u'invalid_attr=false')),
|
||||
expected=errors.ObjectclassViolation(
|
||||
info='attribute "invalid_attr" not allowed'),
|
||||
),
|
||||
|
||||
dict(
|
||||
desc='Try setting out-of-range krbpwdmaxfailure',
|
||||
command=('pwpolicy_mod', [], dict(setattr=u'krbpwdmaxfailure=-1')),
|
||||
expected=errors.ValidationError(name='krbpwdmaxfailure',
|
||||
error='must be at least 0'),
|
||||
),
|
||||
|
||||
dict(
|
||||
desc='Try setting out-of-range maxfail',
|
||||
command=('pwpolicy_mod', [], dict(krbpwdmaxfailure=u'-1')),
|
||||
expected=errors.ValidationError(name='maxfail',
|
||||
error='must be at least 0'),
|
||||
),
|
||||
|
||||
dict(
|
||||
desc='Try setting non-numeric krbpwdmaxfailure',
|
||||
command=('pwpolicy_mod', [], dict(setattr=u'krbpwdmaxfailure=abc')),
|
||||
expected=errors.ConversionError(name='krbpwdmaxfailure',
|
||||
error='must be an integer'),
|
||||
),
|
||||
|
||||
dict(
|
||||
desc='Try setting non-numeric maxfail',
|
||||
command=('pwpolicy_mod', [], dict(krbpwdmaxfailure=u'abc')),
|
||||
expected=errors.ConversionError(name='maxfail',
|
||||
error='must be an integer'),
|
||||
),
|
||||
|
||||
]
|
||||
|
Loading…
Reference in New Issue
Block a user