[Freeipa-devel] [PATCH] 5 User-add random password support

[Ondrej Hamada]

On 12/09/2011 08:46 PM, Rob Crittenden wrote:
> Ondrej Hamada wrote:
>> On 11/29/2011 10:31 AM, Martin Kosek wrote:
>>> On Thu, 2011-11-24 at 17:51 +0100, Ondrej Hamada wrote:
>>>> On 11/24/2011 03:54 PM, Ondrej Hamada wrote:
>>>>> https://fedorahosted.org/freeipa/ticket/1979
>>>>>
>>>>> I've used code from ipalib/plugins/host.py to add support for
>>>>> random
>>>>> password generation. The '--random' option is now available in
>>>>> user-add and user-mod commands. If both the 'password' and 'random'
>>>>> options are used the 'random' option will be ignored.
>>>>>
>>>>>
>>> Functionally, it works OK. I would just like to propose few
>>> improvements:
>>>
>>> 1) Minor API version in VERSION file should be bumped since you add a
>>> new option
>>> 2) We should add some tests exercising this new functionality so 
>>> that we
>>> can detect regressions early
>>> 3) (optional) I am thinking if the passwords we generate are not very
>>> user friendly. I would love to see user's face when he is told that his
>>> new password is 5QU;8l2%]y"? .
>>>
>>> While this is may be OK for hosts bulk passwords which are only
>>> manipulated by admins, we may want to develop more user friendly
>>> passwords in the user plugin.
>>>
>>> Martin
>>>
>> https://fedorahosted.org/freeipa/ticket/1979
>>
>> I've used code from ipalib/plugins/host.py to add
>> support for random password generation. The
>> '--random' option is now available in user-add and
>> user-mod commands. If both the 'password' and 'random'
>> options are used the 'random' option will be ignored.
>>
>> Two test cases were added to unit test's module
>> test_user_plugin.py - they test creating and modifying
>> user with random password. Two fuzzy tests were added:
>> test for password(string that doesn't start or end with
>> whitespace and doesn't containt other whitespace than
>> ' ') and for whatever string(because of krbextradata).
>>
>> I've slightly modified ipa_generate_password in order
>> to make passwords for users more user-friendly(reduce
>> number of non-letters). It has two optional parameters
>> now - first one is string of characters that should be
>> used for generating the passwd and second one is length
>> of password. If none parameter is set default values will
>> be used so there's no need to modify other plugins that
>> use random password generator.
>
> This is very, very close. I just have a couple of observations:
>
> 1. Would it be clearer if in user.py you set the character set to: 
> string.digits + string.ascii_letters + '_,.@' ? I gather you are 
> excluding most of the characters that would cause the shell grief on 
> purpose, right? You can probably add in +, - and = though.
>
> 2. Indention in ipa_generate_password() is a bit off.
>
> 3. Rather than fuzzy_string() I think you should define something like 
> fuzzy_dergeneralizedtime() that specifies the time format in those 
> kerberos time attributes. krbextradata is probably fine as a string.
>
> rob
Corrected and modified according to rob's proposals

Ondra

-- 
Regards,

Ondrej Hamada
FreeIPA team
jabber: ohama at jabbim.cz
IRC: ohamada

-------------- next part --------------
A non-text attachment was scrubbed...
Name: freeipa-ohamada-5-4-User-add-random-password-support.patch
Type: text/x-patch
Size: 17998 bytes
Desc: not available
URL: <http://listman.redhat.com/archives/freeipa-devel/attachments/20111212/da260206/attachment.bin>