Ondrej Hamada wrote:
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


ACK, pushed to master. I fixed up a couple of white space issues (some intentions were 8, not 4 spaces) and expanded the commit message to ~72 chars/line.

rob

_______________________________________________
Freeipa-devel mailing list
Freeipa-devel@redhat.com
https://www.redhat.com/mailman/listinfo/freeipa-devel

Reply via email to