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Ā³

_______________________________________________
Freeipa-devel mailing list
Freeipa-devel@redhat.com
https://www.redhat.com/mailman/listinfo/freeipa-devel

Reply via email to