[FIX] password_security: Fix password stored and token to reset password invalid (#859)

* [FIX] password_security: Fix password stored

* [REF] password_security: use a unified check_password private method to validate rules and history password

password_security/controllers/main.py

@@ -20,7 +20,7 @@ class PasswordSecuritySession(Session):
             dict(map(operator.itemgetter('name', 'value'), fields))
         )
         user_id = request.env.user
-        user_id.check_password(new_password)
+        user_id._check_password(new_password)
         return super(PasswordSecuritySession, self).change_password(fields)
 
 
@@ -29,7 +29,7 @@ class PasswordSecurityHome(AuthSignupHome):
     def do_signup(self, qcontext):
         password = qcontext.get('password')
         user_id = request.env.user
-        user_id.check_password(password)
+        user_id._check_password(password)
         return super(PasswordSecurityHome, self).do_signup(qcontext)
 
     @http.route()

password_security/models/res_users.py

@@ -38,7 +38,7 @@ class ResUsers(models.Model):
     @api.multi
     def write(self, vals):
         if vals.get('password'):
-            self.check_password(vals['password'])
+            self._check_password(vals['password'])
             vals['password_write_date'] = fields.Datetime.now()
         return super(ResUsers, self).write(vals)
 
@@ -55,7 +55,7 @@ class ResUsers(models.Model):
             message.append('\n* ' + _('Numeric digit'))
         if company_id.password_special:
             message.append('\n* ' + _('Special character'))
-        if len(message):
+        if message:
             message = [_('Must contain the following:')] + message
         if company_id.password_length:
             message = [
@@ -65,7 +65,13 @@ class ResUsers(models.Model):
         return '\r'.join(message)
 
     @api.multi
-    def check_password(self, password):
+    def _check_password(self, password):
+        self._check_password_rules(password)
+        self._check_password_history(password)
+        return True
+
+    @api.multi
+    def _check_password_rules(self, password):
         self.ensure_one()
         if not password:
             return True
@@ -81,7 +87,7 @@ class ResUsers(models.Model):
             password_regex.append(r'(?=.*?\W)')
         password_regex.append('.{%d,}$' % company_id.password_length)
         if not re.search(''.join(password_regex), password):
-            raise PassError(_(self.password_match_message()))
+            raise PassError(self.password_match_message())
         return True
 
     @api.multi
@@ -125,7 +131,7 @@ class ResUsers(models.Model):
         return True
 
     @api.multi
-    def _set_password(self, password):
+    def _check_password_history(self, password):
         """ It validates proposed password against existing history
         :raises: PassError on reused password
         """
@@ -138,14 +144,12 @@ class ResUsers(models.Model):
                 recent_passes = rec_id.password_history_ids[
                     0:recent_passes-1
                 ]
-            if len(recent_passes.filtered(
-                lambda r: crypt.verify(password, r.password_crypt)
-            )):
+            if recent_passes.filtered(
+                    lambda r: crypt.verify(password, r.password_crypt)):
                 raise PassError(
                     _('Cannot use the most recent %d passwords') %
                     rec_id.company_id.password_history
                 )
-        super(ResUsers, self)._set_password(password)
 
     @api.multi
     def _set_encrypted_password(self, encrypted):

password_security/tests/test_password_security_home.py

@@ -68,7 +68,7 @@ class TestPasswordSecurityHome(TransactionCase):
     def test_do_signup_check(self):
         """ It should check password on user """
         with self.mock_assets() as assets:
-            check_password = assets['request'].env.user.check_password
+            check_password = assets['request'].env.user._check_password
             check_password.side_effect = EndTestException
             with self.assertRaises(EndTestException):
                 self.password_security_home.do_signup(self.qcontext)

password_security/tests/test_password_security_session.py

@@ -40,7 +40,7 @@ class TestPasswordSecuritySession(TransactionCase):
     def test_change_password_check(self):
         """ It should check password on request user """
         with self.mock_assets() as assets:
-            check_password = assets['request'].env.user.check_password
+            check_password = assets['request'].env.user._check_password
             check_password.side_effect = EndTestException
             with self.assertRaises(EndTestException):
                 self.password_security_session.change_password(self.fields)

password_security/tests/test_res_users.py

@@ -68,14 +68,14 @@ class TestResUsers(TransactionCase):
     def test_check_password_returns_true_for_valid_password(self):
         rec_id = self._new_record()
         self.assertTrue(
-            rec_id.check_password('asdQWE123$%^3'),
+            rec_id._check_password('asdQWE123$%^3'),
             'Password is valid but check failed.',
         )
 
     def test_check_password_raises_error_for_invalid_password(self):
         rec_id = self._new_record()
         with self.assertRaises(PassError):
-            rec_id.check_password('password')
+            rec_id._check_password('password')
 
     def test_save_password_crypt(self):
         rec_id = self._new_record()