This is an automated email from the ASF dual-hosted git repository.

brondsem pushed a commit to branch db/8566
in repository https://gitbox.apache.org/repos/asf/allura.git

commit 6d78f55700ed4388de36a447a4f1fff1d5ad1c52
Author: Dave Brondsema <dbronds...@slashdotmedia.com>
AuthorDate: Tue Jul 2 18:08:27 2024 -0400

    [#8566] support more password hashing algorithms
    
    - replace crypt with passlib
    - use recommended rounds & salts by default
    - automatically rehash user passwords to new algorithm upon login
    - some AuthProvider method changes
    - speed up bootstrap.py, since password hashing takes longer now
---
 Allura/allura/lib/plugin.py                        | 189 ++++++++++++++-------
 Allura/allura/model/auth.py                        |   3 +-
 Allura/allura/tests/decorators.py                  |  13 +-
 Allura/allura/tests/functional/test_auth.py        |  16 +-
 Allura/allura/tests/functional/test_home.py        |   6 +-
 Allura/allura/tests/model/test_auth.py             |   8 +-
 Allura/allura/tests/test_plugin.py                 |  67 +++++++-
 .../allura/tests/unit/test_ldap_auth_provider.py   |  38 +++--
 Allura/allura/websetup/bootstrap.py                |  18 +-
 Allura/development.ini                             |  26 ++-
 pytest.ini                                         |   4 +
 requirements-optional.txt                          |   4 +
 requirements.in                                    |   1 +
 requirements.txt                                   |   6 +-
 14 files changed, 288 insertions(+), 111 deletions(-)

diff --git a/Allura/allura/lib/plugin.py b/Allura/allura/lib/plugin.py
index 0dea14d29..85076c6ce 100644
--- a/Allura/allura/lib/plugin.py
+++ b/Allura/allura/lib/plugin.py
@@ -24,8 +24,9 @@ import os
 import logging
 import subprocess
 import string
-import crypt
 import random
+import sys
+import warnings
 from contextlib import contextmanager
 from urllib.parse import urlparse
 from io import BytesIO
@@ -36,8 +37,10 @@ from datetime import datetime, timedelta
 import typing
 import calendar
 
+import passlib.ifc
 import requests
 import six
+from passlib.context import CryptContext
 
 try:
     import ldap
@@ -49,8 +52,9 @@ import tg
 from tg import config, request, redirect, response, flash
 from tg import tmpl_context as c, app_globals as g
 from webob import exc, Request
-from paste.deploy.converters import asbool, asint
+from paste.deploy.converters import asbool, asint, aslist
 from formencode import validators as fev
+from passlib.registry import get_crypt_handler
 
 from ming.utils import LazyProperty
 from ming.odm import state
@@ -65,6 +69,7 @@ from allura.tasks.index_tasks import 
solr_del_project_artifacts
 
 if typing.TYPE_CHECKING:
     from allura.app import SitemapEntry
+    from allura import model as M
 
 log = logging.getLogger(__name__)
 
@@ -93,6 +98,8 @@ class AuthenticationProvider:
         '/auth/multifactor',
         '/auth/do_multifactor',
     ]
+    cfg_prefix_pwds = 'auth.password.'
+    default_pwd_algo = 'scrypt'  # noqa: S105
 
     def __init__(self, request):
         self.request = request
@@ -264,12 +271,23 @@ class AuthenticationProvider:
         self.session.save()
         response.set_cookie('memorable_forget', '/', 
secure=request.environ['beaker.session'].secure)
 
-    def validate_password(self, user, password):
+    def validate_password(self, user: M.User, password: str) -> bool:
+        ok = self._validate_password(user, password)
+        if ok:
+            self.rehash_password_if_needed(user, password)
+        return ok
+
+    def rehash_password_if_needed(self, user: M.User, password: str) -> None:
+        if user.password_algorithm != self._password_algorithm():
+            self.set_password(user, None, password, set_timestamp=False)
+            h.auditlog_user('Rehashed password automatically', user=user)
+
+    def _validate_password(self, user: M.User, password: str) -> bool:
         '''Check that provided password matches actual user password
 
         :rtype: bool
         '''
-        raise NotImplementedError('validate_password')
+        raise NotImplementedError('_validate_password')
 
     def disable_user(self, user, **kw):
         '''Disable user account'''
@@ -295,13 +313,15 @@ class AuthenticationProvider:
         '''
         raise NotImplementedError('by_username')
 
-    def set_password(self, user, old_password, new_password):
+    def set_password(self, user: M.User, old_password: str | None, 
new_password: str, set_timestamp=True):
         '''
         Set a user's password.
 
-        A provider implementing this method should store the timestamp of this 
change, either
+        A provider implementing this method should store the timestamp of this 
change when set_timestamp=True, either
         on ``user.last_password_updated`` or somewhere else that a custom 
``get_last_password_updated`` method uses.
 
+        Also should set user.password_algorithm to the password algorithm used.
+
         :param user: a :class:`User <allura.model.auth.User>`
         :rtype: None
         :raises: HTTPUnauthorized if old_password is not valid
@@ -504,12 +524,54 @@ class AuthenticationProvider:
             validator._messages['invalid'] = 'Usernames only include small 
letters, numbers, and dashes'
         return validator
 
