Commit 8f9e8689 authored by Jérome Perrin's avatar Jérome Perrin

PasswordTool: redirect with portal_status_level

to indicate success or failure

Also add a code comment about the changes from e50e45e4 (erp5_core:
Password Tool should not leak info on users, 2020-12-30), because while
looking at this code it seems there was a mistake here.
parent c6a2c3eb
...@@ -808,7 +808,7 @@ class TestAuthenticationPolicy(ERP5TypeTestCase): ...@@ -808,7 +808,7 @@ class TestAuthenticationPolicy(ERP5TypeTestCase):
preference = self.portal.portal_catalog.getResultValue( preference = self.portal.portal_catalog.getResultValue(
portal_type='System Preference', portal_type='System Preference',
title='Authentication',) 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 # as a way to check that password reset checks are done in the
# context of the login # context of the login
preference.setPrefferedForceUsernameCheckInPassword(1) preference.setPrefferedForceUsernameCheckInPassword(1)
...@@ -856,8 +856,11 @@ class TestAuthenticationPolicy(ERP5TypeTestCase): ...@@ -856,8 +856,11 @@ class TestAuthenticationPolicy(ERP5TypeTestCase):
# now with a password complying to the policy # now with a password complying to the policy
ret = submit_reset_password_dialog('ok') ret = submit_reset_password_dialog('ok')
self.assertEqual(httplib.FOUND, ret.getStatus()) self.assertEqual(httplib.FOUND, ret.getStatus())
self.assertTrue(ret.getHeader('Location').endswith( redirect_url = urlparse.urlparse(ret.getHeader("Location"))
'/login_form?portal_status_message=Password+changed.')) 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): def test_PreferenceTool_changePassword_checks_policy(self):
person = self.createUser(self.id(), password='current') person = self.createUser(self.id(), password='current')
...@@ -918,7 +921,7 @@ class TestAuthenticationPolicy(ERP5TypeTestCase): ...@@ -918,7 +921,7 @@ class TestAuthenticationPolicy(ERP5TypeTestCase):
# long enough password is accepted # long enough password is accepted
ret = submit_change_password_dialog('long_enough_password') 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(httplib.FOUND, ret.getStatus())
self.assertEqual(self.portal.portal_preferences.absolute_url(), self.assertEqual(self.portal.portal_preferences.absolute_url(),
ret.getHeader("Location")) ret.getHeader("Location"))
......
...@@ -117,6 +117,7 @@ class TestPasswordTool(ERP5TypeTestCase): ...@@ -117,6 +117,7 @@ class TestPasswordTool(ERP5TypeTestCase):
query_string_param = parse_qsl(urlparse(str(ret)).query) query_string_param = parse_qsl(urlparse(str(ret)).query)
self.assertIn(("portal_status_message", "An email has been sent to you."), query_string_param) self.assertIn(("portal_status_message", "An email has been sent to you."), query_string_param)
self.assertIn(("portal_status_level", "success"), query_string_param)
self.tic() self.tic()
(mfrom, mto, mbody), = self.portal.MailHost.getMessageList() (mfrom, mto, mbody), = self.portal.MailHost.getMessageList()
...@@ -134,6 +135,7 @@ class TestPasswordTool(ERP5TypeTestCase): ...@@ -134,6 +135,7 @@ class TestPasswordTool(ERP5TypeTestCase):
password_key=reset_key) password_key=reset_key)
query_string_param = parse_qsl(urlparse(str(ret)).query) query_string_param = parse_qsl(urlparse(str(ret)).query)
self.assertIn(("portal_status_message", "Password changed."), query_string_param) self.assertIn(("portal_status_message", "Password changed."), query_string_param)
self.assertIn(("portal_status_level", "success"), query_string_param)
self.tic() self.tic()
self._assertUserExists('userA-login', 'new-password') self._assertUserExists('userA-login', 'new-password')
self._assertUserDoesNotExists('userA-login', 'userA-password') self._assertUserDoesNotExists('userA-login', 'userA-password')
...@@ -146,6 +148,7 @@ class TestPasswordTool(ERP5TypeTestCase): ...@@ -146,6 +148,7 @@ class TestPasswordTool(ERP5TypeTestCase):
password_key=reset_key) password_key=reset_key)
query_string_param = parse_qsl(urlparse(str(ret)).query) query_string_param = parse_qsl(urlparse(str(ret)).query)
self.assertIn(("portal_status_message", "Key not known. Please ask reset password."), query_string_param) self.assertIn(("portal_status_message", "Key not known. Please ask reset password."), query_string_param)
self.assertIn(("portal_status_level", "error"), query_string_param)
self.tic() self.tic()
self._assertUserExists('userA-login', 'new-password') self._assertUserExists('userA-login', 'new-password')
self._assertUserDoesNotExists('userA-login', 'userA-password') self._assertUserDoesNotExists('userA-login', 'userA-password')
...@@ -155,6 +158,7 @@ class TestPasswordTool(ERP5TypeTestCase): ...@@ -155,6 +158,7 @@ class TestPasswordTool(ERP5TypeTestCase):
user_login='not exist', REQUEST=self.portal.REQUEST) user_login='not exist', REQUEST=self.portal.REQUEST)
query_string_param = parse_qsl(urlparse(str(ret)).query) query_string_param = parse_qsl(urlparse(str(ret)).query)
self.assertIn(("portal_status_message", "An email has been sent to you."), query_string_param) self.assertIn(("portal_status_message", "An email has been sent to you."), query_string_param)
self.assertIn(("portal_status_level", "success"), query_string_param)
self.tic() self.tic()
self.assertFalse(self.portal.MailHost.getMessageList()) self.assertFalse(self.portal.MailHost.getMessageList())
...@@ -163,6 +167,7 @@ class TestPasswordTool(ERP5TypeTestCase): ...@@ -163,6 +167,7 @@ class TestPasswordTool(ERP5TypeTestCase):
user_login='%', REQUEST=self.portal.REQUEST) user_login='%', REQUEST=self.portal.REQUEST)
query_string_param = parse_qsl(urlparse(str(ret)).query) query_string_param = parse_qsl(urlparse(str(ret)).query)
self.assertIn(("portal_status_message", "An email has been sent to you."), query_string_param) self.assertIn(("portal_status_message", "An email has been sent to you."), query_string_param)
self.assertIn(("portal_status_level", "success"), query_string_param)
self.tic() self.tic()
self.assertFalse(self.portal.MailHost.getMessageList()) self.assertFalse(self.portal.MailHost.getMessageList())
...@@ -180,6 +185,7 @@ class TestPasswordTool(ERP5TypeTestCase): ...@@ -180,6 +185,7 @@ class TestPasswordTool(ERP5TypeTestCase):
password_key=reset_key) password_key=reset_key)
query_string_param = parse_qsl(urlparse(str(ret)).query) query_string_param = parse_qsl(urlparse(str(ret)).query)
self.assertIn(("portal_status_message", "Bad login provided."), query_string_param) self.assertIn(("portal_status_message", "Bad login provided."), query_string_param)
self.assertIn(("portal_status_level", "error"), query_string_param)
self.tic() self.tic()
self._assertUserExists('userA-login', 'userA-password') self._assertUserExists('userA-login', 'userA-password')
self._assertUserExists('userB-login', 'userB-password') self._assertUserExists('userB-login', 'userB-password')
...@@ -195,6 +201,7 @@ class TestPasswordTool(ERP5TypeTestCase): ...@@ -195,6 +201,7 @@ class TestPasswordTool(ERP5TypeTestCase):
password_key='wrong key') password_key='wrong key')
query_string_param = parse_qsl(urlparse(str(ret)).query) query_string_param = parse_qsl(urlparse(str(ret)).query)
self.assertIn(("portal_status_message", "Key not known. Please ask reset password."), query_string_param) self.assertIn(("portal_status_message", "Key not known. Please ask reset password."), query_string_param)
self.assertIn(("portal_status_level", "error"), query_string_param)
self.tic() self.tic()
def test_password_reset_date_expired(self): def test_password_reset_date_expired(self):
...@@ -213,6 +220,7 @@ class TestPasswordTool(ERP5TypeTestCase): ...@@ -213,6 +220,7 @@ class TestPasswordTool(ERP5TypeTestCase):
password_key=reset_key) password_key=reset_key)
query_string_param = parse_qsl(urlparse(str(ret)).query) query_string_param = parse_qsl(urlparse(str(ret)).query)
self.assertIn(("portal_status_message", "Date has expired."), query_string_param) self.assertIn(("portal_status_message", "Date has expired."), query_string_param)
self.assertIn(("portal_status_level", "error"), query_string_param)
self.tic() self.tic()
self._assertUserExists('userA-login', 'userA-password') self._assertUserExists('userA-login', 'userA-password')
...@@ -325,6 +333,7 @@ class TestPasswordTool(ERP5TypeTestCase): ...@@ -325,6 +333,7 @@ class TestPasswordTool(ERP5TypeTestCase):
query_string_param = parse_qsl(urlparse(str(ret)).query) query_string_param = parse_qsl(urlparse(str(ret)).query)
# For security reasons, the message should always be the same # For security reasons, the message should always be the same
self.assertIn(("portal_status_message", "An email has been sent to you."), query_string_param) self.assertIn(("portal_status_message", "An email has been sent to you."), query_string_param)
self.assertIn(("portal_status_level", "success"), query_string_param)
# But no mail has been sent # But no mail has been sent
self.assertFalse(self.portal.MailHost.getMessageList()) self.assertFalse(self.portal.MailHost.getMessageList())
...@@ -353,6 +362,7 @@ class TestPasswordTool(ERP5TypeTestCase): ...@@ -353,6 +362,7 @@ class TestPasswordTool(ERP5TypeTestCase):
query_string_param = parse_qsl(urlparse(str(ret)).query) query_string_param = parse_qsl(urlparse(str(ret)).query)
# For security reasons, the message should always be the same # For security reasons, the message should always be the same
self.assertIn(("portal_status_message", "An email has been sent to you."), query_string_param) self.assertIn(("portal_status_message", "An email has been sent to you."), query_string_param)
self.assertIn(("portal_status_level", "success"), query_string_param)
# But no mail has been sent # But no mail has been sent
self.assertFalse(self.portal.MailHost.getMessageList()) self.assertFalse(self.portal.MailHost.getMessageList())
...@@ -382,6 +392,7 @@ class TestPasswordTool(ERP5TypeTestCase): ...@@ -382,6 +392,7 @@ class TestPasswordTool(ERP5TypeTestCase):
query_string_param = parse_qsl(urlparse(str(ret)).query) query_string_param = parse_qsl(urlparse(str(ret)).query)
# For security reasons, the message should always be the same # For security reasons, the message should always be the same
self.assertIn(("portal_status_message", "An email has been sent to you."), query_string_param) self.assertIn(("portal_status_message", "An email has been sent to you."), query_string_param)
self.assertIn(("portal_status_level", "success"), query_string_param)
# But no mail has been sent # But no mail has been sent
self.assertFalse(self.portal.MailHost.getMessageList()) self.assertFalse(self.portal.MailHost.getMessageList())
...@@ -41,9 +41,9 @@ from BTrees.OOBTree import OOBTree ...@@ -41,9 +41,9 @@ from BTrees.OOBTree import OOBTree
from six.moves.urllib.parse import urlencode from six.moves.urllib.parse import urlencode
redirect_path = '/login_form' 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: 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) ret_url = '%s%s?%s' % (site_url, redirect_path, parameter)
return REQUEST.RESPONSE.redirect( ret_url ) return REQUEST.RESPONSE.redirect( ret_url )
else: else:
...@@ -171,10 +171,13 @@ class PasswordTool(BaseTool): ...@@ -171,10 +171,13 @@ class PasswordTool(BaseTool):
"User {user} does not have a valid email address".format(user=user_login) "User {user} does not have a valid email address".format(user=user_login)
) )
if error_encountered: 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: if batch:
raise RuntimeError(msg) raise RuntimeError(msg)
else: else:
return redirect(REQUEST, site_url, msg) return redirect(REQUEST, site_url, msg, 'success')
key = self.getResetPasswordKey(user_login=user_login, key = self.getResetPasswordKey(user_login=user_login,
expiration_date=expiration_date) expiration_date=expiration_date)
...@@ -222,8 +225,7 @@ class PasswordTool(BaseTool): ...@@ -222,8 +225,7 @@ class PasswordTool(BaseTool):
message_text_format=message_text_format, message_text_format=message_text_format,
event_keyword_argument_dict=event_keyword_argument_dict) event_keyword_argument_dict=event_keyword_argument_dict)
if not batch: if not batch:
return redirect(REQUEST, site_url, return redirect(REQUEST, site_url, msg, 'success')
translateString("An email has been sent to you."))
security.declareProtected(Permissions.ModifyPortalContent, 'removeExpiredRequests') security.declareProtected(Permissions.ModifyPortalContent, 'removeExpiredRequests')
def removeExpiredRequests(self): def removeExpiredRequests(self):
...@@ -272,7 +274,7 @@ class PasswordTool(BaseTool): ...@@ -272,7 +274,7 @@ class PasswordTool(BaseTool):
# calling code and making mistakes more difficult # calling code and making mistakes more difficult
# BBB: should probably not translate message when REQUEST is None # BBB: should probably not translate message when REQUEST is None
message = translateString(message) message = translateString(message)
return redirect(REQUEST, site_url, message) return redirect(REQUEST, site_url, message, 'error')
if REQUEST is None: if REQUEST is None:
REQUEST = get_request() REQUEST = get_request()
...@@ -303,6 +305,6 @@ class PasswordTool(BaseTool): ...@@ -303,6 +305,6 @@ class PasswordTool(BaseTool):
login = portal.unrestrictedTraverse(login_dict['path']) login = portal.unrestrictedTraverse(login_dict['path'])
login.setPassword(password) # this will raise if password does not match policy login.setPassword(password) # this will raise if password does not match policy
return redirect(REQUEST, site_url, return redirect(REQUEST, site_url,
translateString("Password changed.")) translateString("Password changed."), 'success')
InitializeClass(PasswordTool) 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