Martin Kosek wrote:
On Wed, 2012-03-28 at 17:28 -0400, Rob Crittenden wrote:
Martin Kosek wrote:
On Thu, 2012-03-22 at 15:21 -0400, Rob Crittenden wrote:
We don't want to create private groups automatically for migrated users,
there could be namespace overlap (and race conditions prevent us from
trying to check in advance).
Check the sanity of groups in general, warn if the group for the
gidnumber doesn't exist at least on the remote server.
Fill in the user's shell as well.
This will migrate users that are non-POSIX on the remote server.
rob
This patch fixes the problem of creating UPGs for migrated users, but
there are several parts which confused me.
1) Confusing defaults
+ if 'def_group_gid' in ctx:
+ entry_attrs.setdefault('gidnumber', ctx['def_group_gid'])
This statement seems redundant, because the account either is posix and
has both uidnumber and gidnumber or it is non-posix and does not have
any.
Now, we don't touch gidNumber for posix user, and non-posix users are
made posix with this statement:
+ # migrated user is not already POSIX, make it so
+ if 'uidnumber' not in entry_attrs:
+ entry_attrs['uidnumber'] = entry_attrs['gidnumber'] = [999]
2) Missing UPG
When UPG is disabled, the following statement will result in a user with
a GID pointing to non-existent group.
+ # migrated user is not already POSIX, make it so
+ if 'uidnumber' not in entry_attrs:
+ entry_attrs['uidnumber'] = entry_attrs['gidnumber'] = [999]
We may want to run ldap.has_upg() and report a add this user to "failed
users" with appropriate error.
3) Check for GID
The patch implements a check if a group with the gidNumber exists on a
remote LDAP server and the result is a warning:
- (g_dn, g_attrs) = ldap.get_entry(ctx['def_group_dn'],
['gidnumber'])
+ (remote_dn, remote_entry) = ds_ldap.find_entry_by_attr(
+ 'gidnumber', entry_attrs['gidnumber'][0], 'posixgroup', [''],
+ search_bases['group']
+ )
I have few (minor-ish) questions there:
a) Is the warning in a apache log enough? Maybe it should be included in
migrate-ds output.
b) This will be a one more remote LDAP call for every user, we may want
to optimize it with something like that:
valid_gids = []
if user.gidnumber not in valid_gids:
run the check in remote LDAP
valid_gids.append(user.gidnumber)
That would save us 999 LDAP queries for 1000 migrated with the same
primary group.
4) Extraneous Warning:
When non-posix user is migrated and thus we make it a posix user, we
still produce a warning for non-existent group:
[Fri Mar 23 04:21:36 2012] [error] ipa: WARNING: Migrated user's GID
number 999 does not point to a known group.
5) Extraneous LDAP call
Isn't the following call to LDAP to get a description redundant? We
already have the description in entry_attrs.
+ (dn, desc_attr) = ldap.get_entry(dn, ['description'])
+ entry_attrs.update(desc_attr)
+ if 'description' in entry_attrs and NO_UPG_MAGIC in
entry_attrs['description']:
Martin
I think this covers your concerns.
I can't do anything but log warnings at this point in order to maintain
backwards compatibility. I looked into returning a warning entry and it
blew up in validate_output() on older clients.
rob
This patch is much better and covers my previous concerns. I just find
an issue with UPG. It is not created for non-posix users when UPGs are
enabled:
# echo "Secret123" | ipa migrate-ds ldap://ldap.example.com
--with-compat --base-dn="dc=greyoak,dc=com"
-----------
migrate-ds:
-----------
Migrated:
user: darcee_leeson, ayaz_kreiger, mnonposix, mollee_weisenberg
group: ipagroup
Failed user:
Failed group:
----------
Passwords have been migrated in pre-hashed format.
IPA is unable to generate Kerberos keys unless provided
with clear text passwords. All migrated users need to
login at https://your.domain/ipa/migration/ before they
can use their Kerberos accounts.
# ipa user-show mnonposix
User login: mnonposix
First name: Mister
Last name: Nonposix
Home directory: /home/mnonposix
Login shell: /bin/sh
UID: 328000195
GID: 328000195
Org. Unit: Product Testing
Job Title: Test User
Account disabled: False
Password: True
Member of groups: ipausers
Kerberos keys available: False
# ipa group-show mnonposix
ipa: ERROR: mnonposix: group not found
Yes, I was always disabling UPG. I now allow it when migrating a
non-POSIX user.
I am also thinking if we need to ask if UPG is enabled for every
migrated user - every ldap.has_upg() call means one query to host LDAP.
Maybe we should ask just in the beginning and store the setting in
"ctx['upg_enabled']. I don't think that anyone needs to switch UPG
status during migration process.
I agree, nice optimization.
rob
>From 4d642f3f7c437a7081851981587a11ba631ff755 Mon Sep 17 00:00:00 2001
From: Rob Crittenden <rcrit...@redhat.com>
Date: Thu, 22 Mar 2012 13:40:54 -0400
Subject: [PATCH] Don't create private groups for migrated users, check for
valid gidnumber
Migrated users don't get a private group, there is no safe way to verify
that the namespace is correct without redoing the uidnumber as well.
Verify that the GID at least points to a valid group on the remote server
and warn if it doesn't (this doesn't guarantee that the group gets migrated
but at least we try).
If the remote entry has no gidNumber and the default user's group is
non-POSIX then don't migrate that user.
Add a loginshell if the remote user doesn't have one.
https://fedorahosted.org/freeipa/ticket/2562
---
ipalib/plugins/migration.py | 109 ++++++++++++++++++++++++++++++------------
1 files changed, 78 insertions(+), 31 deletions(-)
diff --git a/ipalib/plugins/migration.py b/ipalib/plugins/migration.py
index 4b104a8edc62f6876be1186fbc7bdda924472bf3..91759a12d6f4eb8c9f909e492db14be7017b2f53 100644
--- a/ipalib/plugins/migration.py
+++ b/ipalib/plugins/migration.py
@@ -24,6 +24,7 @@ from ipalib import api, errors, output
from ipalib import Command, Password, Str, Flag, StrEnum
from ipalib.cli import to_cli
from ipalib.dn import *
+from ipalib.plugins.user import NO_UPG_MAGIC
if api.env.in_server and api.env.context in ['lite', 'server']:
try:
from ipaserver.plugins.ldap2 import ldap2
@@ -126,21 +127,41 @@ def is_DN_syntax(ldap, attr):
def _pre_migrate_user(ldap, pkey, dn, entry_attrs, failed, config, ctx, **kwargs):
attr_blacklist = ['krbprincipalkey','memberofindirect','memberindirect']
attr_blacklist.extend(kwargs.get('attr_blacklist', []))
+ ds_ldap = ctx['ds_ldap']
+ has_upg = ctx['has_upg']
+ search_bases = kwargs.get('search_bases', None)
+ valid_gids = kwargs['valid_gids']
+
+ disable_upg = True
+
+ # migrated user is not already POSIX, make it so if we can
+ if 'gidnumber' not in entry_attrs:
+ if has_upg:
+ entry_attrs['uidnumber'] = entry_attrs['gidnumber'] = [999]
+ disable_upg = False
+ elif 'def_group_gid' in ctx:
+ entry_attrs['uidnumber'] = [999]
+ entry_attrs['gidnumber'] = ctx['def_group_gid']
+ else:
+ raise errors.NotFound(reason=_('User Private Groups are disabled and the default users group is not POSIX'))
+ else:
+ # See if the gidNumber at least points to a valid group on the remote
+ # server.
+ if entry_attrs['gidnumber'][0] not in valid_gids:
+ try:
+ (remote_dn, remote_entry) = ds_ldap.find_entry_by_attr(
+ 'gidnumber', entry_attrs['gidnumber'][0], 'posixgroup',
+ [''], search_bases['group']
+ )
+ valid_gids.append(entry_attrs['gidnumber'][0])
+ except errors.NotFound:
+ api.log.warn('Migrated user\'s GID number %s does not point to a known group.' % entry_attrs['gidnumber'][0])
- # get default primary group for new users
- if 'def_group_dn' not in ctx:
- def_group = config.get('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'])
- except errors.NotFound:
- error_msg = _('Default group for new users not found.')
- raise errors.NotFound(reason=error_msg)
- if not ldap.has_upg():
- if 'gidnumber' in g_attrs:
- ctx['def_group_gid'] = g_attrs['gidnumber'][0]
- else:
- raise errors.NotFound(reason=_('User Private Groups are disabled and the default users group is not POSIX'))
+ # We don't want to create a UPG so set the magic value in description
+ # to let the DS plugin know.
+ if disable_upg:
+ entry_attrs.setdefault('description', [])
+ entry_attrs['description'].append(NO_UPG_MAGIC)
# fill in required attributes by IPA
entry_attrs['ipauniqueid'] = 'autogenerate'
@@ -149,8 +170,10 @@ def _pre_migrate_user(ldap, pkey, dn, entry_attrs, failed, config, ctx, **kwargs
home_dir = '%s/%s' % (homes_root, pkey)
home_dir = home_dir.replace('//', '/').rstrip('/')
entry_attrs['homedirectory'] = home_dir
- if 'def_group_gid' in ctx:
- entry_attrs.setdefault('gidnumber', ctx['def_group_gid'])
+
+ if 'loginshell' not in entry_attrs:
+ default_shell = config.get('ipadefaultloginshell', ['/bin/sh'])[0]
+ entry_attrs.setdefault('loginshell', default_shell)
# do not migrate all attributes
for attr in entry_attrs.keys():
@@ -178,8 +201,6 @@ def _pre_migrate_user(ldap, pkey, dn, entry_attrs, failed, config, ctx, **kwargs
# Fix any attributes with DN syntax that point to entries in the old
# tree
- search_bases = kwargs.get('search_bases', None)
- ds_ldap = ctx['ds_ldap']
for attr in entry_attrs.keys():
if is_DN_syntax(ldap, attr):
@@ -187,7 +208,7 @@ def _pre_migrate_user(ldap, pkey, dn, entry_attrs, failed, config, ctx, **kwargs
try:
(remote_dn, remote_entry) = ds_ldap.get_entry(value, [api.Object.user.primary_key.name, api.Object.group.primary_key.name])
except errors.NotFound:
- api.log.error('In %s the attribute %s refers to non-existent entry %s' % (dn, attr, value))
+ api.log.warn('%s: attribute %s refers to non-existent entry %s' % (pkey, attr, value))
continue
if value.lower().endswith(search_bases['user']):
primary_key = api.Object.user.primary_key.name
@@ -196,14 +217,14 @@ def _pre_migrate_user(ldap, pkey, dn, entry_attrs, failed, config, ctx, **kwargs
primary_key = api.Object.group.primary_key.name
container = api.env.container_group
else:
- api.log.error('In entry %s value %s in attribute %s does not belong into any known container' % (dn, value, attr))
+ api.log.warn('%s: value %s in attribute %s does not belong into any known container' % (pkey, value, attr))
continue
if not remote_entry.get(primary_key):
- api.log.error('In %s there is no primary key %s to migrate for %s' % (value, primary_key, attr))
+ api.log.warn('%s: there is no primary key %s to migrate for %s' % (pkey, primary_key, attr))
continue
- api.log.info('converting DN value %s for %s in %s' % (value, attr, dn))
+ api.log.debug('converting DN value %s for %s in %s' % (value, attr, dn))
rdnval = remote_entry[primary_key][0].lower()
entry_attrs[attr][ind] = \
str(DN((primary_key, rdnval),
@@ -219,7 +240,13 @@ def _post_migrate_user(ldap, pkey, dn, entry_attrs, failed, config, ctx):
ldap.add_entry_to_group(dn, ctx['def_group_dn'])
except errors.ExecutionError, e:
failed[pkey] = unicode(_grp_err_msg)
-
+ if 'description' in entry_attrs and NO_UPG_MAGIC in entry_attrs['description']:
+ entry_attrs['description'].remove(NO_UPG_MAGIC)
+ kw = {'setattr': unicode('description=%s' % ','.join(entry_attrs['description']))}
+ try:
+ api.Command['user_mod'](pkey, **kw)
+ except (errors.EmptyModlist, errors.NotFound):
+ pass
# GROUP MIGRATION CALLBACKS AND VARS
@@ -626,6 +653,21 @@ can use their Kerberos accounts.''')
else:
blacklists[blacklist] = tuple()
+ # get default primary group for new users
+ if 'def_group_dn' not in context:
+ def_group = config.get('ipadefaultprimarygroup')
+ context['def_group_dn'] = api.Object.group.get_dn(def_group)
+ try:
+ (g_dn, g_attrs) = ldap.get_entry(context['def_group_dn'], ['gidnumber', 'cn'])
+ except errors.NotFound:
+ error_msg = _('Default group for new users not found.')
+ raise errors.NotFound(reason=error_msg)
+ if 'gidnumber' in g_attrs:
+ context['def_group_gid'] = g_attrs['gidnumber'][0]
+
+ context['has_upg'] = ldap.has_upg()
+
+ valid_gids = []
for (dn, entry_attrs) in entries:
if dn is None: # LDAP search reference
failed[ldap_obj_name][entry_attrs[0]] = unicode(_ref_err_msg)
@@ -661,13 +703,18 @@ can use their Kerberos accounts.''')
callback = self.migrate_objects[ldap_obj_name]['pre_callback']
if callable(callback):
- dn = callback(
- ldap, pkey, dn, entry_attrs, failed[ldap_obj_name],
- config, context, schema = options['schema'],
- search_bases = search_bases,
- **blacklists
- )
- if not dn:
+ try:
+ dn = callback(
+ ldap, pkey, dn, entry_attrs, failed[ldap_obj_name],
+ config, context, schema = options['schema'],
+ search_bases = search_bases,
+ valid_gids = valid_gids,
+ **blacklists
+ )
+ if not dn:
+ continue
+ except errors.NotFound, e:
+ failed[ldap_obj_name][pkey] = unicode(e.reason)
continue
try:
@@ -717,7 +764,7 @@ can use their Kerberos accounts.''')
(dn,check_compat) = ldap.get_entry(_compat_dn, normalize=False)
if check_compat is not None and \
check_compat.get('nsslapd-pluginenabled', [''])[0].lower() == 'on':
- return dict(result={},failed={},enabled=True, compat=False)
+ return dict(result={}, failed={}, enabled=True, compat=False)
except errors.NotFound:
pass
--
1.7.6
_______________________________________________
Freeipa-devel mailing list
Freeipa-devel@redhat.com
https://www.redhat.com/mailman/listinfo/freeipa-devel
