Martin Kosek wrote:
On Fri, 2012-03-30 at 09:05 -0400, Rob Crittenden wrote:
Martin Kosek wrote:
On Thu, 2012-03-29 at 11:27 -0400, Rob Crittenden wrote:
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
This patch is OK, everything worked as expected. We just now need to
specify if we want a special option/flag to enable non-posix -> posix
user conversion. If not, then ACK.
Martin
Simo and I had a brief discussion about this in IRC and I'm going to
punt on non-POSIX user conversion for now. There could be specific
reasons for this, like having a shared user (nss_ldap) or some sort of
system user that they don't to be a full POSIX user.
Adding a new option this late in 2.2 doesn't seem like a good idea so
I'll patch it to simply fail non-POSIX users for now. I've opened a
ticket to optionally allow them to be converted.
rob
Ok. Your patch 993 is still useful and fixes the creation of UPGs, we
just need to remove nonposix->posix conversion and make sure non-posix
users are migrated successfully as non-posix users.
Martin
I decided to not migrate non-POSIX users at all. They are a-typical in
IPA and won't show at all using our tools so I don't see the point in
migrating them. non-POSIX users in IPA are typically stored in
cn=sysconfig rather than cn=users.
rob
>From 0c0cd796bba56c676254613d822fb50f28c80ba2 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 then don't migrate that user. We
don't know why that user is non-POSIX, it could be a special user used
for auth, for example.
Add a loginshell if the remote user doesn't have one.
https://fedorahosted.org/freeipa/ticket/2562
---
ipalib/plugins/migration.py | 98 +++++++++++++++++++++++++++++-------------
1 files changed, 67 insertions(+), 31 deletions(-)
diff --git a/ipalib/plugins/migration.py b/ipalib/plugins/migration.py
index 4b104a8edc62f6876be1186fbc7bdda924472bf3..ee83ca579a459a79933fed7621b12fb1d3e16903 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,30 @@ 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']
- # 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'))
+ if 'gidnumber' not in entry_attrs:
+ raise errors.NotFound(reason=_('%(user)s is not a POSIX user.') % dict(user=pkey))
+ 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])
+
+ # We don't want to create a UPG so set the magic value in description
+ # to let the DS plugin know.
+ entry_attrs.setdefault('description', [])
+ entry_attrs['description'].append(NO_UPG_MAGIC)
# fill in required attributes by IPA
entry_attrs['ipauniqueid'] = 'autogenerate'
@@ -149,8 +159,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 +190,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 +197,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 +206,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 +229,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 +642,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 +692,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 +753,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
