[Freeipa-devel] [PATCHES] 0012-13 Don't allow deleting required config options

[Petr Viktorin]

On 02/20/2012 08:51 PM, Rob Crittenden wrote:
> Petr Viktorin wrote:
>> https://fedorahosted.org/freeipa/ticket/2159 says various config options
>> are not marked Required, so entering an empty value for it will pass
>> validation (and IPA will blow up later when it expects a string,not
>> None). Forexample the following:
>> $ ipa config-mod --groupsearch=
>> fails with AttributeError: 'NoneType' object has no attribute 'split'
>>
>> There is a more general problem behind this, though: even if the
>> attributes *are* marked as Required, an empty string will pass
>> validation. This is because `None` is used in `Param.validate` to mean
>> both "No value supplied" and "Empty value supplied". The method
>> currently assumes the former, and skips validation entirely for `None`
>> values to optional parameters.
>>
>> For example, the following will delete "membergroup", even though it's a
>> required attribute :
>>
>> $ ipa delegation-add --attrs=street --group=editors \
>> --membergroup=admins td1
>> $ ipa delegation-mod --membergroup= td1
>>
>> Note that some LDAPObjects handle this with a _check_empty_attrs
>> function, so they aren't affected. That function is specific to LADP
>> objects, though. So I needed to tackle this on a lower level.
>>
>> This patch solves the problem by
>> * adding a 'nonempty' flag when a required parameter of a CRUD Update
>> object is auto-converted to a non-required parameter
>> * making the`validate` method aware of whether the parameter was
>> supplied; and if it was, honor the "nonempty" flag.
>>
>>
>> The second patch fixes https://fedorahosted.org/freeipa/ticket/2159 by
>> marking required config options as required.
>
> This looks good but I think there are other things to protect in config
> as well such as the default e-mail domain. It is probably safe to say
> that everything in there is required.
>
> rob

Let me just double-check this with you.

According to code in the user plugin (around line 330), if the default 
e-mail domain is not set, users don't get an address auto-assigned. Do 
we really want to require user e-mails?

ipaconfigstring (the password plugin flags) are a set (multivalue, not 
required).

The rest of the values I left as not required are for optional features 
or limits: search results & time limit, max. username length, password 
expiry notification. Currently if these are missing, the feature/limit 
is disabled (well, except for the time limit).
But, there are also special values (0 or -1) that have the same effect 
as a missing value. Sometimes they're documented.
So we want to enforce that users use these special values instead of 
removing the config entry?


-- 
Petr³