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

[David O'Brien]

Pavel Zuna wrote:
> 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.

If there's an easy way for me to get access to the messages and anything 
else that gets put in front of the user I'm happy to review it. I 
haven't done much (read "almost nothing") in the way of patch reviews, 
so I'm not sure if that's the best way. Maybe I should just learn to 
read patches...

I'm also looking forward to seeing the wiki page with the migration 
steps; something else for IPA doc.

David

> 
>> - 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
> 
> _______________________________________________
> Freeipa-devel mailing list
> Freeipa-devel at redhat.com
> https://www.redhat.com/mailman/listinfo/freeipa-devel


-- 

David O'Brien
Red Hat Asia Pacific
+61 7 3514 8189

http://freeipa.org/page/DocumentationPortal
http://git.fedorahosted.org/git/ipadocs.git

He who asks is a fool for five minutes, but he who does not ask remains 
a fool forever."
  ~ Chinese proverb