[Freeipa-devel] Re: [PATCHES] Migration wrap-up.

[Pavel Zuna]

Rob Crittenden wrote:
> Pavel Zůna wrote:
>> Okey, I think my migration patches are ready for submission.
>>
>> What's new?
>>
>> - No more forced password change after migration, unless the password 
>> doesn't meet IPA password policy. Expiration time sets correctly 
>> (hooray!).
>> - Migration mode (adding entries with pre-hashed passwords) can now be 
>> turned ON/OFF using the ipaMigrationEnabled attribute in ipaConfig entry.
>> - New fancy password migration page using HTML form based 
>> authentication. (CSS and looks in general will probably have to change 
>> to visually go with the rest of the webUI.)
>> - Better error/log messages and some general code clean up.
>>
>> I didn't change the migration plugin to use IPA commands. Believe me, 
>> I tried. There's just too much overhead and additional work:
>>
>> - We need to sanitize data from DS before we feed it to the IPA 
>> commands and it's not just converting them to unicode.
>> - There are attributes our commands do not accept as parameters and 
>> setattr/addattr doesn't really help that much there. It's going to be 
>> even worst when custom schemas kick in. Our commands also make some 
>> assumptions about attributes - like givenName/sn being required etc. 
>> It's just too hard to do it properly in a generic way.
>> - Using IPA commands generates at least 4 times more LDAP requests.
>> - The code is also longer.
>>
>> The migration plugin might still need some work and I'm thinking of 
>> ways to make it better, more readable and maintainable, but if the 
>> other patches pass and there's no big problems with it, I say we 
>> should push it, so that QE can do some testing.
>>
>> I'm currently writing a wiki page with step by step migration guide, 
>> but I left it open at the office and I'm sick at home at the moment, 
>> so I'm going to resume when back. I will also setup a testing 
>> environment on the blades for DS to IPA migration.
>>
>> Pavel
> 
> A few comments:
> 
> - The comment block in ipapwd_pre_bind() is incorrect. It says that it 
> will generate a principal name.
Ok, I forgot to update it.

> - You check for the existence of userPassword in the entry. Since you've 
> already made sure a simple bind was successful I don't think this is 
> necessary, it is implicit, right? I suppose it doesn't hurt anything.
No, I think that the pre-bind plugin is called anyway.

This shows up in the DS log, when I try to bind with an invalid password:

ipapwd_pre_bind - invalid BIND password for user entry: 
uid=someuser,cn=users,cn=accounts,dc=example,dc=com

> - Under what conditions would the bind password not be found in the 
> userPassword attribute?
If unauthenticated BINDs were allowed. I know they're not, but we better make 
sure. :)

> - Why the formatting change to ipaEscrowKeyCertificate and ipaEscrowKey?
Don't know, guess I'm a bit OCD about code formatting. :)

> - There are a number of typos on the migration HTML pages:
>    - There was a problem with you request.
>    - If the problem persists, contact you administrator.
>    - migrated to a new Indentity management solution
>    - Upon successfull login
Ok, I need to get spell checking in vim! Anyway I think someone better at 
english/user friendliness should review those messages anyway, but I'll fix the 
typos for the next patch.

> - There are a number of ways to get redirected to 
> /ipa/migration/error.html and invalid.html. Should some logging be added 
> so an admin can debug why failures occur?
Ok, I'm going to make the migration plugin generate a stand alone log file 
anyway, because when migrating a lot of users and groups, something is bound to 
fail :) and it's not good enough to have errors printed out to stdout/stderr only.

> - Can you add a validator for the LDAP uri and perhaps an example 
> somewhere?
Sure.

> - When migrating users/groups do we want an option to maintain existing 
> uid/gid?
I don't know, I do keep them for now - some services might depend on them and it 
wouldn't be nice if they changed after migration. So, I think I'd rather add an 
option to generate new UID/GIDs. There might also be collision with already 
existing users/groups, so we need to add some checks.

> - Is there a reason you're using pure ldap calls instead of the ldap2 
> plugin?
Yes, I wanted to use ldap2, I really did. But unfortunately ldap2 is closely 
tied to the rest of the API (it's a plugin after all) and it is currently 
impossible to even spawn a new ldap2 instance and make it connect to an 
arbitrary LDAP server, because it makes assumptions about the base DN and such. 
I really need to make it a bit more flexible, because it would be a shame not to 
use it in these situations. I'll make it a high priority task for myself.

> rob

Pavel