From 5936654b69d7a7705c1b7be34d61d9f0b6e8b993 Mon Sep 17 00:00:00 2001 From: Dave Brondsema Date: Mon, 23 Dec 2024 13:37:39 -0500 Subject: [PATCH] [#8572] option to force pwd changes for all, similar to logic after a HIBP hit --- Allura/allura/lib/plugin.py | 64 ++++++++++++------- .../allura/templates/forgotten_password.html | 4 +- Allura/allura/tests/functional/test_auth.py | 2 +- Allura/development.ini | 6 +- 4 files changed, 49 insertions(+), 27 deletions(-) diff --git a/Allura/allura/lib/plugin.py b/Allura/allura/lib/plugin.py index 6275c20bc0..6f48524cbd 100644 --- a/Allura/allura/lib/plugin.py +++ b/Allura/allura/lib/plugin.py @@ -181,13 +181,13 @@ def _login(self): ''' raise NotImplementedError('_login') - def after_login(self, user, request): + def after_login(self, user: M.User, request): ''' This is a hook so that custom AuthenticationProviders can do things after a successful login. ''' pass - def login(self, user=None, multifactor_success=False): + def login(self, user: M.User = None, multifactor_success: bool = False) -> M.User | None: from allura import model as M if user is None: try: @@ -210,9 +210,8 @@ def login(self, user=None, multifactor_success=False): if self.is_password_expired(user): h.auditlog_user('Successful login; Password expired', user=user) expire_reason = 'via expiration process' - if not expire_reason and 'password' in self.request.params: - # password not present with multifactor token; or if login directly after registering is enabled - expire_reason = self.login_check_password_change_needed(user, self.request.params['password'], + if not expire_reason: + expire_reason = self.login_check_password_change_needed(user, self.request.params.get('password'), login_details) if expire_reason: self.session['pwd-expired'] = True @@ -234,30 +233,49 @@ def login(self, user=None, multifactor_success=False): user.track_login(self.request) return user - def login_check_password_change_needed(self, user, password, login_details): - if not self.hibp_password_check_enabled() \ - or not asbool(tg.config.get('auth.hibp_failure_force_pwd_change', False)): - return - - try: - security.HIBPClient.check_breached_password(password) - except security.HIBPClientError as ex: - log.error("Error invoking HIBP API", exc_info=ex) - except security.HIBPCompromisedCredentials: - trusted = False + def login_check_password_change_needed(self, user: M.User, password: str | None, login_details: M.UserLoginDetails) -> str | None: + reason = reason_code = None + + # check setting to force pwd changes after date + before = asint(config.get('auth.force_pwd_change_after', 0)) + if before and self.get_last_password_updated(user) < datetime.utcfromtimestamp(before): + reason = 'requiring a password change' + reason_code = 'force_pwd_change' + + # check HIBP + if ( + self.hibp_password_check_enabled() + and asbool(tg.config.get('auth.hibp_failure_force_pwd_change', False)) + and password # not present with multifactor token; or if login directly after registering is enabled + ): + try: + security.HIBPClient.check_breached_password(password) + except security.HIBPClientError as ex: + log.error("Error invoking HIBP API", exc_info=ex) + except security.HIBPCompromisedCredentials: + reason = 'password in HIBP breach database' + reason_code = 'hibp' + + # do it + if reason: + + # check if we can trust current login + trusted: str | False = False + if user.get_pref('multifactor'): + trusted = 'multifactor success' try: - trusted = self.trusted_login_source(user, login_details) + trusted = trusted or self.trusted_login_source(user, login_details) except Exception: log.exception('Error checking if login is trusted: %s %s', user.username, login_details) if trusted: # current user must change password - h.auditlog_user('Successful login with password in HIBP breach database, ' - 'from trusted source (reason: {})'.format(trusted), user=user) - return 'hibp' # reason + h.auditlog_user(f'Successful login but {reason}, ' + f'from trusted source (reason: {trusted})', user=user) + return reason_code else: # current user may not continue, must reset password via email - h.auditlog_user('Attempted login from untrusted location with password in HIBP breach database', + h.auditlog_user(f'Attempted login from untrusted location with {reason}', user=user) user.send_password_reset_email(subject_tmpl='Update your {site_name} password') raise exc.HTTPBadRequest('To ensure account security, you must reset your password via email.' @@ -428,7 +446,7 @@ def user_details(self, user): ''' return {} - def is_password_expired(self, user): + def is_password_expired(self, user: M.User) -> bool: days = asint(config.get('auth.pwdexpire.days', 0)) before = asint(config.get('auth.pwdexpire.before', 0)) now = datetime.utcnow() @@ -499,7 +517,7 @@ def get_login_detail(self, request, user): ua=request.headers.get('User-Agent'), ) - def trusted_login_source(self, user, login_details): + def trusted_login_source(self, user, login_details) -> str | False: # TODO: could also factor in User-Agent but hard to know what parts of the UA are meaningful to check here from allura import model as M for prev_login in M.UserLoginDetails.query.find({'user_id': user._id}): diff --git a/Allura/allura/templates/forgotten_password.html b/Allura/allura/templates/forgotten_password.html index a30eb6d21d..6b87035139 100644 --- a/Allura/allura/templates/forgotten_password.html +++ b/Allura/allura/templates/forgotten_password.html @@ -19,9 +19,9 @@ {% set hide_left_bar = True %} {% extends g.theme.master %} -{% block title %}Forgotten password recovery{% endblock %} +{% block title %}Password Reset{% endblock %} -{% block header %}Forgotten password recovery{% endblock %} +{% block header %}Password Reset{% endblock %} {% block content %}
diff --git a/Allura/allura/tests/functional/test_auth.py b/Allura/allura/tests/functional/test_auth.py index 4d01808110..7309495c9c 100644 --- a/Allura/allura/tests/functional/test_auth.py +++ b/Allura/allura/tests/functional/test_auth.py @@ -178,7 +178,7 @@ def test_login_hibp_compromised_password_trusted_client(self, sendsimplemail): f[encoded['username']] = 'test-user' f[encoded['password']] = 'foo' - with audits(r'Successful login with password in HIBP breach database, from trusted source ' + with audits(r'Successful login but password in HIBP breach database, from trusted source ' r'\(reason: exact ip\)', user=True): r = f.submit(status=302) diff --git a/Allura/development.ini b/Allura/development.ini index 9d64bd9106..3336afe833 100644 --- a/Allura/development.ini +++ b/Allura/development.ini @@ -178,8 +178,12 @@ auth.password.algorithm.old = allura_sha256 ; password expiration options (disabled if neither is set) ;auth.pwdexpire.days = 1 -; unix timestamp: +; show a password change form after login, if last pwd change was before this unix timestamp ;auth.pwdexpire.before = 1401949912 +; require password change, if last pwd change was before this unix timestamp +; password can only be changed by the logging-in user if they are "trusted" (e.g. known IP) +; otherwise they must change via email +;auth.force_pwd_change_after = 1734978358 ; if using LDAP, also run `pip install python-ldap` in your Allura environment