[Freeipa-devel] [PATCH] 16 Netgroup nisdomain and hosts validation
Ondrej Hamada
ohamada at redhat.com
Fri Mar 23 22:10:28 UTC 2012
On 03/15/2012 08:13 AM, Martin Kosek wrote:
> On Wed, 2012-03-14 at 16:54 +0100, Ondrej Hamada wrote:
>> On 03/09/2012 04:34 PM, Martin Kosek wrote:
>>> On Thu, 2012-03-08 at 14:52 +0100, Ondrej Hamada wrote:
>>>> Netgroup nisdomain and hosts validation
>>>>
>>>> nisdomain validation:
>>>> Added pattern to the 'nisdomain' parameter to validate the specified
>>>> nisdomain name. According to most common use cases the same patter as
>>>> for netgroup should fit. Unit-tests added.
>>>>
>>>> https://fedorahosted.org/freeipa/ticket/2447
>>>>
>>>> hosts validation:
>>>> Added precallback to netgroup_add_member. It validates the specified
>>>> hostnames and raises ValidationError exception for invalid hostnames.
>>>> Unit-test added.
>>>>
>>>> https://fedorahosted.org/freeipa/ticket/2448
>>> I checked the host validation part and it could be improved. Issue
>>> described in #2447 (you have switched the ticket IDs) affects all
>>> objects that allow external hosts, users, ..., i.e. those who call
>>> add_external_post_callback in their post_callback.
>>>
>>> Should we fix all of these when we deal with this issue? Otherwise user
>>> could do something like this:
>>> # ipa sudorule-add-user foo --users=a+b
>>> Rule name: foo
>>> Enabled: TRUE
>>> External User: a+b
>>>
>>> We could create a similar function called add_external_pre_callback()
>>> and pass it attribute name and validating function (which would be
>>> common with the linked object). It would then do the validation for all
>>> these affected objects consistently and without redundant code.
>>>
>>> I didn't liked much the implemented pre_callback anyway
>>>
>>> + def pre_callback(self, ldap, dn, found, not_found, *keys,
>>> **options):
>>> + # validate entered hostnames
>>> + if 'host' in options:
>>> + invalid_hostnames=[]
>>> + for hostname in options['host']:
>>> + try:
>>> + validate_hostname(hostname, False)
>>> + except ValueError:
>>> + invalid_hostnames.append(hostname)
>>> + if invalid_hostnames:
>>> + raise errors.ValidationError(name='host',
>>> error='hostnames:\"%s\" contain invalid characters' %
>>> ','.join(invalid_hostnames))
>>> + return dn
>>>
>>> I would rather raise the ValidationError with the first invalid hostname
>>> and tell what's wrong (function validate_hostname tells it to you). If
>>> you go with the proposed approach, you wouldn't have to deal with
>>> formatting error messages, you would just raise the one returned by the
>>> validator shared with the linked LDAP object (hostname, user, ...).
>>>
>>> Martin
>> external_pre_callback function seems as a good idea, but there is a
>> problem how to get the validators for various LDAP objects. For the
>> hostname we already have one in ipalib.utils, but for the uid or group
>> name we use only patterns specified in the parameter objects.
>>
>> Below I propose solution how to use the already defined parameter
>> objects for validation (the only problem is that I have to assume, that
>> it is always the first parameter in takes_params). Do you think this is
>> a good approach?
> I think the approach is OK, it can just be much improved in order to get
> rid of the hardcoded parts. See comments below.
>
>> def add_external_pre_callback(memberattr, membertype, externalattr,
>> ldap, dn, found, not_found, *keys, **options):
>> """
>> Pre callback to validate external members.
>> """
>> if membertype in options:
>> validator = api.Object[membertype].takes_params[0]
> You can use api.Object[membertype].params[memberattr]
>
>> for value in options[membertype]:
>> try:
>> validator(value)
>> except errors.ValidationError as e:
>> error_msg = e[(e.find(':')+1):]
> You don't have to parse error message, you can just use e.name or
> e.error right from the caught ValidationError.
>
>> raise errors.ValidationError(name=membertype,
>> error=e[e.find(':')+1:])
>> return dn
>>
>
nisdomain validation:
Added pattern to the 'nisdomain' parameter to validate the specified
nisdomain name. According to most common use cases the same pattern as
for netgroup should fit. Unit-tests added.
https://fedorahosted.org/freeipa/ticket/2448
'add_external_pre_callback' function was created to allow validation of
all external members. Validation is based on usage of objects primary
key parameter. The 'add_external_pre_callback' fucntion has to be called
directly from in the 'pre_callback' function. This change affects
netgroup, hbacrule and sudorule commands.
Special validator is used only for hostname, the validator requires
fully qualified
domain name and enables the hostnames to contain underscores.
Unit-tests added.
https://fedorahosted.org/freeipa/ticket/2447
--
Regards,
Ondrej Hamada
FreeIPA team
jabber: ohama at jabbim.cz
IRC: ohamada
-------------- next part --------------
A non-text attachment was scrubbed...
Name: freeipa-ohamada-16-2-Netgroup-nisdomain-and-hosts-validation.patch
Type: text/x-patch
Size: 26208 bytes
Desc: not available
URL: <http://listman.redhat.com/archives/freeipa-devel/attachments/20120323/6ec34c0c/attachment.bin>
More information about the Freeipa-devel
mailing list