[PATCH v2] auth: secure password reset implementation
Andrew Shadura
andrew at shadura.me
Sat Aug 8 15:25:27 UTC 2015
# HG changeset patch
# User Andrew Shadura <andrew at shadura.me>
# Date 1431821238 -7200
# Sun May 17 02:07:18 2015 +0200
# Node ID c4ad7e6deed43b0f725b60b37ae1d2161eead200
# Parent e84c2738fbd84569492c56e5a9361ee94af5d420
auth: secure password reset implementation
This is a better implementation of password reset function, which
doesn't involve sending a new password to the user's email address
in clear text, and at the same time doesn't require state storing.
The old implementation generated a new password and sent it
in clear text to whatever email assigned to the user currently,
invalidating the old password the very moment a new one is requested.
Apart from potential insecurity, this makes it possible for anyone
to disrupt users' workflow by repeatedly resetting their passwords.
The idea behind this implementation is to generate
an authentication token which is dependent on the user state
at the time before the password change takes place, so the token
is one-time and can't be reused, and also to bind the token to
the session.
The token is calculated as SHA1 hash of the following:
* user's identifier (number, not a name)
* timestamp
* hashed user's password
* session identifier
* per-application secret
We use numeric user's identifier, as it's fixed and doesn't change,
so renaming users doesn't affect the mechanism. Timestamp is added
to make it possible to limit the token's validness (currently hard
coded to 24h), and we don't want users to be able to fake that field
easily. Hashed user's password is needed to prevent using the token
again once the password has been changed. Session identifier is
an additional security measure to ensure someone else stealing the
token can't use it. Finally, per-application secret is just another
way to make it harder for an attacker to guess all values in an
attempt to generate a valid token.
When the token is generated, an anonymous user is directed to a
confirmation page where the timestamp and the usernames are already
preloaded, so the user needs to specify the token. User can either
click the link in the email if it's really them reading it, or to type
the token manually.
Using the right token in the same session as it was requested directs
the user to a password change form, where the user is supposed to
specify a new password (twice, of course). Upon completing the form
(which is POSTed) the password change happens and a notification
mail is sent.
The test is updated to test the basic functionality with a bad and
a good token, but it doesn't (yet) cover all code paths.
diff --git a/kallithea/controllers/login.py b/kallithea/controllers/login.py
--- a/kallithea/controllers/login.py
+++ b/kallithea/controllers/login.py
@@ -42,7 +42,8 @@ from kallithea.lib.base import BaseContr
from kallithea.lib.exceptions import UserCreationError
from kallithea.lib.utils2 import safe_str
from kallithea.model.db import User, Setting
-from kallithea.model.forms import LoginForm, RegisterForm, PasswordResetForm
+from kallithea.model.forms import \
+ LoginForm, RegisterForm, PasswordResetForm, PasswordResetConfirmationForm
from kallithea.model.user import UserModel
from kallithea.model.meta import Session
@@ -197,12 +198,12 @@ class LoginController(BaseController):
error_dict = {'recaptcha_field': _msg}
raise formencode.Invalid(_msg, _value, None,
error_dict=error_dict)
- UserModel().reset_password_link(form_result)
+ redirect_link = UserModel().reset_password_link(form_result)
h.flash(_('Your password reset link was sent'),
category='success')
- return redirect(url('login_home'))
+ return redirect(redirect_link)
- except formencode.Invalid, errors:
+ except formencode.Invalid as errors:
return htmlfill.render(
render('/password_reset.html'),
defaults=errors.value,
@@ -214,17 +215,45 @@ class LoginController(BaseController):
return render('/password_reset.html')
def password_reset_confirmation(self):
- if request.GET and request.GET.get('key'):
+ if request.GET:
+ c.data = dict(
+ email = request.GET.get('email'),
+ timestamp = request.GET.get('timestamp'),
+ token = request.GET.get('token'),
+ )
+ if c.data['token']:
+ log.debug("password reset confirmation (GET): %s", c.data)
+ if UserModel().reset_password_confirm(c.data):
+ return render('/password_reset_confirmation.html')
+ else:
+ h.flash(_('Invalid password reset token'),
+ category='error')
+ return redirect(url('reset_password'))
+ else:
+ return render('/password_reset_confirmation.html')
+ elif request.POST:
+ password_reset_confirmation_form = PasswordResetConfirmationForm()()
+ c.data = dict(
+ email = request.POST.get('email'),
+ password = request.POST.get('password'),
+ password_confirm = request.POST.get('password_confirm'),
+ timestamp = request.POST.get('timestamp'),
+ token = request.POST.get('token'),
+ )
try:
- user = User.get_by_api_key(request.GET.get('key'))
- data = dict(email=user.email)
- UserModel().reset_password(data)
- h.flash(_('Your password reset was successful, '
- 'new password has been sent to your email'),
- category='success')
- except Exception, e:
- log.error(e)
- return redirect(url('reset_password'))
+ log.debug("password reset confirmation (POST): %s", c.data)
+ form_result = password_reset_confirmation_form.to_python(dict(request.POST))
+ UserModel().reset_password(c.data)
+ h.flash(_('Successfully updated password'),
+ category='success')
+ except formencode.Invalid as errors:
+ return htmlfill.render(
+ render('/password_reset_confirmation.html'),
+ defaults=errors.value,
+ errors=errors.error_dict or {},
+ prefix_error=False,
+ encoding="UTF-8",
+ force_defaults=False)
return redirect(url('login_home'))
diff --git a/kallithea/lib/utils2.py b/kallithea/lib/utils2.py
--- a/kallithea/lib/utils2.py
+++ b/kallithea/lib/utils2.py
@@ -34,6 +34,7 @@ import uuid
import datetime
import urllib
import binascii
+import hashlib
import webob
import urlobject
@@ -170,6 +171,14 @@ def generate_api_key():
return binascii.hexlify(os.urandom(20))
+def hash_values(*args):
+ """
+ Calculate SHA1 for the string representations of the arguments,
+ concatenated and joined using NUL characters. Returns a hexadecimal
+ representation of the hash.
+ """
+ return hashlib.sha1('\0'.join(str(arg) for arg in args)).hexdigest()
+
def safe_int(val, default=None):
"""
Returns int() of val if val is not convertable to int use default
diff --git a/kallithea/model/forms.py b/kallithea/model/forms.py
--- a/kallithea/model/forms.py
+++ b/kallithea/model/forms.py
@@ -212,6 +212,17 @@ def PasswordResetForm():
email = v.Email(not_empty=True)
return _PasswordResetForm
+def PasswordResetConfirmationForm():
+ class _PasswordResetConfirmationForm(formencode.Schema):
+ allow_extra_fields = True
+ filter_extra_fields = True
+
+ password = All(v.ValidPassword(), v.UnicodeString(strip=False, min=6))
+ password_confirm = All(v.ValidPassword(), v.UnicodeString(strip=False, min=6))
+
+ chained_validators = [v.ValidPasswordsMatch('password',
+ 'password_confirm')]
+ return _PasswordResetConfirmationForm
def RepoForm(edit=False, old_data={}, supported_backends=BACKENDS.keys(),
repo_groups=[], landing_revs=[]):
diff --git a/kallithea/model/user.py b/kallithea/model/user.py
--- a/kallithea/model/user.py
+++ b/kallithea/model/user.py
@@ -27,13 +27,15 @@ Original author and date, and relevant c
import logging
+import time
import traceback
+from pylons import config
from pylons.i18n.translation import _
from sqlalchemy.exc import DatabaseError
from kallithea import EXTERN_TYPE_INTERNAL
-from kallithea.lib.utils2 import safe_unicode, generate_api_key, get_current_authuser
+from kallithea.lib.utils2 import safe_unicode, generate_api_key, get_current_authuser, hash_values
from kallithea.lib.caching_query import FromCache
from kallithea.model import BaseModel
from kallithea.model.db import User, UserToPerm, Notification, \
@@ -271,6 +273,33 @@ class UserModel(BaseModel):
from kallithea.lib.hooks import log_delete_user
log_delete_user(user.get_dict(), cur_user)
+ def reset_password_token(self, user, timestamp, session_id):
+ """
+ The token is calculated as SHA1 hash of the following:
+
+ * user's identifier (number, not a name)
+ * timestamp
+ * hashed user's password
+ * session identifier
+ * per-application secret
+
+ We use numeric user's identifier, as it's fixed and doesn't change,
+ so renaming users doesn't affect the mechanism. Timestamp is added
+ to make it possible to limit the token's validness (currently hard
+ coded to 24h), and we don't want users to be able to fake that field
+ easily. Hashed user's password is needed to prevent using the token
+ again once the password has been changed. Session identifier is
+ an additional security measure to ensure someone else stealing the
+ token can't use it. Finally, per-application secret is just another
+ way to make it harder for an attacker to guess all values in an
+ attempt to generate a valid token.
+ """
+ return hash_values(user.user_id,
+ timestamp,
+ user.password,
+ session_id,
+ config.get('app_instance_uuid'))
+
def reset_password_link(self, data):
from kallithea.lib.celerylib import tasks, run_task
from kallithea.model.notification import EmailNotificationModel
@@ -278,27 +307,65 @@ class UserModel(BaseModel):
user_email = data['email']
user = User.get_by_email(user_email)
+ timestamp = int(time.time())
if user is not None:
- log.debug('password reset user found %s' % user)
+ log.debug('password reset user %s found', user)
+ token = self.reset_password_token(user,
+ timestamp,
+ h.authentication_token)
link = h.canonical_url('reset_password_confirmation',
- key=user.api_key)
+ email=user_email,
+ timestamp=timestamp,
+ token=token)
reg_type = EmailNotificationModel.TYPE_PASSWORD_RESET
body = EmailNotificationModel().get_email_tmpl(
reg_type, 'txt',
user=user.short_contact,
+ reset_token=token,
reset_url=link)
html_body = EmailNotificationModel().get_email_tmpl(
reg_type, 'html',
user=user.short_contact,
+ reset_token=token,
reset_url=link)
log.debug('sending email')
run_task(tasks.send_email, [user_email],
_("Password reset link"), body, html_body)
- log.info('send new password mail to %s' % user_email)
+ log.info('send new password mail to %s', user_email)
else:
- log.debug("password reset email %s not found" % user_email)
+ log.debug("password reset email %s not found", user_email)
- return True
+ return h.canonical_url('reset_password_confirmation',
+ email=user_email,
+ timestamp=timestamp)
+
+ def reset_password_confirm(self, data):
+ from kallithea.lib.celerylib import tasks, run_task
+ from kallithea.lib import auth
+ import kallithea.lib.helpers as h
+ user = User.get_by_email(data['email'])
+ if not user:
+ log.debug("user with email %s not found", data['email'])
+ return False
+
+ now = int(time.time())
+ timestamp = int(data['timestamp'])
+
+ if (timestamp is None) or (timestamp > now):
+ log.debug('timestamp is None or from the future')
+ return False
+
+ # TODO for now, hardcode 24h, need to be a setting
+ if (now - timestamp) > 86400:
+ log.debug('token expired')
+ return False
+
+ token = self.reset_password_token(user,
+ data['timestamp'],
+ h.authentication_token)
+ log.debug('computed token: %s', token)
+ log.debug('received token: %s', data['token'])
+ return token == data['token']
def reset_password(self, data):
from kallithea.lib.celerylib import tasks, run_task
@@ -311,14 +378,14 @@ class UserModel(BaseModel):
user.password = auth.get_crypt_password(new_passwd)
Session().add(user)
Session().commit()
- log.info('change password for %s' % user_email)
+ log.info('change password for %s', user_email)
if new_passwd is None:
raise Exception('unable to generate new password')
run_task(tasks.send_email, [user_email],
_('Your new password'),
_('Your new Kallithea password:%s') % (new_passwd,))
- log.info('send new password mail to %s' % user_email)
+ log.info('send new password mail to %s', user_email)
return True
diff --git a/kallithea/templates/email_templates/password_reset.html b/kallithea/templates/email_templates/password_reset.html
--- a/kallithea/templates/email_templates/password_reset.html
+++ b/kallithea/templates/email_templates/password_reset.html
@@ -3,8 +3,10 @@
<h4>${_('Hello %s') % user}</h4>
-<p>${_('We received a request to create a new password for your account.')}</p>
-<p>${_('You can generate it by clicking following URL')}:</p>
+<p>${_('We have received a request to reset the password for your account.')}</p>
+<p>${_('To set a new password, click the following link')}:</p>
<p><a href="${reset_url}">${reset_url}</a></p>
-<p>${_("Please ignore this email if you did not request a new password .")}</p>
+<p>${_("Should you not be able to use the link above, please type the following code into the password reset form")}: <code>${reset_token}</code></p>
+
+<p>${_("If it weren't you who requested the password reset, just disregard this message.")}</p>
diff --git a/kallithea/templates/email_templates/password_reset.txt b/kallithea/templates/email_templates/password_reset.txt
--- a/kallithea/templates/email_templates/password_reset.txt
+++ b/kallithea/templates/email_templates/password_reset.txt
@@ -3,9 +3,11 @@
${_('Hello %s') % user|n,unicode}
-${_('We received a request to create a new password for your account.')|n,unicode}
-${_('You can generate it by clicking following URL')|n,unicode}:
+${_('We have received a request to reset the password for your account..')|n,unicode}
+${_('To set a new password, click the following link')|n,unicode}:
${reset_url|n,unicode}
-${_("Please ignore this email if you did not request a new password .")|n,unicode}
+${_("Should you not be able to use the link above, please type the following code into the password reset form")|n,unicode}: ${reset_token|n,unicode}
+
+${_("If it weren't you who requested the password reset, just disregard this message.")|n,unicode}
diff --git a/kallithea/templates/password_reset_confirmation.html b/kallithea/templates/password_reset_confirmation.html
--- a/kallithea/templates/password_reset_confirmation.html
+++ b/kallithea/templates/password_reset_confirmation.html
@@ -0,0 +1,73 @@
+## -*- coding: utf-8 -*-
+<%inherit file="base/root.html"/>
+
+<%block name="title">
+ ${_('Password Reset Confirmation')}
+</%block>
+
+<div id="register">
+ <%include file="/base/flash_msg.html"/>
+ <div class="title withlogo">
+ %if c.site_name:
+ <h5>${_('Reset Your Password to %s') % c.site_name}</h5>
+ %else:
+ <h5>${_('Reset Your Password')}</h5>
+ %endif
+ </div>
+ <div class="inner">
+ %if c.data['email']:
+ <p>${_('You are about to set a new password for the email address %s') % c.data['email']}</p>
+ ${h.form(h.canonical_url('reset_password_confirmation'), method='post' if c.data['token'] else 'get')}
+ <div style="display: none;">
+ ${h.hidden('email',value=c.data['email'])}
+ %if c.data['token']:
+ ${h.hidden('token',value=c.data['token'])}
+ %endif
+ %if c.data['timestamp']:
+ ${h.hidden('timestamp',value=c.data['timestamp'])}
+ %endif
+ </div>
+ %endif
+ <div class="form">
+ <!-- fields -->
+ <div class="fields">
+ %if c.data['token']:
+ <div class="field">
+ <div class="label">
+ <label for="password">${_('New Password')}:</label>
+ </div>
+ <div class="input">
+ ${h.password('password',class_='focus')}
+ </div>
+ </div>
+
+ <div class="field">
+ <div class="label">
+ <label for="password_confirm">${_('Confirm New Password')}:</label>
+ </div>
+ <div class="input">
+ ${h.password('password_confirm',class_='focus')}
+ </div>
+ </div>
+ %else:
+ <div class="field">
+ <div class="label">
+ <label for="token">${_('Code you received in the email')}:</label>
+ </div>
+ <div class="input">
+ ${h.text('token',class_='focus')}
+ </div>
+ </div>
+
+ %endif
+
+ <div class="buttons">
+ <div class="nohighlight">
+ ${h.submit('send',_('Confirm'),class_="btn")}
+ </div>
+ </div>
+ </div>
+ </div>
+ ${h.end_form()}
+ </div>
+ </div>
diff --git a/kallithea/tests/functional/test_login.py b/kallithea/tests/functional/test_login.py
--- a/kallithea/tests/functional/test_login.py
+++ b/kallithea/tests/functional/test_login.py
@@ -12,6 +12,7 @@ from kallithea.model.api_key import ApiK
from kallithea.model import validators
from kallithea.model.db import User, Notification
from kallithea.model.meta import Session
+from kallithea.model.user import UserModel
fixture = Fixture()
@@ -336,6 +337,7 @@ class TestLoginController(TestController
response.mustcontain('An email address must contain a single @')
def test_forgot_password(self):
+ import time
response = self.app.get(url(controller='login',
action='password_reset'))
self.assertEqual(response.status, '200 OK')
@@ -345,6 +347,7 @@ class TestLoginController(TestController
email = 'username at python-works.com'
name = 'passwd'
lastname = 'reset'
+ timestamp = int(time.time())
new = User()
new.username = username
@@ -364,27 +367,45 @@ class TestLoginController(TestController
response = response.follow()
- # BAD KEY
+ # BAD TOKEN
- key = "bad"
+ token = "bad"
response = self.app.get(url(controller='login',
action='password_reset_confirmation',
- key=key))
+ email=email,
+ timestamp=timestamp,
+ token=token))
self.assertEqual(response.status, '302 Found')
self.assertTrue(response.location.endswith(url('reset_password')))
+ response = response.follow()
+
# GOOD KEY
- key = User.get_by_username(username).api_key
+ token = UserModel().reset_password_token(User.get_by_username(username),
+ timestamp,
+ h.authentication_token)
+
response = self.app.get(url(controller='login',
action='password_reset_confirmation',
- key=key))
+ email=email,
+ timestamp=timestamp,
+ token=token))
+ self.assertEqual(response.status, '200 OK')
+ response.mustcontain("You are about to set a new password for the email address %s" % email)
+
+ response = self.app.post(url(controller='login',
+ action='password_reset_confirmation'),
+ {'email': email,
+ 'timestamp': timestamp,
+ 'password': "p at ssw0rd",
+ 'password_confirm': "p at ssw0rd",
+ 'token': token,
+ })
self.assertEqual(response.status, '302 Found')
- self.assertTrue(response.location.endswith(url('login_home')))
self.checkSessionFlash(response,
- ('Your password reset was successful, '
- 'new password has been sent to your email'))
+ ('Successfully updated password'))
response = response.follow()
More information about the kallithea-general
mailing list