Petr Viktorin wrote:
On 02/29/2012 11:14 AM, Jan Cholasta wrote:
On 29.2.2012 11:09, Petr Viktorin wrote:
On 02/28/2012 03:19 PM, Jan Cholasta wrote:
On 28.2.2012 11:54, Petr Viktorin wrote:
On 02/27/2012 10:44 PM, Rob Crittenden wrote:
Petr Viktorin wrote:
On 02/20/2012 08:51 PM, Rob Crittenden wrote:
Petr Viktorin wrote: says various config
are not marked Required, so entering an empty value for it will
validation (and IPA will blow up later when it expects a
None). Forexample the following:
$ ipa config-mod --groupsearch=
fails with AttributeError: 'NoneType' object has no attribute

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
both "No value supplied" and "Empty value supplied". The method
currently assumes the former, and skips validation entirely for
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
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
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 by
marking required config options as required.

This looks good but I think there are other things to protect in
as well such as the default e-mail domain. It is probably safe to
that everything in there is required.


Let me just double-check this with you.

According to code in the user plugin (around line 330), if the
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,

The rest of the values I left as not required are for optional
or limits: search results & time limit, max. username length,
expiry notification. Currently if these are missing, the
is disabled (well, except for the time limit).
But, there are also special values (0 or -1) that have the same
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?

I think we want to enforce that these are defined. It will be
for users if these are not there at all. I don't think we need to
the special options, just declare that the attribute is required.


Attaching updated patch 13.

Only the default e-mail domain
( and ipaconfigstring are
still optional.

You have removed all the config-related defensive code in the patch, is
this a good idea? What will happen if someone e.g. deletes a required
config attribute directly from LDAP?

Then IPA crashes. The defensive code wasn't there for all cases anyway,
as ticket #2159 shows.

If we want to protect against this it would probably be better to make
the config class itself give the default when a required value is

This, and raise an error in cases where no default is available (the
check should probably be done in ldap.get_ipa_config).


Would a better approach be to modify the LDAP schema to require these

I think that may be a longer-term fix. I propose we keep the defensive code in for now and correct it in the future.


Freeipa-devel mailing list

Reply via email to