On 04/12/2012 04:17 AM, Martin Kosek wrote:
On Wed, 2012-04-11 at 22:05 -0400, John Dennis wrote:
Revised patch attached. We'll leave the DN parameter changes till later.
This is essentially the same as the original patch with the addition of
the fixes necessary to support passing an empty container arg, an issue
Martin discovered in his review. FWIW the answer was not to make the
param required (actually it would have been adding the flag 'nonempty')
because you should be able to say you don't want to introduce a
container into the search bases (see commit comment)
I don't agree with the removal of default values for the containers and
allowing an empty value for them. Please, see my reasoning:
1) I don't think its unlikely to have ou=People and ou=groups as
containers for users/groups as they are default containers in fresh LDAP
installs. I think most of the small LDAP deployments will use these
values.
2) I am also not sure if somebody would want to pass empty user and
group container. Users and groups won't be shared in the same container
and since we search with _ldap.SCOPE_ONELEVEL the migration would not
find users or groups in containers nested under the search base anyway.
OK. Patch is revised, restored the defaults, usercontainer and
groupcontainer are now required to be non-empty. Also, basedn had been
optional without a default which didn't make much sense, now basedn is a
required parameter.
--
John Dennis <jden...@redhat.com>
Looking to carve out IT costs?
www.redhat.com/carveoutcosts/
>From fe05239e22b6c754aff22dd3e149e12e9d9a5bf7 Mon Sep 17 00:00:00 2001
From: John Dennis <jden...@redhat.com>
Date: Wed, 11 Apr 2012 21:51:17 -0400
Subject: [PATCH 72-2] Validate DN & RDN parameters for migrate command
Content-Type: text/plain; charset="utf-8"
Content-Transfer-Encoding: 8bit
Ticket #2555
We were generating a traceback (server error) if a malformed RDN was
passed as a parameter to the migrate command.
* add parameter validation functions validate_dn_param() and
validate_rdn_param() to ipalib.util. Those functions simply invoke
the DN or RDN constructor from our dn module passing it the string
representation. If the constructor does not throw an error it's
valid.
* Add the parameter validation function pointers to the Param objects
in the migrate command.
* Require that usercontainer and groupcontainer is nonempty, thus
passing --usercontainer= on the command line will produce
ipa: ERROR: 'user_container' is required
* Fix _get_search_bases() so if a container dn is empty it it just
uses the base dn alone instead of faulting (currently
bullet-proofing because now the containers are required).
* Update the doc for usercontainer and groupcontainer to reflect the
fact they are DN's not RDN's. A RDN can only be one level and it
should be possible to have a container more than one RDN removed
from the base.
* Make the basedn required, formerly it was optional without a default
which really doesn't make much sense.
---
ipalib/plugins/migration.py | 24 +++++++++++++++---------
ipalib/util.py | 16 ++++++++++++++++
2 files changed, 31 insertions(+), 9 deletions(-)
diff --git a/ipalib/plugins/migration.py b/ipalib/plugins/migration.py
index 873ff4c..97bea25 100644
--- a/ipalib/plugins/migration.py
+++ b/ipalib/plugins/migration.py
@@ -23,6 +23,7 @@ import ldap as _ldap
from ipalib import api, errors, output
from ipalib import Command, Password, Str, Flag, StrEnum
from ipalib.cli import to_cli
+from ipalib.util import validate_dn_param
from ipalib.dn import *
from ipalib.plugins.user import NO_UPG_MAGIC
if api.env.in_server and api.env.context in ['lite', 'server']:
@@ -418,25 +419,27 @@ class migrate_ds(Command):
)
takes_options = (
- Str('binddn?',
+ Str('binddn?', validate_dn_param,
cli_name='bind_dn',
label=_('Bind DN'),
default=u'cn=directory manager',
autofill=True,
),
- Str('usercontainer?',
+ Str('usercontainer?', validate_dn_param,
cli_name='user_container',
label=_('User container'),
- doc=_('RDN of container for users in DS relative to base DN'),
+ doc=_('DN of container for users in DS relative to base DN'),
default=u'ou=people',
autofill=True,
+ flags = ['nonempty'],
),
- Str('groupcontainer?',
+ Str('groupcontainer?', validate_dn_param,
cli_name='group_container',
label=_('Group container'),
- doc=_('RDN of container for groups in DS relative to base DN'),
+ doc=_('DN of container for groups in DS relative to base DN'),
default=u'ou=groups',
autofill=True,
+ flags = ['nonempty'],
),
Str('userobjectclass*',
cli_name='user_objectclass',
@@ -505,7 +508,7 @@ class migrate_ds(Command):
doc=_('Continuous operation mode. Errors are reported but the process continues'),
default=False,
),
- Str('basedn?',
+ Str('basedn',
cli_name='base_dn',
label=_('Base DN'),
doc=_('Base DN on remote LDAP server'),
@@ -589,9 +592,12 @@ can use their Kerberos accounts.''')
def _get_search_bases(self, options, ds_base_dn, migrate_order):
search_bases = dict()
for ldap_obj_name in migrate_order:
- search_bases[ldap_obj_name] = '%s,%s' % (
- options['%scontainer' % to_cli(ldap_obj_name)], ds_base_dn
- )
+ container = options.get('%scontainer' % to_cli(ldap_obj_name))
+ if container:
+ search_base = str(DN(container, ds_base_dn))
+ else:
+ search_base = ds_base_dn
+ search_bases[ldap_obj_name] = search_base
return search_bases
def migrate(self, ldap, config, ds_ldap, ds_base_dn, options):
diff --git a/ipalib/util.py b/ipalib/util.py
index a79f41c..487d278 100644
--- a/ipalib/util.py
+++ b/ipalib/util.py
@@ -31,6 +31,7 @@ from weakref import WeakKeyDictionary
from ipalib import errors
from ipalib.text import _
+from ipalib.dn import DN, RDN
from ipapython import dnsclient
from ipapython.ipautil import decode_ssh_pubkey
@@ -484,3 +485,18 @@ def gen_dns_update_policy(realm, rrtypes=('A', 'AAAA', 'SSHFP')):
policy += ";"
return policy
+
+def validate_rdn_param(ugettext, value):
+ try:
+ rdn = RDN(value)
+ except Exception, e:
+ return str(e)
+ return None
+
+def validate_dn_param(ugettext, value):
+ try:
+ rdn = DN(value)
+ except Exception, e:
+ return str(e)
+ return None
+
--
1.7.7.6
_______________________________________________
Freeipa-devel mailing list
Freeipa-devel@redhat.com
https://www.redhat.com/mailman/listinfo/freeipa-devel
