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: oh...@jabbim.cz
IRC: ohamada
From 05da095018897760f285c5f07bbf07c18e5b81e4 Mon Sep 17 00:00:00 2001
From: Ondrej Hamada <oham...@redhat.com>
Date: Mon, 12 Dec 2011 12:59:06 +0100
Subject: [PATCH] User-add random password support
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.
https://fedorahosted.org/freeipa/ticket/1979
---
API.txt | 6 +-
VERSION | 2 +-
ipalib/plugins/user.py | 36 +++++++++
ipapython/ipautil.py | 32 ++++++---
tests/test_xmlrpc/test_user_plugin.py | 128 ++++++++++++++++++++++++++++++++-
tests/test_xmlrpc/xmlrpc_test.py | 10 +++
6 files changed, 201 insertions(+), 13 deletions(-)
diff --git a/API.txt b/API.txt
index c2f4863fc5ee94178aacabd8a207c8fa32d1ca0d..aba3d8aa0250113d137878c97903922ff14ee664 100644
--- a/API.txt
+++ b/API.txt
@@ -2894,7 +2894,7 @@ output: Output('summary', (<type 'unicode'>, <type 'NoneType'>), None)
output: Entry('result', <type 'dict'>, Gettext('A dictionary representing an LDAP entry', domain='ipa', localedir=None))
output: Output('value', <type 'unicode'>, None)
command: user_add
-args: 1,31,3
+args: 1,32,3
arg: Str('uid', attribute=True, cli_name='login', maxlength=255, multivalue=False, pattern='^[a-zA-Z0-9_.][a-zA-Z0-9_.-]{0,252}[a-zA-Z0-9_.$-]?$', pattern_errmsg='may only include letters, numbers, _, -, . and $', primary_key=True, required=True)
option: Str('givenname', attribute=True, cli_name='first', multivalue=False, required=True)
option: Str('sn', attribute=True, cli_name='last', multivalue=False, required=True)
@@ -2907,6 +2907,7 @@ option: Str('loginshell', attribute=True, cli_name='shell', default=u'/bin/sh',
option: Str('krbprincipalname', attribute=True, autofill=True, cli_name='principal', multivalue=False, required=False)
option: Str('mail', attribute=True, cli_name='email', multivalue=True, required=False)
option: Password('userpassword', attribute=True, cli_name='password', exclude='webui', multivalue=False, required=False)
+option: Flag('random', attribute=False, autofill=True, cli_name='random', default=False, multivalue=False, required=False)
option: Int('uidnumber', attribute=True, autofill=True, cli_name='uid', default=999, minvalue=1, multivalue=False, required=True)
option: Int('gidnumber', attribute=True, autofill=True, cli_name='gidnumber', multivalue=False, required=True)
option: Str('street', attribute=True, cli_name='street', multivalue=False, required=False)
@@ -3000,7 +3001,7 @@ output: ListOfEntries('result', (<type 'list'>, <type 'tuple'>), Gettext('A list
output: Output('count', <type 'int'>, None)
output: Output('truncated', <type 'bool'>, None)
command: user_mod
-args: 1,32,3
+args: 1,33,3
arg: Str('uid', attribute=True, cli_name='login', maxlength=255, multivalue=False, pattern='^[a-zA-Z0-9_.][a-zA-Z0-9_.-]{0,252}[a-zA-Z0-9_.$-]?$', pattern_errmsg='may only include letters, numbers, _, -, . and $', primary_key=True, query=True, required=True)
option: Str('givenname', attribute=True, autofill=False, cli_name='first', multivalue=False, required=False)
option: Str('sn', attribute=True, autofill=False, cli_name='last', multivalue=False, required=False)
@@ -3012,6 +3013,7 @@ option: Str('gecos', attribute=True, autofill=False, cli_name='gecos', multivalu
option: Str('loginshell', attribute=True, autofill=False, cli_name='shell', default=u'/bin/sh', multivalue=False, required=False)
option: Str('mail', attribute=True, autofill=False, cli_name='email', multivalue=True, required=False)
option: Password('userpassword', attribute=True, autofill=False, cli_name='password', exclude='webui', multivalue=False, required=False)
+option: Flag('random', attribute=False, autofill=True, cli_name='random', default=False, multivalue=False, required=False)
option: Int('uidnumber', attribute=True, autofill=False, cli_name='uid', default=999, minvalue=1, multivalue=False, required=False)
option: Int('gidnumber', attribute=True, autofill=False, cli_name='gidnumber', multivalue=False, required=False)
option: Str('street', attribute=True, autofill=False, cli_name='street', multivalue=False, required=False)
diff --git a/VERSION b/VERSION
index 4fbf7a9a0a258d42d9afeea73c2b2d007ed923c7..081643745c47e78e7739f2b1092be762acd14e5f 100644
--- a/VERSION
+++ b/VERSION
@@ -79,4 +79,4 @@ IPA_DATA_VERSION=20100614120000
# #
########################################################
IPA_API_VERSION_MAJOR=2
-IPA_API_VERSION_MINOR=17
+IPA_API_VERSION_MINOR=18
diff --git a/ipalib/plugins/user.py b/ipalib/plugins/user.py
index a3c17dc4c0b23c541f6b8f5893ea9cfe4cab5ac0..70a111dd34c66ff22f906834a1348c65dbfd1bd5 100644
--- a/ipalib/plugins/user.py
+++ b/ipalib/plugins/user.py
@@ -25,6 +25,8 @@ from ipalib.request import context
from time import gmtime, strftime
import copy
from ipalib import _, ngettext
+from ipapython.ipautil import ipa_generate_password
+import string
__doc__ = _("""
Users
@@ -74,6 +76,9 @@ user_output_params = (
),
)
+# characters to be used for generating random user passwords
+user_pwdchars = string.digits + string.ascii_letters + '_,.@+-='
+
def validate_nsaccountlock(entry_attrs):
if 'nsaccountlock' in entry_attrs:
nsaccountlock = entry_attrs['nsaccountlock']
@@ -238,6 +243,15 @@ class user(LDAPObject):
# bomb out via the webUI.
exclude='webui',
),
+ Flag('random?',
+ doc=_('Generate a random user password'),
+ flags=('no_search', 'virtual_attribute'),
+ default=False,
+ ),
+ Str('randompassword?',
+ label=_('Random password'),
+ flags=('no_create', 'no_update', 'no_search', 'virtual_attribute'),
+ ),
Int('uidnumber',
cli_name='uid',
label=_('UID'),
@@ -430,6 +444,11 @@ class user_add(LDAPCreate):
raise errors.NotFound(reason=error_msg)
entry_attrs['gidnumber'] = group_attrs['gidnumber']
+ if 'userpassword' not in entry_attrs and options.get('random'):
+ entry_attrs['userpassword'] = ipa_generate_password(user_pwdchars)
+ # save the password so it can be displayed in post_callback
+ setattr(context, 'randompassword', entry_attrs['userpassword'])
+
if 'mail' in entry_attrs:
entry_attrs['mail'] = self.obj._normalize_email(entry_attrs['mail'], config)
@@ -465,6 +484,13 @@ class user_add(LDAPCreate):
newentry = wait_for_value(ldap, dn, 'objectclass', 'mepOriginEntry')
entry_from_entry(entry_attrs, newentry)
+ if options.get('random', False):
+ try:
+ entry_attrs['randompassword'] = unicode(getattr(context, 'randompassword'))
+ except AttributeError:
+ # if both randompassword and userpassword options were used
+ pass
+
self.obj.get_password_attributes(ldap, dn, entry_attrs)
return dn
@@ -495,9 +521,19 @@ class user_mod(LDAPUpdate):
if 'manager' in entry_attrs:
entry_attrs['manager'] = self.obj._normalize_manager(entry_attrs['manager'])
validate_nsaccountlock(entry_attrs)
+ if 'userpassword' not in entry_attrs and options.get('random'):
+ entry_attrs['userpassword'] = ipa_generate_password(user_pwdchars)
+ # save the password so it can be displayed in post_callback
+ setattr(context, 'randompassword', entry_attrs['userpassword'])
return dn
def post_callback(self, ldap, dn, entry_attrs, *keys, **options):
+ if options.get('random', False):
+ try:
+ entry_attrs['randompassword'] = unicode(getattr(context, 'randompassword'))
+ except AttributeError:
+ # if both randompassword and userpassword options were used
+ pass
convert_nsaccountlock(entry_attrs)
self.obj._convert_manager(entry_attrs, **options)
self.obj.get_password_attributes(ldap, dn, entry_attrs)
diff --git a/ipapython/ipautil.py b/ipapython/ipautil.py
index c06e7bbcfa77a7762ed00dc933e8a55a2099bad7..33e4442a21361a1532f27f39492d41e2e61fb238 100644
--- a/ipapython/ipautil.py
+++ b/ipapython/ipautil.py
@@ -550,21 +550,35 @@ def parse_generalized_time(timestr):
except ValueError:
return None
-def ipa_generate_password():
+def ipa_generate_password(characters=None,pwd_len=None):
+ ''' Generates password. Password cannot start or end with a whitespace
+ character. It also cannot be formed by whitespace characters only.
+ Length of password as well as string of characters to be used by
+ generator could be optionaly specified by characters and pwd_len
+ parameters, otherwise default values will be used: characters string
+ will be formed by all printable non-whitespace characters and space,
+ pwd_len will be equal to value of GEN_PWD_LEN.
+ '''
+ if not characters:
+ characters=string.digits + string.ascii_letters + string.punctuation + ' '
+ else:
+ if characters.isspace():
+ raise ValueError("password cannot be formed by whitespaces only")
+ if not pwd_len:
+ pwd_len = GEN_PWD_LEN
+
+ upper_bound = len(characters) - 1
rndpwd = ''
r = random.SystemRandom()
- for x in range(GEN_PWD_LEN):
- # do not generate space (chr(32)) as the first or last character
- if x == 0 or x == (GEN_PWD_LEN-1):
- rndchar = chr(r.randint(33,126))
- else:
- rndchar = chr(r.randint(32,126))
+ for x in range(pwd_len):
+ rndchar = characters[r.randint(0,upper_bound)]
+ if (x == 0) or (x == pwd_len-1):
+ while rndchar.isspace():
+ rndchar = characters[r.randint(0,upper_bound)]
rndpwd += rndchar
-
return rndpwd
-
def format_list(items, quote=None, page_width=80):
'''Format a list of items formatting them so they wrap to fit the
available width. The items will be sorted.
diff --git a/tests/test_xmlrpc/test_user_plugin.py b/tests/test_xmlrpc/test_user_plugin.py
index 8cd2739609ef93b7a00d4c7a8431729c71df3053..0370ec74d294f15b742aadacbdbf6fa296ebb718 100644
--- a/tests/test_xmlrpc/test_user_plugin.py
+++ b/tests/test_xmlrpc/test_user_plugin.py
@@ -25,7 +25,7 @@ Test the `ipalib/plugins/user.py` module.
from ipalib import api, errors
from tests.test_xmlrpc import objectclasses
-from xmlrpc_test import Declarative, fuzzy_digits, fuzzy_uuid
+from xmlrpc_test import Declarative, fuzzy_digits, fuzzy_uuid, fuzzy_password, fuzzy_string, fuzzy_dergeneralizedtime
from ipalib.dn import *
user1=u'tuser1'
@@ -722,6 +722,132 @@ class test_user(Declarative):
),
),
+ dict(
+ desc='Create %r with random password' % user1,
+ command=(
+ 'user_add', [user1], dict(givenname=u'Test', sn=u'User1', random=True)
+ ),
+ expected=dict(
+ value=user1,
+ summary=u'Added user "tuser1"',
+ result=dict(
+ gecos=[u'Test User1'],
+ givenname=[u'Test'],
+ homedirectory=[u'/home/tuser1'],
+ krbprincipalname=[u'tuser1@' + api.env.realm],
+ loginshell=[u'/bin/sh'],
+ objectclass=objectclasses.user,
+ sn=[u'User1'],
+ uid=[user1],
+ uidnumber=[fuzzy_digits],
+ gidnumber=[fuzzy_digits],
+ displayname=[u'Test User1'],
+ cn=[u'Test User1'],
+ initials=[u'TU'],
+ ipauniqueid=[fuzzy_uuid],
+ krbpwdpolicyreference=lambda x: [DN(i) for i in x] == \
+ [DN(('cn','global_policy'),('cn',api.env.realm),
+ ('cn','kerberos'),api.env.basedn)],
+ mepmanagedentry=lambda x: [DN(i) for i in x] == \
+ [DN(('cn',user1),('cn','groups'),('cn','accounts'),
+ api.env.basedn)],
+ memberof_group=[u'ipausers'],
+ has_keytab=True,
+ has_password=True,
+ randompassword=fuzzy_password,
+ krbextradata=[fuzzy_string],
+ krbpasswordexpiration=[fuzzy_dergeneralizedtime],
+ krblastpwdchange=[fuzzy_dergeneralizedtime],
+ dn=lambda x: DN(x) == \
+ DN(('uid','tuser1'),('cn','users'),('cn','accounts'),
+ api.env.basedn),
+ ),
+ ),
+ ),
+
+ dict(
+ desc='Delete %r' % user1,
+ command=('user_del', [user1], {}),
+ expected=dict(
+ result=dict(failed=u''),
+ summary=u'Deleted user "tuser1"',
+ value=user1,
+ ),
+ ),
+
+ dict(
+ desc='Create %r' % user2,
+ command=(
+ 'user_add', [user2], dict(givenname=u'Test', sn=u'User2')
+ ),
+ expected=dict(
+ value=user2,
+ summary=u'Added user "tuser2"',
+ result=dict(
+ gecos=[u'Test User2'],
+ givenname=[u'Test'],
+ homedirectory=[u'/home/tuser2'],
+ krbprincipalname=[u'tuser2@' + api.env.realm],
+ loginshell=[u'/bin/sh'],
+ objectclass=objectclasses.user,
+ sn=[u'User2'],
+ uid=[user2],
+ uidnumber=[fuzzy_digits],
+ gidnumber=[fuzzy_digits],
+ displayname=[u'Test User2'],
+ cn=[u'Test User2'],
+ initials=[u'TU'],
+ ipauniqueid=[fuzzy_uuid],
+ krbpwdpolicyreference=lambda x: [DN(i) for i in x] == \
+ [DN(('cn','global_policy'),('cn',api.env.realm),
+ ('cn','kerberos'),api.env.basedn)],
+ mepmanagedentry=lambda x: [DN(i) for i in x] == \
+ [DN(('cn',user2),('cn','groups'),('cn','accounts'),
+ api.env.basedn)],
+ memberof_group=[u'ipausers'],
+ has_keytab=False,
+ has_password=False,
+ dn=lambda x: DN(x) == \
+ DN(('uid','tuser2'),('cn','users'),('cn','accounts'),
+ api.env.basedn),
+ ),
+ ),
+ ),
+
+ dict(
+ desc='Modify %r with random password' % user2,
+ command=(
+ 'user_mod', [user2], dict(random=True)
+ ),
+ expected=dict(
+ result=dict(
+ givenname=[u'Test'],
+ homedirectory=[u'/home/tuser2'],
+ loginshell=[u'/bin/sh'],
+ sn=[u'User2'],
+ uid=[user2],
+ uidnumber=[fuzzy_digits],
+ gidnumber=[fuzzy_digits],
+ memberof_group=[u'ipausers'],
+ nsaccountlock=False,
+ has_keytab=True,
+ has_password=True,
+ randompassword=fuzzy_password,
+ ),
+ summary=u'Modified user "tuser2"',
+ value=user2,
+ ),
+ ),
+
+ dict(
+ desc='Delete %r' % user2,
+ command=('user_del', [user2], {}),
+ expected=dict(
+ result=dict(failed=u''),
+ summary=u'Deleted user "tuser2"',
+ value=user2,
+ ),
+ ),
dict(
desc='Create user %r with upper-case principal' % user1,
diff --git a/tests/test_xmlrpc/xmlrpc_test.py b/tests/test_xmlrpc/xmlrpc_test.py
index 4f29fb7ceb4fcb6b8602e4884f634f63babdcfa6..3535040e89aeafa5981255e14ed618d6f9a2b840 100644
--- a/tests/test_xmlrpc/xmlrpc_test.py
+++ b/tests/test_xmlrpc/xmlrpc_test.py
@@ -53,6 +53,16 @@ fuzzy_date = Fuzzy('^[a-zA-Z]{3} [a-zA-Z]{3} \d{2} \d{2}:\d{2}:\d{2} \d{4} UTC$'
fuzzy_issuer = Fuzzy(type=basestring, test=lambda issuer: valid_issuer(issuer, api.env.realm))
+# Matches password - password consists of all printable characters without whitespaces
+# The only exception is space, but space cannot be at the beggingin or end of the pwd
+fuzzy_password = Fuzzy('^\S([\S ]*\S)*$')
+
+# Matches generalized time value. Time format is: %Y%m%d%H%M%SZ
+fuzzy_dergeneralizedtime = Fuzzy('^[0-9]{14}Z$')
+
+# match any string
+fuzzy_string = Fuzzy(type=basestring)
+
try:
if not api.Backend.xmlclient.isconnected():
api.Backend.xmlclient.connect(fallback=False)
--
1.7.6.4
_______________________________________________
Freeipa-devel mailing list
Freeipa-devel@redhat.com
https://www.redhat.com/mailman/listinfo/freeipa-devel
