[#8279] option to require password resets if listed on HIBP
diff --git a/Allura/allura/controllers/auth.py b/Allura/allura/controllers/auth.py
index 9273147..299664d 100644
--- a/Allura/allura/controllers/auth.py
+++ b/Allura/allura/controllers/auth.py
@@ -211,33 +211,20 @@
allow_non_primary_email_reset = asbool(config.get('auth.allow_non_primary_email_password_reset', True))
if not re.match(r"[^@]+@[^@]+\.[^@]+", email):
- flash('Enter email in correct format!','error')
+ flash('Enter email in correct format!', 'error')
redirect('/auth/forgotten_password')
if not allow_non_primary_email_reset:
message = 'If the given email address is on record, '\
'a password reset email has been sent to the account\'s primary email address.'
email_record = M.EmailAddress.get(email=provider.get_primary_email_address(user_record=user_record),
- confirmed=True)
+ confirmed=True)
else:
message = 'A password reset email has been sent, if the given email address is on record in our system.'
email_record = M.EmailAddress.get(email=email, confirmed=True)
if user_record and email_record and email_record.confirmed:
- hash = h.nonce(42)
- user_record.set_tool_data('AuthPasswordReset',
- hash=hash,
- hash_expiry=datetime.utcnow() +
- timedelta(seconds=int(config.get('auth.recovery_hash_expiry_period', 600))))
-
- log.info('Sending password recovery link to %s', email_record.email)
- subject = '%s Password recovery' % config['site_name']
- text = g.jinja2_env.get_template('allura:templates/mail/forgot_password.txt').render(dict(
- user=user_record,
- config=config,
- hash=hash,
- ))
- send_system_mail_to_user(email_record.email, subject, text)
+ user_record.send_password_reset_email(email_record.email)
h.auditlog_user('Password recovery link sent to: %s', email, user=user_record)
flash(message)
redirect('/')
@@ -522,9 +509,10 @@
session.pop('pwd-expired', None)
session['username'] = session.get('expired-username')
session.pop('expired-username', None)
+ expired_reason = session.pop('expired-reason', None)
session.save()
- h.auditlog_user('Password reset (via expiration process)')
+ h.auditlog_user('Password reset ({})'.format(expired_reason))
if return_to and return_to != request.url:
redirect(return_to)
else:
diff --git a/Allura/allura/lib/plugin.py b/Allura/allura/lib/plugin.py
index 9156f79..c52453f 100644
--- a/Allura/allura/lib/plugin.py
+++ b/Allura/allura/lib/plugin.py
@@ -41,7 +41,7 @@
ldap = modlist = None
import pkg_resources
import tg
-from tg import config, request, redirect, response
+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
@@ -187,10 +187,20 @@
else:
self.session.pop('multifactor-username', None)
+ login_details = self.get_login_detail(self.request)
+
+ expire_reason = None
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'],
+ login_details)
+ if expire_reason:
self.session['pwd-expired'] = True
self.session['expired-username'] = user.username
- h.auditlog_user('Successful login; Password expired', user=user)
+ self.session['expired-reason'] = expire_reason
else:
self.session['username'] = user.username
h.auditlog_user('Successful login', user=user)
@@ -204,7 +214,7 @@
self.session['login_expires'] = True
self.session.save()
g.statsUpdater.addUserLogin(user)
- user.add_login_detail(self.get_login_detail(self.request))
+ user.add_login_detail(login_details)
user.track_login(self.request)
# set a non-secure cookie with same expiration as session,
# so an http request can know if there is a related session on https
@@ -213,6 +223,36 @@
secure=False, httponly=True)
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
+ try:
+ trusted = 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(u'Successful login with password in HIBP breach database, '
+ u'from trusted source (reason: {})'.format(trusted), user=user)
+ return 'hibp' # reason
+ 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',
+ user=user)
+ user.send_password_reset_email(subject_tmpl=u'Update your {site_name} password')
+ raise exc.HTTPBadRequest('To ensure account security, you must reset your password via email.'
+ '\n'
+ 'Please check your email to continue.')
+
def logout(self):
self.session.invalidate()
self.session.save()
@@ -416,6 +456,17 @@
ua=request.headers.get('User-Agent'),
)
+ def trusted_login_source(self, user, login_details):
+ # TODO: could also factor in User-Agent but hard to know what parts of the UA are meaningful to check here
+ for prev_login in user.previous_login_details:
+ if prev_login['ip'] == login_details['ip']:
+ return 'exact ip'
+ if asbool(tg.config.get('auth.trust_ip_3_octets_match', False)) and \
+ utils.close_ipv4_addrs(prev_login['ip'], login_details['ip']):
+ return 'close ip'
+
+ return False
+
class LocalAuthenticationProvider(AuthenticationProvider):
diff --git a/Allura/allura/lib/utils.py b/Allura/allura/lib/utils.py
index 450bade..91faac1 100644
--- a/Allura/allura/lib/utils.py
+++ b/Allura/allura/lib/utils.py
@@ -846,3 +846,7 @@
then encoded as per normal.
"""
return urllib.urlencode([i for i in generate_smart_str(params)])
+
+
+def close_ipv4_addrs(ip1, ip2):
+ return ip1.split('.')[0:3] == ip2.split('.')[0:3]
diff --git a/Allura/allura/model/auth.py b/Allura/allura/model/auth.py
index 44ed66d..2a15a45 100644
--- a/Allura/allura/model/auth.py
+++ b/Allura/allura/model/auth.py
@@ -385,6 +385,24 @@
if login_detail:
self.add_login_detail(login_detail)
+ def send_password_reset_email(self, email_address=None, subject_tmpl=u'{site_name} Password recovery'):
+ if email_address is None:
+ email_address = self.get_pref('email_address')
+ hash = h.nonce(42)
+ self.set_tool_data('AuthPasswordReset',
+ hash=hash,
+ hash_expiry=datetime.utcnow() +
+ timedelta(seconds=int(config.get('auth.recovery_hash_expiry_period', 600))))
+
+ log.info('Sending password recovery link to %s', email_address)
+ subject = subject_tmpl.format(site_name=config['site_name'])
+ text = g.jinja2_env.get_template('allura:templates/mail/forgot_password.txt').render(dict(
+ user=self,
+ config=config,
+ hash=hash,
+ ))
+ allura.tasks.mail_tasks.send_system_mail_to_user(email_address, subject, text)
+
def can_send_user_message(self):
"""Return true if User is permitted to send a mesage to another user.
diff --git a/Allura/allura/nf/allura/css/site_style.css b/Allura/allura/nf/allura/css/site_style.css
index 69d0b5f..040336d 100644
--- a/Allura/allura/nf/allura/css/site_style.css
+++ b/Allura/allura/nf/allura/css/site_style.css
@@ -682,7 +682,7 @@
-o-box-shadow: none;
box-shadow: none;
display: block;
- cursor: default;
+ cursor: auto;
margin-bottom: 1em;
padding: 0;
}
@@ -3109,8 +3109,9 @@
font-style: italic;
}
-div.message.scm-learn-basics, div.message.scm-ssh-key, div.message.scm-empty-repo {
- cursor: default;
+/* .message default styles are hidden and controlled by jquery.notify.js. These are to be displayed as regular blocks */
+div.message.show-message, div.message.scm-learn-basics, div.message.scm-ssh-key, div.message.scm-empty-repo {
+ cursor: auto;
display: block;
width: auto;
margin: 0 1em 1em;
diff --git a/Allura/allura/templates/mail/forgot_password.txt b/Allura/allura/templates/mail/forgot_password.txt
index 5f5f71a..0e23312 100644
--- a/Allura/allura/templates/mail/forgot_password.txt
+++ b/Allura/allura/templates/mail/forgot_password.txt
@@ -19,6 +19,6 @@
Your username is {{ user.username }}
-To reset your password on {{ config['site_name'] }}, please visit the following URL:
+To update your password on {{ config['site_name'] }}, please visit the following URL:
{{ config['base_url'] }}/auth/forgotten_password/{{hash}}
diff --git a/Allura/allura/templates/pwd_expired.html b/Allura/allura/templates/pwd_expired.html
index 2af886d..476bc89 100644
--- a/Allura/allura/templates/pwd_expired.html
+++ b/Allura/allura/templates/pwd_expired.html
@@ -20,13 +20,30 @@
{% set hide_left_bar = True %}
{% extends g.theme.master %}
-{% block title %}{{c.user.username}} / Update expired password{% endblock %}
+{% block title %}{{c.user.username}} / Update password{% endblock %}
-{% block header %}Update expired password for {{c.user.username}}{% endblock %}
+{% block header %}Update password for {{c.user.username}}{% endblock %}
{% block content %}
- <div class='grid-23'>
- <p>Your {{config['site_name']}} password has expired - You must now choose a new password before logging into the site</p>
+ <div class='grid-15'>
+ <p>Your {{config['site_name']}} password
+ {% if session.get('expired-reason') == 'hibp' %}
+ must be updated to be more secure.
+ {% else %}
+ has expired.
+ {% endif %}
+ You must now choose a new password before logging into the site.</p>
+ </div>
+ <div class="grid-23">
{{ c.form.display(action='/auth/pwd_expired_change', value=dict(return_to=return_to)) }}
</div>
+ {% if h.asbool(config.get('auth.multifactor.totp')) %}
+ <div class='grid-8' style="position: absolute">
+ <div class="message info show-message">
+ Did you know? You can also enable multifactor authentication on your account.
+ <br><br>
+ After changing your password, go to your account settings to set it up.
+ </div>
+ </div>
+ {% endif %}
{% endblock content %}
diff --git a/Allura/allura/templates/widgets/forge_form.html b/Allura/allura/templates/widgets/forge_form.html
index c62404e..1ff6702 100644
--- a/Allura/allura/templates/widgets/forge_form.html
+++ b/Allura/allura/templates/widgets/forge_form.html
@@ -26,13 +26,13 @@
{% set extra_width = 4 %}
{% endif %}
{% if errors and not errors.iteritems and show_errors %}
- <div class="grid-{{19 + extra_width}}"><span {{widget.j2_attrs({'class':error_class})}}>{{errors}}</span></div>
+ <div class="grid-{{19 + extra_width}}"><span {{widget.j2_attrs({'class':error_class})}}>{{errors|nl2br}}</span></div>
{% endif %}
{% for field in widget.fields %}
{% set ctx=widget.context_for(field) %}
{% if field.field_type != 'hidden' %}
{% if ctx.errors and field.show_errors -%}
- <div class="grid-{{19 + extra_width}}"><span {{widget.j2_attrs({'class':error_class})}}>{{ctx.errors}}</span></div>
+ <div class="grid-{{19 + extra_width}}"><span {{widget.j2_attrs({'class':error_class})}}>{{ctx.errors|nl2br}}</span></div>
{%- endif %}
{% if field.show_label and field.label %}
<label for="{{ctx.id}}" class="grid-4">{{field.label}}:</label>
diff --git a/Allura/allura/templates_responsive/widgets/forge_form.html b/Allura/allura/templates_responsive/widgets/forge_form.html
index 29ef744..43b72e7 100644
--- a/Allura/allura/templates_responsive/widgets/forge_form.html
+++ b/Allura/allura/templates_responsive/widgets/forge_form.html
@@ -30,13 +30,13 @@
{% if target %}target="{{target}}"{% endif %}
action="{{action}}">
{% if errors and not errors.iteritems and show_errors %}
- <div class=""><span {{widget.j2_attrs({'class':error_class})}}>{{errors}}</span></div>
+ <div class=""><span {{widget.j2_attrs({'class':error_class})}}>{{errors|nl2br}}</span></div>
{% endif %}
{% for field in widget.fields %}
{% set ctx=widget.context_for(field) %}
{% if field.field_type != 'hidden' %}
{% if ctx.errors and field.show_errors -%}
- <div class="column small-12 large-6"><span {{widget.j2_attrs({'class':error_class})}}>{{ctx.errors}}</span></div>
+ <div class="column small-12 large-6"><span {{widget.j2_attrs({'class':error_class})}}>{{ctx.errors|nl2br}}</span></div>
{%- endif %}
{% if field.show_label and field.label %}
<label for="{{ctx.id}}" class="column small-12 large-3">{{field.label}}:</label>
diff --git a/Allura/allura/tests/functional/test_auth.py b/Allura/allura/tests/functional/test_auth.py
index a0419ff..09af206 100644
--- a/Allura/allura/tests/functional/test_auth.py
+++ b/Allura/allura/tests/functional/test_auth.py
@@ -128,6 +128,63 @@
assert_equal(wf['status'], 'error')
assert_equal(wf['message'], 'Spambot protection engaged')
+ @patch('allura.lib.plugin.AuthenticationProvider.hibp_password_check_enabled', Mock(return_value=True))
+ @patch('allura.tasks.mail_tasks.sendsimplemail')
+ def test_login_hibp_compromised_password_untrusted_client(self, sendsimplemail):
+ # first & only login by this user, so won't have any trusted previous logins
+ self.app.extra_environ = {'disable_auth_magic': 'True'}
+ r = self.app.get('/auth/')
+ f = r.forms[0]
+ encoded = self.app.antispam_field_names(f)
+ f[encoded['username']] = 'test-user'
+ f[encoded['password']] = 'foo'
+
+ with audits('Attempted login from untrusted location with password in HIBP breach database', user=True):
+ r = f.submit(status=200)
+
+ r.mustcontain('reset your password via email.')
+ r.mustcontain('reset your password via email.<br>\nPlease check your email')
+
+ args, kwargs = sendsimplemail.post.call_args
+ assert_equal(sendsimplemail.post.call_count, 1)
+ assert_equal(kwargs['subject'], u'Update your %s password' % config['site_name'])
+ assert_in('/auth/forgotten_password/', kwargs['text'])
+
+ @patch('allura.tasks.mail_tasks.sendsimplemail')
+ def test_login_hibp_compromised_password_trusted_client(self, sendsimplemail):
+ self.app.extra_environ = {'disable_auth_magic': 'True'}
+
+ # regular login first, so IP address will be recorded and then trusted
+ r = self.app.get('/auth/')
+ f = r.forms[0]
+ encoded = self.app.antispam_field_names(f)
+ f[encoded['username']] = 'test-user'
+ f[encoded['password']] = 'foo'
+ with audits('Successful login', user=True):
+ f.submit(status=302)
+ self.app.get('/auth/logout')
+
+ # this login will get caught by HIBP check, but trusted due to IP address being same
+ with patch('allura.lib.plugin.AuthenticationProvider.hibp_password_check_enabled', Mock(return_value=True)):
+ r = self.app.get('/auth/')
+ f = r.forms[0]
+ encoded = self.app.antispam_field_names(f)
+ f[encoded['username']] = 'test-user'
+ f[encoded['password']] = 'foo'
+
+ with audits(r'Successful login with password in HIBP breach database, from trusted source '
+ r'\(reason: exact ip\)', user=True):
+ r = f.submit(status=302)
+
+ assert r.session.get('pwd-expired')
+ assert_equal(r.session.get('expired-reason'), 'hibp')
+ assert_equal(r.location, 'http://localhost/auth/pwd_expired')
+
+ r = r.follow()
+ r.mustcontain('must be updated to be more secure')
+
+ # changing password covered in TestPasswordExpire
+
def test_logout(self):
self.app.extra_environ = {'disable_auth_magic': 'True'}
nav_pattern = ('nav', {'class': 'nav-main'})
@@ -1488,7 +1545,7 @@
# confirm email sent
text = '''Your username is test-admin
-To reset your password on %s, please visit the following URL:
+To update your password on %s, please visit the following URL:
%s/auth/forgotten_password/%s''' % (config['site_name'], config['base_url'], hash)
sendmail.post.assert_called_once_with(
@@ -1578,7 +1635,7 @@
@patch('allura.lib.plugin.AuthenticationProvider.hibp_password_check_enabled', Mock(return_value=True))
@patch('allura.tasks.mail_tasks.sendsimplemail')
@patch('allura.lib.helpers.gen_message_id')
- def test_hibp_check(self, gen_message_id, sendmail):
+ def test_pwd_reset_hibp_check(self, gen_message_id, sendmail):
self.app.get('/').follow() # establish session
user = M.User.query.get(username='test-admin')
email = M.EmailAddress.find({'claimed_by_user_id': user._id}).first()
@@ -2533,6 +2590,40 @@
# confirm code used up
assert_not_in(recovery_code, RecoveryCodeService().get().get_codes(user))
+ @patch('allura.lib.plugin.AuthenticationProvider.hibp_password_check_enabled', Mock(return_value=True))
+ def test_login_totp_with_hibp(self):
+ # this is essentially the same as regular TOTP test, just making sure that HIBP doesn't get in the way
+ # or cause any problems. It shouldn't even run since a password isn't present when the final login happens
+
+ self._init_totp()
+
+ # so test-admin isn't automatically logged in for all requests
+ self.app.extra_environ = {'disable_auth_magic': 'True'}
+
+ # regular login
+ r = self.app.get('/auth/?return_to=/p/foo')
+ encoded = self.app.antispam_field_names(r.form)
+ r.form[encoded['username']] = 'test-admin'
+ r.form[encoded['password']] = 'foo'
+ with audits('Multifactor login - password ok, code not entered yet', user=True):
+ r = r.form.submit()
+
+ # check results
+ assert r.location.endswith('/auth/multifactor?return_to=%2Fp%2Ffoo'), r
+ r = r.follow()
+ assert not r.session.get('username')
+
+ # use a valid code
+ totp = TotpService().Totp(self.sample_key)
+ code = totp.generate(time_time())
+ r.form['code'] = code
+ with audits('Successful login', user=True):
+ r = r.form.submit()
+
+ # confirm login and final page
+ assert_equal(r.session['username'], 'test-admin')
+ assert r.location.endswith('/p/foo'), r
+
def test_view_key(self):
self._init_totp()
diff --git a/Allura/allura/tests/functional/test_site_admin.py b/Allura/allura/tests/functional/test_site_admin.py
index cb61e58..0831926 100644
--- a/Allura/allura/tests/functional/test_site_admin.py
+++ b/Allura/allura/tests/functional/test_site_admin.py
@@ -705,7 +705,7 @@
hash = user.get_tool_data('AuthPasswordReset', 'hash')
text = '''Your username is test-user
-To reset your password on %s, please visit the following URL:
+To update your password on %s, please visit the following URL:
%s/auth/forgotten_password/%s''' % (config['site_name'], config['base_url'], hash)
sendmail.post.assert_called_once_with(
diff --git a/Allura/allura/tests/test_utils.py b/Allura/allura/tests/test_utils.py
index 5f642a7..199b4e5 100644
--- a/Allura/allura/tests/test_utils.py
+++ b/Allura/allura/tests/test_utils.py
@@ -398,3 +398,9 @@
assert not utils.is_nofollow_url('http://foo.com/')
assert not utils.is_nofollow_url('http://bar.io/')
assert utils.is_nofollow_url('http://bar.iot/')
+
+
+def test_close_ipv4_addrs():
+ assert utils.close_ipv4_addrs('1.2.3.4', '1.2.3.4')
+ assert utils.close_ipv4_addrs('1.2.3.4', '1.2.3.255')
+ assert not utils.close_ipv4_addrs('1.2.3.4', '1.2.4.4')
\ No newline at end of file
diff --git a/Allura/development.ini b/Allura/development.ini
index 640ab73..ed2cf50 100644
--- a/Allura/development.ini
+++ b/Allura/development.ini
@@ -227,6 +227,13 @@
; that are known to be compromised
auth.hibp_password_check = false
+; if auth.hibp_password_check is true and this is also set to true, then a password reset will be forced
+; either via the current session if the login ip is "trusted", or via a password reset email if not trusted
+auth.hibp_failure_force_pwd_change = true
+; if auth.hibp_password_check and auth.hibp_failure_force_pwd_change are true, then this is used to determine if a
+; login can be trusted to reset their own ip address. If set to false, then only the exact same IP can reset a
+; HIBP-listed password without going through email
+auth.auth.trust_ip_3_octets_match = true
user_prefs_storage.method = local
; user_prefs_storage.method = ldap
