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:
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?
I think we want to enforce that these are defined. It will be confusing
for users if these are not there at all. I don't think we need to show
the special options, just declare that the attribute is required.
rob
Attaching updated patch 13.
Only the default e-mail domain
(https://fedorahosted.org/freeipa/ticket/2409) and ipaconfigstring are
still optional.
--
Petr³
From a899c08e1b10c09f5c757d858db057e1d64f312f Mon Sep 17 00:00:00 2001
From: Petr Viktorin <pvikt...@redhat.com>
Date: Mon, 20 Feb 2012 04:03:27 -0500
Subject: [PATCH] Mark most config options as required
IPA assumes most config options are present, but allowed the user
to delete them. This patch marks them as required.
https://fedorahosted.org/freeipa/ticket/2159
Also, access to the options is changed to use indexing instead of get()
in all cases, as they no nonger need a default.
---
ipalib/plugins/config.py | 34 +++++++++++++++-------------------
ipalib/plugins/group.py | 2 +-
ipalib/plugins/migration.py | 4 ++--
ipalib/plugins/user.py | 12 ++++++------
ipaserver/plugins/ldap2.py | 8 ++------
ipaserver/plugins/selfsign.py | 2 +-
6 files changed, 27 insertions(+), 35 deletions(-)
diff --git a/ipalib/plugins/config.py b/ipalib/plugins/config.py
index c4615e3d1843a848e65090d64fd50fa833d81220..6da51af544dcb5b36eb257b2dacbdfddbdb540cb 100644
--- a/ipalib/plugins/config.py
+++ b/ipalib/plugins/config.py
@@ -97,22 +97,22 @@ class config(LDAPObject):
label_singular = _('Configuration')
takes_params = (
- Int('ipamaxusernamelength?',
+ Int('ipamaxusernamelength',
cli_name='maxusername',
label=_('Maximum username length'),
minvalue=1,
),
- IA5Str('ipahomesrootdir?',
+ IA5Str('ipahomesrootdir',
cli_name='homedirectory',
label=_('Home directory base'),
doc=_('Default location of home directories'),
),
- Str('ipadefaultloginshell?',
+ Str('ipadefaultloginshell',
cli_name='defaultshell',
label=_('Default shell'),
doc=_('Default shell for new users'),
),
- Str('ipadefaultprimarygroup?',
+ Str('ipadefaultprimarygroup',
cli_name='defaultgroup',
label=_('Default users group'),
doc=_('Default group for new users'),
@@ -122,52 +122,52 @@ class config(LDAPObject):
label=_('Default e-mail domain'),
doc=_('Default e-mail domain'),
),
- Int('ipasearchtimelimit?', validate_searchtimelimit,
+ Int('ipasearchtimelimit', validate_searchtimelimit,
cli_name='searchtimelimit',
label=_('Search time limit'),
doc=_('Maximum amount of time (seconds) for a search (> 0, or -1 for unlimited)'),
minvalue=-1,
),
- Int('ipasearchrecordslimit?',
+ Int('ipasearchrecordslimit',
cli_name='searchrecordslimit',
label=_('Search size limit'),
doc=_('Maximum number of records to search (-1 is unlimited)'),
minvalue=-1,
),
- IA5Str('ipausersearchfields?',
+ IA5Str('ipausersearchfields',
cli_name='usersearch',
label=_('User search fields'),
doc=_('A comma-separated list of fields to search in when searching for users'),
),
- IA5Str('ipagroupsearchfields?',
+ IA5Str('ipagroupsearchfields',
cli_name='groupsearch',
label='Group search fields',
doc=_('A comma-separated list of fields to search in when searching for groups'),
),
- Bool('ipamigrationenabled?',
+ Bool('ipamigrationenabled',
cli_name='enable_migration',
label=_('Enable migration mode'),
doc=_('Enable migration mode'),
),
- Str('ipacertificatesubjectbase?',
+ Str('ipacertificatesubjectbase',
cli_name='subject',
label=_('Certificate Subject base'),
doc=_('Base for certificate subjects (OU=Test,O=Example)'),
flags=['no_update'],
),
- Str('ipagroupobjectclasses*',
+ Str('ipagroupobjectclasses+',
cli_name='groupobjectclasses',
label=_('Default group objectclasses'),
doc=_('Default group objectclasses (comma-separated list)'),
csv=True,
),
- Str('ipauserobjectclasses*',
+ Str('ipauserobjectclasses+',
cli_name='userobjectclasses',
label=_('Default user objectclasses'),
doc=_('Default user objectclasses (comma-separated list)'),
csv=True,
),
- Int('ipapwdexpadvnotify?',
+ Int('ipapwdexpadvnotify',
cli_name='pwdexpnotify',
label=_('Password Expiration Notification (days)'),
doc=_('Number of days\'s notice of impending password expiration'),
@@ -180,11 +180,11 @@ class config(LDAPObject):
values=(u'AllowLMhash', u'AllowNThash'),
csv=True,
),
- Str('ipaselinuxusermaporder?',
+ Str('ipaselinuxusermaporder',
label=_('SELinux user map order'),
doc=_('Order in increasing priority of SELinux users, delimited by $'),
),
- Str('ipaselinuxusermapdefault?',
+ Str('ipaselinuxusermapdefault',
label=_('Default SELinux user'),
doc=_('Default SELinux user when no match is found in SELinux map rule'),
),
@@ -249,10 +249,6 @@ class config_mod(LDAPUpdate):
error=_('%(obj)s default attribute %(attr)s would not be allowed!') \
% dict(obj=obj, attr=obj_attr))
- if 'ipaselinuxusermapdefault' in options and options['ipaselinuxusermapdefault'] is None:
- raise errors.ValidationError(name='ipaselinuxusermapdefault',
- error=_('SELinux user map default user may not be empty'))
-
# Make sure the default user is in the list
if 'ipaselinuxusermapdefault' in options or \
'ipaselinuxusermaporder' in options:
diff --git a/ipalib/plugins/group.py b/ipalib/plugins/group.py
index b101d128565275ece89b0603305aeab04ff274df..d642311db3d03da09c2168142eee6faa0afed8f2 100644
--- a/ipalib/plugins/group.py
+++ b/ipalib/plugins/group.py
@@ -157,7 +157,7 @@ class group_del(LDAPDelete):
def pre_callback(self, ldap, dn, *keys, **options):
config = ldap.get_ipa_config()[1]
- def_primary_group = config.get('ipadefaultprimarygroup', '')
+ def_primary_group = config['ipadefaultprimarygroup']
def_primary_group_dn = group_dn = self.obj.get_dn(def_primary_group)
if dn == def_primary_group_dn:
raise errors.DefaultGroupError()
diff --git a/ipalib/plugins/migration.py b/ipalib/plugins/migration.py
index 688265fd3ea7f62bb22bf78abbc7f26e64f7470b..a58b2670f52851dacdaa14c4b67dbcbfe690b301 100644
--- a/ipalib/plugins/migration.py
+++ b/ipalib/plugins/migration.py
@@ -110,7 +110,7 @@ def _pre_migrate_user(ldap, pkey, dn, entry_attrs, failed, config, ctx, **kwargs
# get default primary group for new users
if 'def_group_dn' not in ctx:
- def_group = config.get('ipadefaultprimarygroup')
+ def_group = config['ipadefaultprimarygroup']
ctx['def_group_dn'] = api.Object.group.get_dn(def_group)
try:
(g_dn, g_attrs) = ldap.get_entry(ctx['def_group_dn'], ['gidnumber'])
@@ -628,7 +628,7 @@ can use their Kerberos accounts.''')
config = ldap.get_ipa_config()[1]
# check if migration mode is enabled
- if config.get('ipamigrationenabled', ('FALSE', ))[0] == 'FALSE':
+ if config['ipamigrationenabled'][0] == 'FALSE':
return dict(result={}, failed={}, enabled=False)
# connect to DS
diff --git a/ipalib/plugins/user.py b/ipalib/plugins/user.py
index d8da3a373ba613ccdeecc7e450aa4fb979bc73af..098eff0eb9ca3e21e8a8ed965a399009dd097ebe 100644
--- a/ipalib/plugins/user.py
+++ b/ipalib/plugins/user.py
@@ -416,21 +416,21 @@ class user_add(LDAPCreate):
validate_nsaccountlock(entry_attrs)
config = ldap.get_ipa_config()[1]
if 'ipamaxusernamelength' in config:
- if len(keys[-1]) > int(config.get('ipamaxusernamelength')[0]):
+ if len(keys[-1]) > int(config['ipamaxusernamelength'][0]):
raise errors.ValidationError(
name=self.obj.primary_key.cli_name,
error=_('can be at most %(len)d characters') % dict(
- len = int(config.get('ipamaxusernamelength')[0])
+ len = int(config['ipamaxusernamelength'][0])
)
)
- default_shell = config.get('ipadefaultloginshell', ['/bin/sh'])[0]
+ default_shell = config['ipadefaultloginshell'][0]
entry_attrs.setdefault('loginshell', default_shell)
# hack so we can request separate first and last name in CLI
full_name = '%s %s' % (entry_attrs['givenname'], entry_attrs['sn'])
entry_attrs.setdefault('cn', full_name)
if 'homedirectory' not in entry_attrs:
# get home's root directory from config
- homes_root = config.get('ipahomesrootdir', ['/home'])[0]
+ homes_root = config['ipahomesrootdir'][0]
# build user's home directory based on his uid
entry_attrs['homedirectory'] = posixpath.join(homes_root, keys[-1])
entry_attrs.setdefault('krbpwdpolicyreference', 'cn=global_policy,cn=%s,cn=kerberos,%s' % (api.env.realm, api.env.basedn))
@@ -444,7 +444,7 @@ class user_add(LDAPCreate):
else:
# we're adding new users to a default group, get its gidNumber
# get default group name from config
- def_primary_group = config.get('ipadefaultprimarygroup')
+ def_primary_group = config['ipadefaultprimarygroup']
group_dn = self.api.Object['group'].get_dn(def_primary_group)
try:
(group_dn, group_attrs) = ldap.get_entry(group_dn, ['gidnumber'])
@@ -469,7 +469,7 @@ class user_add(LDAPCreate):
def post_callback(self, ldap, dn, entry_attrs, *keys, **options):
config = ldap.get_ipa_config()[1]
# add the user we just created into the default primary group
- def_primary_group = config.get('ipadefaultprimarygroup')
+ def_primary_group = config['ipadefaultprimarygroup']
group_dn = self.api.Object['group'].get_dn(def_primary_group)
ldap.add_entry_to_group(dn, group_dn)
if self.api.env.wait_for_attr:
diff --git a/ipaserver/plugins/ldap2.py b/ipaserver/plugins/ldap2.py
index ffe2fba8ad050064e49297e6e743ab13f9b4678d..ec632dbe0c5637571e5c4cf20b7e2c107075869e 100644
--- a/ipaserver/plugins/ldap2.py
+++ b/ipaserver/plugins/ldap2.py
@@ -707,9 +707,9 @@ class ldap2(CrudBackend, Encoder):
if time_limit is None or size_limit is None:
(cdn, config) = self.get_ipa_config()
if time_limit is None:
- time_limit = config.get('ipasearchtimelimit', [-1])[0]
+ time_limit = config['ipasearchtimelimit'][0]
if size_limit is None:
- size_limit = config.get('ipasearchrecordslimit', [0])[0]
+ size_limit = config['ipasearchrecordslimit'][0]
if time_limit == 0:
time_limit = -1
if not isinstance(size_limit, int):
@@ -801,7 +801,6 @@ class ldap2(CrudBackend, Encoder):
size_limit=size_limit, normalize=normalize
)[0][0]
- config_defaults = {'ipasearchtimelimit': [2], 'ipasearchrecordslimit': [0]}
def get_ipa_config(self, attrs_list=None):
"""Returns the IPA configuration entry (dn, entry_attrs)."""
cdn = "%s,%s" % (api.Object.config.get_dn(), api.env.basedn)
@@ -818,9 +817,6 @@ class ldap2(CrudBackend, Encoder):
)[0][0]
except errors.NotFound:
config_entry = {}
- for a in self.config_defaults:
- if a not in config_entry:
- config_entry[a] = self.config_defaults[a]
setattr(context, 'config_entry', copy.deepcopy(config_entry))
return (cdn, config_entry)
diff --git a/ipaserver/plugins/selfsign.py b/ipaserver/plugins/selfsign.py
index 2f13b1fd583cba851f86ce0fe1897a09c2b1550f..f9e1c5e5a9fba3cde7356fe8d20fd42ab1ebfc16 100644
--- a/ipaserver/plugins/selfsign.py
+++ b/ipaserver/plugins/selfsign.py
@@ -86,7 +86,7 @@ class ra(rabase.rabase):
"""
try:
config = api.Command['config_show']()['result']
- subject_base = config.get('ipacertificatesubjectbase')[0]
+ subject_base = config['ipacertificatesubjectbase'][0]
hostname = get_csr_hostname(csr)
base = re.split(',\s*(?=\w+=)', subject_base)
base.insert(0,'CN=%s' % hostname)
--
1.7.7.6
_______________________________________________
Freeipa-devel mailing list
Freeipa-devel@redhat.com
https://www.redhat.com/mailman/listinfo/freeipa-devel
