Use passlib

issue4161
review51461002

CHANGELOG

@@ -1,3 +1,4 @@
+* Use passlib to hash and update password
 * Remove unique constraint on attachment name
 * New icons
 * Add pyson TimeDelta

doc/topics/configuration.rst

@@ -334,6 +334,15 @@ The time in seconds until the reset password expires.
 
 Default: `86400` (24h)
 
+passlib
+-------
+
+The path to the `INI file to load as CryptContext
+<https://passlib.readthedocs.io/en/stable/narr/context-tutorial.html#loading-saving-a-cryptcontext>`_.
+If not path is set, Tryton will use the schemes `bcrypt` or `pbkdf2_sha512`.
+
+Default: `None`
+
 attachment
 ----------
 

doc/topics/install.rst

@@ -17,6 +17,7 @@ Prerequisites
     * polib (https://bitbucket.org/izi/polib/wiki/Home)
     * python-sql 0.4 or later (http://code.google.com/p/python-sql/)
     * ipaddress (https://github.com/phihag/ipaddress)
+    * passlib (https://bitbucket.org/ecollins/passlib)
     * Optional: psycopg 2.5.0 or later (http://www.initd.org/)
     * Optional: psycopg2cffi 2.5.0 or later
       (http://github.com/chtd/psycopg2cffi)

setup.py

@@ -110,12 +110,13 @@ setup(name=name,
         'werkzeug',
         'wrapt',
         'ipaddress',
+        'passlib',
         ],
     extras_require={
         'PostgreSQL': pg_require,
         'graphviz': ['pydot'],
         'Levenshtein': ['python-Levenshtein'],
-        'BCrypt': ['bcrypt'],
+        'BCrypt': ['passlib[bcrypt]'],
         'html2text': ['html2text'],
         },
     zip_safe=False,

trytond/res/user.py

@@ -16,6 +16,7 @@ try:
 except ImportError:
     secrets = None
 import ipaddress
+import warnings
 from email.header import Header
 from functools import wraps
 from itertools import groupby
@@ -28,6 +29,8 @@ from sql.conditionals import Coalesce, Case
 from sql.aggregate import Count
 from sql.operators import Concat
 
+from passlib.context import CryptContext
+
 try:
     import bcrypt
 except ImportError:
@@ -57,6 +60,15 @@ logger = logging.getLogger(__name__)
 _has_password = 'password' in config.get(
     'session', 'authentications', default='password').split(',')
 
+passlib_path = config.get('password', 'passlib')
+if passlib_path:
+    CRYPT_CONTEXT = CryptContext.from_path(passlib_path)
+else:
+    schemes = ['pbkdf2_sha512']
+    if bcrypt:
+        schemes.insert(0, 'bcrypt')
+    CRYPT_CONTEXT = CryptContext(schemes=schemes)
+
 
 def gen_password(length=8):
     alphabet = string.ascii_letters + string.digits
@@ -633,30 +645,45 @@ class User(DeactivableMixin, ModelSQL, ModelView):
         user_id, password_hash, password_reset = cls._get_login(login)
         if user_id and password_hash:
             password = parameters['password']
-            if cls.check_password(password, password_hash):
+            valid, new_hash = cls.check_password(password, password_hash)
+            if valid:
+                if new_hash:
+                    logger.info("Update password hash for %s", user_id)
+                    with Transaction().new_transaction() as transaction:
+                        with transaction.set_user(0):
+                            cls.write([cls(user_id)], {
+                                    'password_hash': new_hash,
+                                    })
                 return user_id
         elif user_id and password_reset:
             if password_reset == parameters['password']:
                 return user_id
 
-    @staticmethod
-    def hash_method():
-        return 'bcrypt' if bcrypt else 'sha1'
-
     @classmethod
     def hash_password(cls, password):
         '''Hash given password in the form
         <hash_method>$<password>$<salt>...'''
         if not password:
             return None
-        return getattr(cls, 'hash_' + cls.hash_method())(password)
+        return CRYPT_CONTEXT.hash(password)
 
     @classmethod
     def check_password(cls, password, hash_):
         if not hash_:
             return False
-        hash_method = hash_.split('$', 1)[0]
-        return getattr(cls, 'check_' + hash_method)(password, hash_)
+        try:
+            return CRYPT_CONTEXT.verify_and_update(password, hash_)
+        except ValueError:
+            hash_method = hash_.split('$', 1)[0]
+            warnings.warn(
+                "Use deprecated hash method %s" % hash_method,
+                DeprecationWarning)
+            valid = getattr(cls, 'check_' + hash_method)(password, hash_)
+            if valid:
+                new_hash = CRYPT_CONTEXT.hash(password)
+            else:
+                new_hash = None
+            return valid, new_hash
 
     @classmethod
     def hash_sha1(cls, password):

trytond/tests/test_user.py

@@ -7,7 +7,6 @@ from unittest.mock import patch, ANY
 
 from trytond.tests.test_tryton import activate_module, with_transaction
 from trytond.pool import Pool
-from trytond.res.user import bcrypt
 from trytond.config import config
 from trytond.error import UserError
 from trytond.res import user as user_module
@@ -46,7 +45,7 @@ class UserTestCase(unittest.TestCase):
         config.set('email', 'from', FROM)
         self.addCleanup(lambda: config.set('email', 'from', reset_from))
 
-    def create_user(self, login, password, hash_method=None, email=None):
+    def create_user(self, login, password, email=None):
         pool = Pool()
         User = pool.get('res.user')
 
@@ -54,16 +53,8 @@ class UserTestCase(unittest.TestCase):
                     'name': login,
                     'login': login,
                     'email': email,
-                    }])
-        if hash_method:
-            hash = getattr(User, 'hash_' + hash_method)
-            User.write([user], {
-                    'password_hash': hash(password),
-                    })
-        else:
-            User.write([user], {
                     'password': password,
-                    })
+                    }])
         return user
 
     def check_user(self, login, password):
@@ -87,19 +78,6 @@ class UserTestCase(unittest.TestCase):
         self.create_user('user', '12345')
         self.check_user('user', '12345')
 
-    @with_transaction()
-    def test_test_sha1(self):
-        'Test sha1 password'
-        self.create_user('user', '12345', 'sha1')
-        self.check_user('user', '12345')
-
-    @unittest.skipIf(bcrypt is None, 'requires bcrypt')
-    @with_transaction()
-    def test_test_bcrypt(self):
-        'Test bcrypt password'
-        self.create_user('user', '12345', 'bcrypt')
-        self.check_user('user', '12345')
-
     @with_transaction()
     def test_read_password_hash(self):
         "Test password_hash can not be read"