You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@allura.apache.org by gc...@apache.org on 2024/08/02 19:39:10 UTC
(allura) 01/05: [#8566] support more password hashing algorithms
This is an automated email from the ASF dual-hosted git repository.
gcruz pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/allura.git
commit b8b1ac4397edb507b30de4041dd6862b0d846377
Author: Dave Brondsema <db...@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