+    def _passlib_crypt_by_id(self, algorithm: str) -> passlib.ifc.PasswordHash:
+        return get_crypt_handler(algorithm)
+
+    def _encode_password_legacy_sha256(self, password: str, salt: str) -> str:
+        from allura import model as M
+
+        if salt is None:
+            salt = ''.join(chr(randint(1, 0x7f))
+                           for i in range(M.User.SALT_LEN))
+        hashpass = sha256((salt + password).encode('utf-8')).digest()
+        return 'sha256' + salt + six.ensure_text(b64encode(hashpass))
+
+    def _password_algorithm(self) -> str:
+        return config.get(self.cfg_prefix_pwds + 'algorithm', 
self.default_pwd_algo)
+
+    def _encode_password(self, password: str, salt=None) -> tuple[str, str]:
+        algorithm = self._password_algorithm()
+
+        if algorithm == 'allura_sha256':
+            return self._encode_password_legacy_sha256(password, salt), 
algorithm
+
+        crypt = self._passlib_crypt_by_id(algorithm)
+
+        passlib_kwargs = dict()
+        if crypt.name.startswith('bcrypt'):
+            passlib_kwargs['truncate_error'] = True
+
+        rounds = config.get(self.cfg_prefix_pwds + 'rounds')
+        if rounds:
+            if int(rounds) < getattr(crypt, 'default_rounds', 0):
+                warnings.warn(f'rounds for {crypt.name} should be at least 
{crypt.default_rounds}', stacklevel=1)
+            passlib_kwargs['rounds'] = rounds
+
+        salt_len = config.get(self.cfg_prefix_pwds + 'salt_len')
+        if salt_len:
+            passlib_kwargs['salt_size'] = int(salt_len)
+        if salt:
+            if crypt._salt_is_bytes:
+                salt = salt.encode()  # must be bytes
+            passlib_kwargs['salt'] = salt
+        encrypted = crypt.using(**passlib_kwargs).hash(password)
+        return encrypted, algorithm
+
 
 class LocalAuthenticationProvider(AuthenticationProvider):
 
     '''
-    Stores user passwords on the User model, in mongo.  Uses per-user salt and
-    SHA-256 encryption.
+    Stores user password hashes on the User model, in mongo.
     '''
 
     forgotten_password_process = True
@@ -525,7 +587,7 @@ class LocalAuthenticationProvider(AuthenticationProvider):
 
     def _login(self):
         user = self.by_username(self.request.params['username'])
-        if not self._validate_password(user, self.request.params['password']):
+        if not self.validate_password(user, self.request.params['password']):
             raise exc.HTTPUnauthorized()
         return user
 
@@ -553,19 +615,27 @@ class LocalAuthenticationProvider(AuthenticationProvider):
         if kw.get('audit', True):
             h.auditlog_user('Account changed to pending', user=user)
 
-    def validate_password(self, user, password):
-        return self._validate_password(user, password)
-
-    def _validate_password(self, user, password):
+    def _validate_password(self, user: M.User, password) -> bool:
         if user is None:
             return False
         if not user.password:
             return False
-        salt = str(user.password[6:6 + user.SALT_LEN])
-        check = self._encode_password(password, salt)
-        if check != user.password:
-            return False
-        return True
+
+        algorithm = config.get('auth.password.algorithm', 
self.default_pwd_algo)
+        old_algos = aslist(config.get('auth.password.algorithm.old', 
'allura_sha256'))
+
+        if user.password.startswith('sha256') and 'allura_sha256' in old_algos:
+            # legacy.  'allura_sha256' is a custom name for our old logic
+            salt = str(user.password[6:6 + user.SALT_LEN])
+            check = self._encode_password_legacy_sha256(password, salt)
+            return check == user.password
+        else:
+            # 
https://passlib.readthedocs.io/en/stable/narr/context-tutorial.html#deprecation-hash-migration
+            crypt = CryptContext(
+                schemes=[self._passlib_crypt_by_id(algorithm)],
+                deprecated=[self._passlib_crypt_by_id(algo) for algo in 
old_algos if algo != 'allura_sha256'],
+            )
+            return crypt.verify(password, user.password)
 
     def by_username(self, username):
         from allura import model as M
@@ -576,22 +646,16 @@ class LocalAuthenticationProvider(AuthenticationProvider):
         rex = r'^' + un + '$'
         return M.User.query.get(username={'$regex': rex}, disabled=False, 
pending=False)
 
-    def set_password(self, user, old_password, new_password):
+    def set_password(self, user, old_password, new_password, 
set_timestamp=True):
         if old_password is not None and not self.validate_password(user, 
old_password):
             raise exc.HTTPUnauthorized()
         else:
-            user.password = self._encode_password(new_password)
-            user.last_password_updated = datetime.utcnow()
+            user.password, algorithm = self._encode_password(new_password)
+            user.password_algorithm = algorithm
+            if set_timestamp:
+                user.last_password_updated = datetime.utcnow()
             session(user).flush(user)
 
-    def _encode_password(self, password, salt=None):
-        from allura import model as M
-        if salt is None:
-            salt = ''.join(chr(randint(1, 0x7f))
-                           for i in range(M.User.SALT_LEN))
-        hashpass = sha256((salt + password).encode('utf-8')).digest()
-        return 'sha256' + salt + six.ensure_text(b64encode(hashpass))
-
     def user_project_shortname(self, user):
         # "_" isn't valid for subdomains (which project names are used with)
         # so if a username has a "_" we change it to "-"
@@ -671,9 +735,13 @@ def ldap_user_dn(username):
 class LdapAuthenticationProvider(AuthenticationProvider):
 
     forgotten_password_process = True
+    cfg_prefix_pwds = 'auth.ldap.password.'
+    default_pwd_algo = 'ldap_pbkdf2_sha512'  # noqa: S105
 
     def register_user(self, user_doc):
         from allura import model as M
+        pwd_hash, algorithm = self._encode_password(user_doc['password'])
+        user_doc['password_algorithm'] = algorithm
         result = M.User(**user_doc)
         if asbool(config.get('auth.ldap.autoregister', True)):
             if asbool(config.get('auth.allow_user_registration', True)):
@@ -690,7 +758,7 @@ class LdapAuthenticationProvider(AuthenticationProvider):
         display_name = user_doc['display_name'].encode('utf-8')
         ldif_u = modlist.addModlist(dict(
             uid=uname,
-            userPassword=self._encode_password(user_doc['password']),
+            userPassword=pwd_hash,
             objectClass=[b'account', b'posixAccount'],
             cn=display_name,
             uidNumber=uid,
@@ -732,39 +800,44 @@ class LdapAuthenticationProvider(AuthenticationProvider):
                           username, errmsg)
             raise AssertionError(errmsg)
 
-    def _get_salt(self, length):
-        def random_char():
-            return random.choice(string.ascii_uppercase + string.digits)
-        return ''.join(random_char() for i in range(length))
-
-    def _encode_password(self, password, salt=None):
-        cfg_prefix = 'auth.ldap.password.'
-        salt_len = asint(config.get(cfg_prefix + 'salt_len', 16))
-        algorithm = config.get(cfg_prefix + 'algorithm', 6)
-        rounds = asint(config.get(cfg_prefix + 'rounds', 6000))
-        salt = self._get_salt(salt_len) if salt is None else salt
-        encrypted = crypt.crypt(
-            six.ensure_str(password),
-            f'${algorithm}$rounds={rounds}${salt}')
-        return b'{CRYPT}%s' % encrypted.encode('utf-8')
-
     def by_username(self, username):
         from allura import model as M
         return M.User.query.get(username=username, disabled=False, 
pending=False)
 
-    def set_password(self, user, old_password, new_password):
+    def _passlib_crypt_by_id(self, algorithm: str) -> passlib.ifc.PasswordHash:
+        try:
+            crypt = get_crypt_handler(algorithm)
+        except KeyError:
+            # handle a few old system "crypt" ids.  
https://passlib.readthedocs.io/en/stable/modular_crypt_format.html#mcf-identifiers
+            crypt = get_crypt_handler(
+                {
+                    # '1': 'ldap_md5_crypt',   # could support this, but it is 
super weak
+                    '2b': 'ldap_bcrypt',
+                    '5': 'ldap_sha256_crypt',
+                    '6': 'ldap_sha512_crypt',
+                }[algorithm]
+            )
+        else:
+            if not algorithm.startswith('ldap_'):
+                raise ValueError(f'LDAP algorithms should always start with 
ldap_ (got {algorithm})')
+        return crypt
+
+
+    def set_password(self, user, old_password, new_password, 
set_timestamp=True):
         dn = ldap_user_dn(user.username)
-        if old_password:
+        if old_password is not None:
             ldap_ident = dn
             ldap_pass = old_password.encode('utf-8')
         else:
             ldap_ident = ldap_pass = None
         try:
-            new_password = self._encode_password(new_password)
+            new_password, algorithm = self._encode_password(new_password)
             with ldap_conn(ldap_ident, ldap_pass) as con:
                 con.modify_s(
-                    dn, [(ldap.MOD_REPLACE, 'userPassword', new_password)])
-            user.last_password_updated = datetime.utcnow()
+                    dn, [(ldap.MOD_REPLACE, 'userPassword', 
new_password.encode('utf-8'))])
+            if set_timestamp:
+                user.last_password_updated = datetime.utcnow()
+            user.password_algorithm = algorithm
             session(user).flush(user)
         except ldap.INVALID_CREDENTIALS:
             raise exc.HTTPUnauthorized()
@@ -778,7 +851,7 @@ class LdapAuthenticationProvider(AuthenticationProvider):
             username = str(self.request.params['username'])
         except UnicodeEncodeError:
             raise exc.HTTPBadRequest('Unicode is not allowed in usernames')
-        if not self._validate_password(username, 
self.request.params['password']):
+        if not self._validate_password_username(username, 
self.request.params['password']):
             raise exc.HTTPUnauthorized()
         user = M.User.query.get(username=username)
         if user is None:
@@ -792,17 +865,17 @@ class LdapAuthenticationProvider(AuthenticationProvider):
             else:
                 log.debug(f'LdapAuth: no user {username} found in local mongo')
                 raise exc.HTTPUnauthorized()
-        elif user.disabled or user.pending:
+        else:
+            self.rehash_password_if_needed(user, 
self.request.params['password'])
+        if user.disabled or user.pending:
             log.debug(f'LdapAuth: user {username} is disabled or pending in 
Allura')
             raise exc.HTTPUnauthorized()
         return user
 
-    def validate_password(self, user, password):
-        '''by user'''
-        return self._validate_password(user.username, password)
+    def _validate_password(self, user: M.User, password: str) -> bool:
+        return self._validate_password_username(user.username, password)
 
-    def _validate_password(self, username, password):
-        '''by username'''
+    def _validate_password_username(self, username: str, password: str) -> 
bool:
         password = h.really_unicode(password).encode('utf-8')
         try:
             ldap_user = ldap_user_dn(username)
diff --git a/Allura/allura/model/auth.py b/Allura/allura/model/auth.py
index 0b73f1b34..23ed932ba 100644
--- a/Allura/allura/model/auth.py
+++ b/Allura/allura/model/auth.py
@@ -259,8 +259,9 @@ class User(MappedClass, ActivityNode, ActivityObject, 
SearchIndexable):
     sfx_userid = FieldProperty(S.Deprecated)
     username = FieldProperty(str)
     email_addresses = FieldProperty([str])
-    password = FieldProperty(str)
+    password = FieldProperty(str)  # hashed
     last_password_updated = FieldProperty(datetime)  # to access, use 
AuthProvider's get_last_password_updated
+    password_algorithm = FieldProperty(str)
     reg_date = FieldProperty(datetime)  # to access, use 
user.registration_date()
     projects = FieldProperty(S.Deprecated)
     # full mount point: prefs dict
diff --git a/Allura/allura/tests/decorators.py 
b/Allura/allura/tests/decorators.py
index 7311f58be..f72c15648 100644
--- a/Allura/allura/tests/decorators.py
+++ b/Allura/allura/tests/decorators.py
@@ -151,7 +151,7 @@ class patch_middleware_config:
 
 
 @contextlib.contextmanager
-def audits(*messages, **kwargs):
+def audits(*messages, user=False, actor=r'.*', ip_addr=r'.*', 
user_agent=r'.*'):
     """
     Asserts all the messages exist in audit log
 
@@ -161,10 +161,7 @@ def audits(*messages, **kwargs):
     """
     M.AuditLog.query.remove()
     yield
-    if kwargs.get('user'):
-        actor = kwargs.get('actor', '.*')
-        ip_addr = kwargs.get('ip_addr', '.*')
-        user_agent = kwargs.get('user_agent', '.*')
+    if user:
         preamble = f'(Done by user: {actor}\n)?IP Address: 
{ip_addr}\nUser-Agent: {user_agent}\n'
     else:
         preamble = ''
@@ -182,7 +179,7 @@ def audits(*messages, **kwargs):
 
 
 @contextlib.contextmanager
-def out_audits(*messages, **kwargs):
+def out_audits(*messages, user=False, actor=r'.*', ip_addr=r'.*'):
     """
     Asserts none the messages exist in audit log.  "without audits"
 
@@ -192,9 +189,7 @@ def out_audits(*messages, **kwargs):
     """
     M.AuditLog.query.remove()
     yield
-    if kwargs.get('user'):
-        actor = kwargs.get('actor', '.*')
-        ip_addr = kwargs.get('ip_addr', '.*')
+    if user:
         preamble = f'(Done by user: {actor}\n)?IP Address: {ip_addr}\n'
     else:
         preamble = ''
diff --git a/Allura/allura/tests/functional/test_auth.py 
b/Allura/allura/tests/functional/test_auth.py
index be33727bb..868dbc26f 100644
--- a/Allura/allura/tests/functional/test_auth.py
+++ b/Allura/allura/tests/functional/test_auth.py
@@ -213,6 +213,18 @@ class TestAuth(TestController):
         with audits('Failed login', user=True):
             r = f.submit(extra_environ={'username': '*anonymous'})
 
+    @pytest.mark.parametrize('old_algo', ['allura_sha256', 'sha512_crypt'])
+    def test_login_old_password_hash(self, old_algo):
+        u = M.User.query.get(username='test-user')
+        u.password_algorithm = old_algo
+        r = self.app.get('/auth/', extra_environ={'username': '*anonymous'})
+        f = r.forms[0]
+        encoded = self.app.antispam_field_names(f)
+        f[encoded['username']] = 'test-user'
+        f[encoded['password']] = 'foo'
+        with audits('Successful login', 'Rehashed password automatically', 
user=True):
+            r = f.submit(extra_environ={'username': '*anonymous'})
+
     def test_login_overlay(self):
         r = self.app.get('/auth/login_fragment/', extra_environ={'username': 
'*anonymous'})
         f = r.forms[0]
@@ -1745,7 +1757,7 @@ To update your password on %s, please visit the following 
URL:
         user = M.User.query.get(username='test-admin')
         assert old_pw_hash != user.password
         provider = plugin.LocalAuthenticationProvider(None)
-        assert provider._validate_password(user, new_password)
+        assert provider.validate_password(user, new_password)
 
         # confirm reset fields cleared
         user = M.User.query.get(username='test-admin')
@@ -1860,7 +1872,7 @@ To update your password on %s, please visit the following 
URL:
         # confirm password changed and works
         user = M.User.query.get(username='test-admin')
         provider = plugin.LocalAuthenticationProvider(None)
-        assert provider._validate_password(user, new_password)
+        assert provider.validate_password(user, new_password)
 
         # confirm can log in now in same session
         r = r.follow()
diff --git a/Allura/allura/tests/functional/test_home.py 
b/Allura/allura/tests/functional/test_home.py
index 1e668e219..24601cd3e 100644
--- a/Allura/allura/tests/functional/test_home.py
+++ b/Allura/allura/tests/functional/test_home.py
@@ -199,10 +199,9 @@ class TestProjectHome(TestController):
         test_project.add_user(M.User.by_username('test-user-1'), ['B_role'])
         test_project.add_user(M.User.by_username('test-user'), ['Developer'])
         test_project.add_user(M.User.by_username('test-user-0'), ['Member'])
-        test_project.add_user(M.User.by_username('test-user-2'), ['Member'])
+        test_project.add_user(M.User.by_username('test-user-2'), ['Admin'])
         test_project.add_user(M.User.by_username('test-user-3'), ['Member'])
         test_project.add_user(M.User.by_username('test-user-3'), ['Developer'])
-        test_project.add_user(M.User.by_username('test-user-4'), ['Admin'])
         ThreadLocalODMSession.flush_all()
         r = self.app.get('/p/test/_members/')
 
@@ -211,12 +210,11 @@ class TestProjectHome(TestController):
         assert '<td>Admin</td>' in r
         tr = r.html.findAll('tr')
         assert "<td>Test Admin</td>" in str(tr[1])
-        assert "<td>Test User 4</td>" in str(tr[2])
+        assert "<td>Test User 2</td>" in str(tr[2])
         assert "<td>Test User</td>" in str(tr[3])
         assert "<td>Test User 3</td>" in str(tr[4])
         assert "<td>Test User 0</td>" in str(tr[5])
         assert "<td>Test User 1</td>" in str(tr[6])
-        assert "<td>Test User 2</td>" in str(tr[7])
 
     def test_members_anonymous(self):
         r = self.app.get('/p/test/_members/',
diff --git a/Allura/allura/tests/model/test_auth.py 
b/Allura/allura/tests/model/test_auth.py
index 11cd00cb4..4f6e708e4 100644
--- a/Allura/allura/tests/model/test_auth.py
+++ b/Allura/allura/tests/model/test_auth.py
@@ -135,11 +135,11 @@ class TestAuth:
         assert len(roles) == 3, roles
         u.set_password('foo')
         provider = plugin.LocalAuthenticationProvider(Request.blank('/'))
-        assert provider._validate_password(u, 'foo')
-        assert not provider._validate_password(u, 'foobar')
+        assert provider.validate_password(u, 'foo')
+        assert not provider.validate_password(u, 'foobar')
         u.set_password('foobar')
-        assert provider._validate_password(u, 'foobar')
-        assert not provider._validate_password(u, 'foo')
+        assert provider.validate_password(u, 'foobar')
+        assert not provider.validate_password(u, 'foo')
 
     def test_user_project_creates_on_demand(self):
         u = M.User.register(dict(username='foobar123'), make_project=False)
diff --git a/Allura/allura/tests/test_plugin.py 
b/Allura/allura/tests/test_plugin.py
index 085e3a143..a75e444f5 100644
--- a/Allura/allura/tests/test_plugin.py
+++ b/Allura/allura/tests/test_plugin.py
@@ -14,12 +14,15 @@
 #       KIND, either express or implied.  See the License for the
 #       specific language governing permissions and limitations
 #       under the License.
-
+import contextlib
 import datetime as dt
 import calendar
+from unittest import SkipTest
 
+import passlib
 import tg
 from tg import tmpl_context as c
+from tg import config
 from webob import Request, exc
 from bson import ObjectId
 from ming.odm.odmsession import ThreadLocalODMSession
@@ -30,7 +33,7 @@ from allura import model as M
 from allura.lib import plugin
 from allura.lib import phone
 from allura.lib import helpers as h
-from allura.lib.plugin import ProjectRegistrationProvider
+from allura.lib.plugin import ProjectRegistrationProvider, 
AuthenticationProvider
 from allura.lib.plugin import ThemeProvider
 from allura.lib.exceptions import ProjectConflict, ProjectShortnameInvalid
 from allura.tests.decorators import audits
@@ -624,21 +627,69 @@ class TestLocalAuthenticationProvider:
         setup_global_objects()
         self.provider = plugin.LocalAuthenticationProvider(Request.blank('/'))
 
-    def test_password_encoder(self):
+    @patch.dict(tg.config, {'auth.password.algorithm': 'allura_sha256'})
+    def test_password_encoder_legacy(self):
         # Verify salt
         ep = self.provider._encode_password
         assert ep('test_pass') != ep('test_pass')
         assert ep('test_pass', '0000') == ep('test_pass', '0000')
-        assert ep('test_pass', '0000') == 
'sha2560000j7pRjKKZ5L8G0jScZKja9ECmYF2zBV82Mi+E3wkop30='
+        assert ep('test_pass', '0000') == 
('sha2560000j7pRjKKZ5L8G0jScZKja9ECmYF2zBV82Mi+E3wkop30=', 'allura_sha256')
+
+    
@pytest.mark.parametrize('algorithm,rounds,specific_salt,salt_len,expected_config',
 [
+        ('sha512_crypt', None,None, None, '$6$rounds=656000$'),
+        ('pbkdf2_sha256', None,None, None, '$pbkdf2-sha256$29000$'),
+        ('pbkdf2_sha512', None,None, None, '$pbkdf2-sha512$25000$'),
+        # test with explicit # of rounds & salt_len
+        ('pbkdf2_sha512', 26789, None, 50, '$pbkdf2-sha512$26789$'),
+        ('bcrypt_sha256', None, 'O'*22, None, '$bcrypt-sha256$v=2,t=2b,r=12$'),
+        ('scrypt', None, None, None, '$scrypt$ln=16,r=8,p=1$'),
+        ('argon2', None, '0'*8, None, '$argon2id$v=19$m=65536,t=3,p=4$'),
+    ])
+    def test_password_encoder(self, algorithm: str, rounds, specific_salt, 
salt_len, expected_config):
+        self._test_password_encoder(
+            'auth.password.',
+            self.provider,
+            algorithm, rounds, specific_salt, salt_len, expected_config,
+        )
+
+    # shared by test_ldap_auth_provider.py
+    @staticmethod
+    def _test_password_encoder(cfg_prefix: str, provider: 
AuthenticationProvider,
+                               algorithm, rounds, specific_salt, salt_len, 
expected_config):
+        if specific_salt is None:
+            specific_salt = '0000'
+        if rounds is not None:
+            rounds_patch = patch.dict(config, {f'{cfg_prefix}rounds': 
str(rounds)})
+        else:
+            rounds_patch = contextlib.nullcontext()
+        if salt_len is not None:
+            salt_len_patch = patch.dict(config, {f'{cfg_prefix}salt_len': 
str(salt_len)})
+        else:
+            salt_len_patch = contextlib.nullcontext()
+        with patch.dict(config, {f'{cfg_prefix}algorithm': algorithm}), 
rounds_patch, salt_len_patch:
+            ep = provider._encode_password
+            try:
+                # different values because of salting
+                assert ep('test_pass') != ep('test_pass')
+            except passlib.exc.MissingBackendError as e:
+                if 'bcrypt' in str(e):
+                    raise SkipTest('bcrypt not available')
+                if 'argon2' in str(e):
+                    raise SkipTest('argon2 not available')
+
+            # same value when same salt
+            assert ep('test_pass', specific_salt) == ep('test_pass', 
specific_salt)
+
+            # Test password format
+            assert ep('pwd')[0].startswith(expected_config)
 
     def test_set_password_with_old_password(self):
         user = Mock()
         user.__ming__ = Mock()
         self.provider.validate_password = lambda u, p: False
-        self.provider._encode_password = Mock()
-        pytest.raises(
-            exc.HTTPUnauthorized,
-            self.provider.set_password, user, 'old', 'new')
+        self.provider._encode_password = Mock(return_value=['asdfasdf', 
'allura_sha256'])
+        with pytest.raises(exc.HTTPUnauthorized):
+            self.provider.set_password(user, 'old', 'new')
         assert self.provider._encode_password.call_count == 0
 
         self.provider.validate_password = lambda u, p: True
diff --git a/Allura/allura/tests/unit/test_ldap_auth_provider.py 
b/Allura/allura/tests/unit/test_ldap_auth_provider.py
index 818b09e17..b1c4d1f04 100644
--- a/Allura/allura/tests/unit/test_ldap_auth_provider.py
+++ b/Allura/allura/tests/unit/test_ldap_auth_provider.py
@@ -16,13 +16,14 @@
 #       under the License.
 
 import calendar
-import platform
 from datetime import datetime
 
+import pytest
 from bson import ObjectId
 from mock import patch, Mock
-from unittest import SkipTest
 from webob import Request
+
+from allura.tests.test_plugin import TestLocalAuthenticationProvider
 from ming.odm.odmsession import ThreadLocalODMSession
 from tg import config
 
@@ -30,7 +31,6 @@ from alluratest.controller import setup_basic_test
 from allura.lib import plugin
 from allura.lib import helpers as h
 from allura import model as M
-import six
 
 
 class TestLdapAuthenticationProvider:
@@ -39,22 +39,26 @@ class TestLdapAuthenticationProvider:
         setup_basic_test()
         self.provider = plugin.LdapAuthenticationProvider(Request.blank('/'))
 
-    def test_password_encoder(self):
-        # Verify salt
-        ep = self.provider._encode_password
-        # Note: OSX uses a crypt library with a known issue relating the 
hashing algorithms.
-        if b'$6$rounds=' not in ep('pwd') and platform.system() == 'Darwin':
-            raise SkipTest
-        assert ep('test_pass') != ep('test_pass')
-        assert ep('test_pass', '0000') == ep('test_pass', '0000')
-        # Test password format
-        assert ep('pwd').startswith(b'{CRYPT}$6$rounds=6000$')
+    
@pytest.mark.parametrize('algorithm,rounds,specific_salt,salt_len,expected_config',
 [
+        ('2b', None, 'O'*22, None, '{CRYPT}$2b$12'),
+        ('5', None, None, None, '{CRYPT}$5$rounds=535000$'),
+        ('6', None, None, None, '{CRYPT}$6$rounds=656000$'),
+        ('ldap_pbkdf2_sha256', None, None, None, '{PBKDF2-SHA256}29000$'),
+        ('ldap_pbkdf2_sha512', None, None, None, '{PBKDF2-SHA512}25000$'),
+        ('ldap_bcrypt', None, 'O'*22, None, '{CRYPT}$2b$12$'),
+    ])
+    def test_password_encoder(self, algorithm: str, rounds, specific_salt, 
salt_len, expected_config):
+        TestLocalAuthenticationProvider._test_password_encoder(
+            'auth.ldap.password.',
+            self.provider,
+            algorithm, rounds, specific_salt, salt_len, expected_config,
+        )
 
     @patch('allura.lib.plugin.ldap')
     def test_set_password(self, ldap):
         user = Mock(username='test-user')
         user.__ming__ = Mock()
-        self.provider._encode_password = Mock(return_value=b'new-pass-hash')
+        self.provider._encode_password = Mock(return_value=('new-pass-hash', 
'somealgo'))
         ldap.dn.escape_dn_chars = lambda x: x
 
         dn = 'uid=%s,ou=people,dc=localdomain' % user.username
@@ -75,11 +79,13 @@ class TestLdapAuthenticationProvider:
         self.provider.request.method = 'POST'
         self.provider.request.body = '&'.join([f'{k}={v}' for k, v in 
params.items()]).encode('utf-8')
         ldap.dn.escape_dn_chars = lambda x: x
+        user = M.User.query.get(username=params['username'])
+        user.password_algorithm = self.provider._password_algorithm()  # 
default is non-ldap algo so would cause rehash
 
         self.provider._login()
 
         dn = 'uid=%s,ou=people,dc=localdomain' % params['username']
-        ldap.initialize.assert_called_once_with('ldaps://localhost/')
+        ldap.initialize.assert_called_with('ldaps://localhost/')
         connection = ldap.initialize.return_value
         connection.simple_bind_s.assert_called_once_with(dn, b'test-password')
         assert connection.unbind_s.call_count == 1
@@ -113,7 +119,7 @@ class TestLdapAuthenticationProvider:
             'password': 'new-password',
         }
         ldap.dn.escape_dn_chars = lambda x: x
-        self.provider._encode_password = 
Mock(return_value=b'new-password-hash')
+        self.provider._encode_password = 
Mock(return_value=('new-password-hash', 'somealgo'))
 
         assert M.User.query.get(username=user_doc['username']) is None
         with h.push_config(config, **{'auth.ldap.autoregister': 'false'}):
diff --git a/Allura/allura/websetup/bootstrap.py 
b/Allura/allura/websetup/bootstrap.py
index 21cfa6781..dba5f136f 100644
--- a/Allura/allura/websetup/bootstrap.py
+++ b/Allura/allura/websetup/bootstrap.py
@@ -20,6 +20,7 @@ import os
 import sys
 import logging
 import shutil
+from datetime import datetime
 from textwrap import dedent
 
 import tg
@@ -27,6 +28,7 @@ from tg import tmpl_context as c, app_globals as g
 from paste.deploy.converters import asbool
 import ew
 
+from allura.lib.decorators import memoize
 from allura.model.oauth import dummy_oauths
 from ming import Session, mim
 from ming.odm import state, session
@@ -178,7 +180,7 @@ def bootstrap(command, conf, vars):
 
     if create_test_data:
         # Add some test users
-        for unum in range(10):
+        for unum in range(4):
             make_user('Test User %d' % unum)
 
     log.info('Creating basic project categories')
@@ -205,7 +207,7 @@ def bootstrap(command, conf, vars):
             u_admin = make_user('Test Admin')
             u_admin.preferences = 
dict(email_address='test-admin@users.localhost')
             u_admin.email_addresses = ['test-admin@users.localhost']
-            u_admin.set_password('foo')
+            fast_set_pwd(u_admin, 'foo')
             u_admin.claim_address('test-admin@users.localhost')
             ThreadLocalODMSession.flush_all()
 
@@ -306,6 +308,16 @@ def clear_all_database_tables():
                 continue
             db.drop_collection(coll)
 
+# this re-uses hashes for the same pwd, which gives a huge speedup during 
tests.  Not good for real usage, since salting is the same.
+user_pwd_hash_speedup_cache = {}
+
+def fast_set_pwd(user: M.User, password: str):
+    if password in user_pwd_hash_speedup_cache:
+        user.password, user.password_algorithm = 
user_pwd_hash_speedup_cache[password]
+        user.last_password_updated = datetime.utcnow()
+    else:
+        user.set_password(password)
+        user_pwd_hash_speedup_cache[password] = (user.password, 
user.password_algorithm)
 
 def create_user(display_name, username=None, password='foo', 
make_project=False):  # noqa: S107
     if not username:
@@ -321,7 +333,7 @@ def create_user(display_name, username=None, 
password='foo', make_project=False)
     em.confirmed = True
     em.set_nonce_hash()
     user.set_pref('email_address',email)
-    user.set_password(password)
+    fast_set_pwd(user, password)
     return user
 
 
diff --git a/Allura/development.ini b/Allura/development.ini
index 25384c647..b91356088 100644
--- a/Allura/development.ini
+++ b/Allura/development.ini
@@ -164,6 +164,18 @@ auth.post_logout_url = /
 auth.min_password_len = 6
 auth.max_password_len = 30
 
+; supported values include argon2, scrypt, bcrypt_sha256, pbkdf2_sha512, 
sha512_crypt 
https://passlib.readthedocs.io/en/stable/lib/passlib.hash.html#active-hashes
+; argon2 and bcrypt need additional packages, see requirements-optional.txt
+; 
https://cheatsheetseries.owasp.org/cheatsheets/Password_Storage_Cheat_Sheet.html#password-hashing-algorithms
+; https://passlib.readthedocs.io/en/stable/narr/quickstart.html#choosing-a-hash
+auth.password.algorithm = scrypt
+; list of previously-used algorithms to support when verifying passwords.  
Upon successful login, the password will be rehashed with the current algorithm
+auth.password.algorithm.old = allura_sha256
+; By default passlib's recommended values will be used for rounds & salts, but 
they may be too low.
+; Override them if needed, to match OWASP.org recommendations for example
+; auth.password.rounds =
+; auth.password.salt_len =
+
 ; password expiration options (disabled if neither is set)
 ;auth.pwdexpire.days = 1
 ; unix timestamp:
@@ -177,9 +189,17 @@ auth.ldap.admin_dn = cn=admin,dc=localdomain
 auth.ldap.admin_password = secret
 auth.ldap.schroot_name = scm
 auth.ldap.use_schroot = false
-auth.ldap.password.algorithm = 6
-auth.ldap.password.rounds = 6000
-auth.ldap.password.salt_len = 16
+; supported values include ldap_bcrypt, ldap_pbkdf2_sha512, 
ldap_pbkdf2_sha256, ldap_sha512_crypt 
https://passlib.readthedocs.io/en/stable/lib/passlib.hash.html#standard-ldap-hashes
+; bcrypt needs additional packages, see requirements-optional.txt
+; Legacy "2b" and "5" and "6" mean ldap_bcrypt, ldap_sha256_crypt and 
ldap_sha512_crypt respectively
+; 
https://cheatsheetseries.owasp.org/cheatsheets/Password_Storage_Cheat_Sheet.html#password-hashing-algorithms
+; https://passlib.readthedocs.io/en/stable/narr/quickstart.html#choosing-a-hash
+auth.ldap.password.algorithm = ldap_pbkdf2_sha512
+; By default passlib's recommended values will be used for rounds & salts, but 
they may be too low.
+; Override them if needed, to match OWASP.org recommendations for example
+; auth.ldap.password.rounds = 6000
+; auth.ldap.password.salt_len = 16
+
 ; "autoregister" allows users to log in to Allura with an existing LDAP account
 ; If using ldap, with autoregister, you should also set 
"allow_user_registration"
 ; to false below.
diff --git a/pytest.ini b/pytest.ini
index ffc5fc09b..f5490508c 100644
--- a/pytest.ini
+++ b/pytest.ini
@@ -37,9 +37,13 @@ filterwarnings =
     ignore:pkg_resources is deprecated as an API:DeprecationWarning:formencode
     # supporting py3.8 still then can revert 
https://sourceforge.net/p/activitystream/code/ci/c0884668ac0f4445acb423edb25d18b7bd368be7/
     ignore:SelectableGroups dict interface is deprecated. Use 
select.:DeprecationWarning:activitystream
+    # optional import within passlib
+    ignore:'crypt' is deprecated:DeprecationWarning:passlib.utils
+    ignore:Accessing argon2.__version__ is 
deprecated:DeprecationWarning:passlib.handlers.argon2
 
     # py3.12
     ignore::DeprecationWarning:smtpd
+    ignore:the imp module is 
deprecated:DeprecationWarning:mercurial.utils.resourceutil
 
     # py3.13
     ignore:'cgi' is deprecated:DeprecationWarning:webob.compat
diff --git a/requirements-optional.txt b/requirements-optional.txt
index f68ec31a2..44d140829 100644
--- a/requirements-optional.txt
+++ b/requirements-optional.txt
@@ -9,3 +9,7 @@ mediawiki2html  # GPL
 
 # for spam checking
 akismet>=1.3
+
+# for this type of password hashing
+bcrypt
+argon2-cffi
diff --git a/requirements.in b/requirements.in
index a8c041986..98d20f933 100644
--- a/requirements.in
+++ b/requirements.in
@@ -20,6 +20,7 @@ MarkupSafe
 Ming
 oauthlib
 paginate
+passlib
 Paste
 PasteDeploy
 PasteScript
diff --git a/requirements.txt b/requirements.txt
index be44e8c04..b762cfd7a 100644
--- a/requirements.txt
+++ b/requirements.txt
@@ -17,9 +17,7 @@ beautifulsoup4==4.12.3
     #   -r requirements.in
     #   webtest
 bleach[css]==6.1.0
-    # via
-    #   bleach
-    #   pypeline
+    # via pypeline
 certifi==2024.7.4
     # via requests
 cffi==1.16.0
@@ -125,6 +123,8 @@ packaging==24.0
     #   pytest-sugar
 paginate==0.5.6
     # via -r requirements.in
+passlib==1.7.4
+    # via -r requirements.in
 paste==3.9.0
     # via
     #   -r requirements.in


Reply via email to