Skip to content

Commit

Permalink
[#8572] option to force pwd changes for all, similar to logic after a…
Browse files Browse the repository at this point in the history
… HIBP hit
  • Loading branch information
brondsem authored and Kenton Taylor committed Dec 23, 2024
1 parent 83408d1 commit 5936654
Show file tree
Hide file tree
Showing 4 changed files with 49 additions and 27 deletions.
64 changes: 41 additions & 23 deletions Allura/allura/lib/plugin.py
Original file line number Diff line number Diff line change
Expand Up @@ -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:
Expand All @@ -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
Expand All @@ -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.'
Expand Down Expand Up @@ -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()
Expand Down Expand Up @@ -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}):
Expand Down
4 changes: 2 additions & 2 deletions Allura/allura/templates/forgotten_password.html
Original file line number Diff line number Diff line change
Expand Up @@ -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 %}
<div class="grid-20">
Expand Down
2 changes: 1 addition & 1 deletion Allura/allura/tests/functional/test_auth.py
Original file line number Diff line number Diff line change
Expand Up @@ -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)

Expand Down
6 changes: 5 additions & 1 deletion Allura/development.ini
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down

0 comments on commit 5936654

Please sign in to comment.