diff --git a/CHANGELOG b/CHANGELOG index a51f7d9063..d3237e52c6 100644 --- a/CHANGELOG +++ b/CHANGELOG @@ -1,3 +1,4 @@ +* Use passlib to hash and update password * Remove unique constraint on attachment name * New icons * Add pyson TimeDelta diff --git a/doc/topics/configuration.rst b/doc/topics/configuration.rst index c335b00adf..d5c4e504e8 100644 --- a/doc/topics/configuration.rst +++ b/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 +`_. +If not path is set, Tryton will use the schemes `bcrypt` or `pbkdf2_sha512`. + +Default: `None` + attachment ---------- diff --git a/doc/topics/install.rst b/doc/topics/install.rst index 00af702f56..ea9e73da08 100644 --- a/doc/topics/install.rst +++ b/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) diff --git a/setup.py b/setup.py index b738113855..7045dd32e6 100644 --- a/setup.py +++ b/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, diff --git a/trytond/res/user.py b/trytond/res/user.py index fcdc85332b..fb38d22e86 100644 --- a/trytond/res/user.py +++ b/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 $$...''' 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): diff --git a/trytond/tests/test_user.py b/trytond/tests/test_user.py index d335606b6b..36245fa5b3 100644 --- a/trytond/tests/test_user.py +++ b/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"