Commit 6da3c680 authored by Jérome Perrin's avatar Jérome Perrin

base: check password comply with authentication policy on change own password action

parent 288bffbe
"""External validator for PreferenceTool_viewChangePasswordDialog/your_password.
Check that password matchs with confirmation and that it complies to the authentication policy.
"""
from AccessControl import getSecurityManager
from Products.Formulator.Errors import ValidationError
password_confirm = request.get('field_password_confirm',
request.get('password_confirm'))
# password does not match confirmation, returns the default external validator message.
if password_confirm != editor:
return 0
login = getSecurityManager().getUser().getLoginValue()
if login is not None:
validation_message_list = login.analyzePassword(editor)
  • @jerome I think here we should check if portal.portal_preferences.isAuthenticationPolicyEnabled() before analysing, am I correct?

  • mmm, but maybe this preference does not exist. So maybe this should be in analyzePassword methods?

Please register or sign in to reply
if validation_message_list:
message = u' '.join([str(x) for x in validation_message_list])
raise ValidationError('external_validator_failed', context, error_text=message)
return 1
<?xml version="1.0"?>
<ZopeData>
<record id="1" aka="AAAAAAAAAAE=">
<pickle>
<global name="PythonScript" module="Products.PythonScripts.PythonScript"/>
</pickle>
<pickle>
<dictionary>
<item>
<key> <string>Script_magic</string> </key>
<value> <int>3</int> </value>
</item>
<item>
<key> <string>_bind_names</string> </key>
<value>
<object>
<klass>
<global name="NameAssignments" module="Shared.DC.Scripts.Bindings"/>
</klass>
<tuple/>
<state>
<dictionary>
<item>
<key> <string>_asgns</string> </key>
<value>
<dictionary>
<item>
<key> <string>name_container</string> </key>
<value> <string>container</string> </value>
</item>
<item>
<key> <string>name_context</string> </key>
<value> <string>context</string> </value>
</item>
<item>
<key> <string>name_m_self</string> </key>
<value> <string>script</string> </value>
</item>
<item>
<key> <string>name_subpath</string> </key>
<value> <string>traverse_subpath</string> </value>
</item>
</dictionary>
</value>
</item>
</dictionary>
</state>
</object>
</value>
</item>
<item>
<key> <string>_params</string> </key>
<value> <string>editor, request</string> </value>
</item>
<item>
<key> <string>_proxy_roles</string> </key>
<value>
<tuple>
<string>Manager</string>
</tuple>
</value>
</item>
<item>
<key> <string>id</string> </key>
<value> <string>PreferenceTool_validatePassword</string> </value>
</item>
</dictionary>
</pickle>
</record>
</ZopeData>
...@@ -10,6 +10,7 @@ ...@@ -10,6 +10,7 @@
<key> <string>delegated_list</string> </key> <key> <string>delegated_list</string> </key>
<value> <value>
<list> <list>
<string>external_validator</string>
<string>title</string> <string>title</string>
</list> </list>
</value> </value>
...@@ -71,6 +72,10 @@ ...@@ -71,6 +72,10 @@
<key> <string>values</string> </key> <key> <string>values</string> </key>
<value> <value>
<dictionary> <dictionary>
<item>
<key> <string>external_validator</string> </key>
<value> <string></string> </value>
</item>
<item> <item>
<key> <string>field_id</string> </key> <key> <string>field_id</string> </key>
<value> <string>my_password</string> </value> <value> <string>my_password</string> </value>
......
...@@ -10,6 +10,7 @@ ...@@ -10,6 +10,7 @@
<key> <string>delegated_list</string> </key> <key> <string>delegated_list</string> </key>
<value> <value>
<list> <list>
<string>external_validator</string>
<string>title</string> <string>title</string>
</list> </list>
</value> </value>
...@@ -71,6 +72,12 @@ ...@@ -71,6 +72,12 @@
<key> <string>values</string> </key> <key> <string>values</string> </key>
<value> <value>
<dictionary> <dictionary>
<item>
<key> <string>external_validator</string> </key>
<value>
<persistent> <string encoding="base64">AAAAAAAAAAI=</string> </persistent>
</value>
</item>
<item> <item>
<key> <string>field_id</string> </key> <key> <string>field_id</string> </key>
<value> <string>my_password</string> </value> <value> <string>my_password</string> </value>
...@@ -93,4 +100,17 @@ ...@@ -93,4 +100,17 @@
</dictionary> </dictionary>
</pickle> </pickle>
</record> </record>
<record id="2" aka="AAAAAAAAAAI=">
<pickle>
<global name="Method" module="Products.Formulator.MethodField"/>
</pickle>
<pickle>
<dictionary>
<item>
<key> <string>method_name</string> </key>
<value> <string>PreferenceTool_validatePassword</string> </value>
</item>
</dictionary>
</pickle>
</record>
</ZopeData> </ZopeData>
...@@ -29,7 +29,10 @@ ...@@ -29,7 +29,10 @@
############################################################################## ##############################################################################
import unittest import unittest
import urllib
from StringIO import StringIO
import time import time
import httplib
from Products.ERP5Type.tests.ERP5TypeTestCase import ERP5TypeTestCase from Products.ERP5Type.tests.ERP5TypeTestCase import ERP5TypeTestCase
from Products.Formulator.Errors import ValidationError from Products.Formulator.Errors import ValidationError
from Products.ERP5Type.Document import newTempBase from Products.ERP5Type.Document import newTempBase
...@@ -739,6 +742,65 @@ class TestAuthenticationPolicy(ERP5TypeTestCase): ...@@ -739,6 +742,65 @@ class TestAuthenticationPolicy(ERP5TypeTestCase):
default_destination_uid = login.getUid(), default_destination_uid = login.getUid(),
validation_state = "expired"))) validation_state = "expired")))
def test_PreferenceTool_changePassword_checks_policy(self):
person = self.createUser(self.id(), password='current')
person.newContent(portal_type = 'Assignment').open()
login = person.objectValues(portal_type='ERP5 Login')[0]
preference = self.portal.portal_catalog.getResultValue(
portal_type='System Preference',
title='Authentication',)
preference.setPreferredMinPasswordLength(10)
self._clearCache()
self.tic()
# too short password is refused
ret = self.publish(
'%s/portal_preferences' % self.portal.getPath(),
basic='%s:current' % self.id(),
stdin=StringIO(urllib.urlencode({
'Base_callDialogMethod:method': '',
'dialog_id': 'PreferenceTool_viewChangePasswordDialog',
'dialog_method': 'PreferenceTool_setNewPassword',
'field_your_current_password': 'current',
'field_your_new_password': 'short',
'field_password_confirm': 'short',
})),
request_method="POST",
handle_errors=False)
self.assertEqual(httplib.OK, ret.getStatus())
self.assertIn(
'<span class="error">Too short.</span>',
ret.getBody())
# if for some reason, PreferenceTool_setNewPassword is called directly,
# the password policy is also checked, so this cause an unhandled exception.
self.login(person.getUserId())
self.assertRaises(
ValueError,
self.portal.PreferenceTool_setNewPassword,
current_password='current',
new_password='short')
# long enough password is accepted
ret = self.publish(
'%s/portal_preferences' % self.portal.getPath(),
basic='%s:current' % self.id(),
stdin=StringIO(urllib.urlencode({
'Base_callDialogMethod:method': '',
'dialog_id': 'PreferenceTool_viewChangePasswordDialog',
'dialog_method': 'PreferenceTool_setNewPassword',
'field_your_current_password': 'current',
'field_your_new_password': 'long_enough_password',
'field_password_confirm': 'long_enough_password',
})),
request_method="POST",
handle_errors=False)
# When password reset is succesful, user is logged out
self.assertEqual(httplib.FOUND, ret.getStatus())
self.assertTrue(ret.getHeader("Location").endswith("/logout"))
# password is changed on the login
self.assertTrue(login.checkPassword('long_enough_password'))
def test_suite(): def test_suite():
......
  • @jerome this crashes when erp5_authentication_policy is not installed, because the type based script Login_analyzePassword does not exist.

    I'm wondering how to fix this issue:

    • patching login_account_provider.analyzePassword to return True (or False) when the method is not found
    • adding a default Login_analyzePassword returning True in erp5_base
    • a combination of both previous proposal
  • Oh you are right, this looks like a mistake. Because analyzePassword is part of the login interface, it seems logical to consider that password is valid when there is no authentication policy, so the first suggestion, returning an empty list when no type-based method, looks good. Adding a default Login_analyzePassword seems a bit more complex because we'll have to take care of skin folder ordering.

    Do you agree ? let me know if you want me to make these changes.

  • Seems fine. Don't bother about this, I'll do the fix.

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