Commit dc9ffa12 authored by Jérome Perrin's avatar Jérome Perrin

Modernize Password Tool

- Use portal_status_level when redirecting
- Check that password and confirmation match
- Fix  wrong argument name `password_confirmation` sometimes used

See merge request !1792
parents 8cc780c1 05a3390b
......@@ -808,7 +808,7 @@ class TestAuthenticationPolicy(ERP5TypeTestCase):
preference = self.portal.portal_catalog.getResultValue(
portal_type='System Preference',
title='Authentication',)
# Here we activate the "password should contain usename" policy
# Here we activate the "password should contain username" policy
# as a way to check that password reset checks are done in the
# context of the login
preference.setPrefferedForceUsernameCheckInPassword(1)
......@@ -856,8 +856,11 @@ class TestAuthenticationPolicy(ERP5TypeTestCase):
# now with a password complying to the policy
ret = submit_reset_password_dialog('ok')
self.assertEqual(httplib.FOUND, ret.getStatus())
self.assertTrue(ret.getHeader('Location').endswith(
'/login_form?portal_status_message=Password+changed.'))
redirect_url = urlparse.urlparse(ret.getHeader("Location"))
self.assertEqual(redirect_url.path, '{}/login_form'.format(self.portal.absolute_url_path()))
redirect_url_params = urlparse.parse_qsl(redirect_url.query)
self.assertIn(('portal_status_message', 'Password changed.'), redirect_url_params)
self.assertIn(('portal_status_level', 'success'), redirect_url_params)
def test_PreferenceTool_changePassword_checks_policy(self):
person = self.createUser(self.id(), password='current')
......@@ -918,7 +921,7 @@ class TestAuthenticationPolicy(ERP5TypeTestCase):
# long enough password is accepted
ret = submit_change_password_dialog('long_enough_password')
# When password reset is succesful, user is logged out
# When password reset is successful, user is logged out
self.assertEqual(httplib.FOUND, ret.getStatus())
self.assertEqual(self.portal.portal_preferences.absolute_url(),
ret.getHeader("Location"))
......
......@@ -3,7 +3,7 @@
"""
REQUEST = context.REQUEST
next_url = context.portal_password.changeUserPassword(password=REQUEST['password'],
password_confirmation=REQUEST['password_confirm'],
password_confirm=REQUEST['password_confirm'],
password_key=REQUEST['password_key'],
user_login=REQUEST.get('user_login', None),
REQUEST=REQUEST)
......
REQUEST = context.REQUEST
return context.portal_password.changeUserPassword(password=REQUEST['password'],
password_confirmation=REQUEST['password_confirm'],
password_confirm=REQUEST['password_confirm'],
password_key=REQUEST['password_key'],
user_login=REQUEST.get('user_login', None),
REQUEST=REQUEST)
......@@ -41,9 +41,9 @@ from BTrees.OOBTree import OOBTree
from six.moves.urllib.parse import urlencode
redirect_path = '/login_form'
def redirect(REQUEST, site_url, message):
def redirect(REQUEST, site_url, message, level):
if REQUEST is not None and getattr(REQUEST.RESPONSE, 'redirect', None) is not None:
parameter = urlencode({'portal_status_message': message})
parameter = urlencode({'portal_status_message': message, 'portal_status_level': level})
ret_url = '%s%s?%s' % (site_url, redirect_path, parameter)
return REQUEST.RESPONSE.redirect( ret_url )
else:
......@@ -171,10 +171,13 @@ class PasswordTool(BaseTool):
"User {user} does not have a valid email address".format(user=user_login)
)
if error_encountered:
# note that we intentionally use the same msg here regardless of whether the
# email was successfully sent or not in order not to leak information about user
# existence.
if batch:
raise RuntimeError(msg)
else:
return redirect(REQUEST, site_url, msg)
return redirect(REQUEST, site_url, msg, 'success')
key = self.getResetPasswordKey(user_login=user_login,
expiration_date=expiration_date)
......@@ -222,8 +225,7 @@ class PasswordTool(BaseTool):
message_text_format=message_text_format,
event_keyword_argument_dict=event_keyword_argument_dict)
if not batch:
return redirect(REQUEST, site_url,
translateString("An email has been sent to you."))
return redirect(REQUEST, site_url, msg, 'success')
security.declareProtected(Permissions.ModifyPortalContent, 'removeExpiredRequests')
def removeExpiredRequests(self):
......@@ -266,13 +268,12 @@ class PasswordTool(BaseTool):
"""
Reset the password for a given login
"""
# BBB: password_confirm: unused argument
def error(message):
# BBB: should "raise Redirect" instead of just returning, simplifying
# calling code and making mistakes more difficult
# BBB: should probably not translate message when REQUEST is None
message = translateString(message)
return redirect(REQUEST, site_url, message)
return redirect(REQUEST, site_url, message, 'error')
if REQUEST is None:
REQUEST = get_request()
......@@ -291,6 +292,8 @@ class PasswordTool(BaseTool):
if user_login is not None and register_user_login != user_login:
# XXX: not descriptive enough
return error("Bad login provided.")
if password_confirm is not None and password_confirm != password:
return error("Password does not match the confirm password.")
if DateTime() > expiration_date:
return error("Date has expired.")
del self._password_request_dict[password_key]
......@@ -303,6 +306,6 @@ class PasswordTool(BaseTool):
login = portal.unrestrictedTraverse(login_dict['path'])
login.setPassword(password) # this will raise if password does not match policy
return redirect(REQUEST, site_url,
translateString("Password changed."))
translateString("Password changed."), 'success')
InitializeClass(PasswordTool)
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment