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

[Rob Crittenden]

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.
- 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.
- Under what conditions would the bind password not be found in the 
userPassword attribute?
- Why the formatting change to ipaEscrowKeyCertificate and ipaEscrowKey?
- 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
- 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?
- Can you add a validator for the LDAP uri and perhaps an example somewhere?
- When migrating users/groups do we want an option to maintain existing 
uid/gid?
- Is there a reason you're using pure ldap calls instead of the ldap2 
plugin?

rob