[PATCH] secure password reset implementation
Andrew Shadura
andrew at shadura.me
Sun Jul 19 13:35:55 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 98cb64feddfb89f106f66763462061fd2ca3f412
# Parent f103b1a2383bc4fba5d28f9732ba832025e3bf00
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 proposed 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 (note: email template needs to be improved).
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.
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("data = %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("data = %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,9 @@ def generate_api_key():
return binascii.hexlify(os.urandom(20))
+def hash_values(*args):
+ 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
@@ -205,6 +205,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,16 @@ Original author and date, and relevant c
import logging
+import time
import traceback
+from pylons import config
+from pylons import session
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, \
@@ -280,6 +283,13 @@ 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):
+ 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
@@ -287,18 +297,24 @@ class UserModel(BaseModel):
user_email = data['email']
user = User.get_by_email(user_email)
+ timestamp = int(time.time())
if user:
log.debug('password reset user found %s' % user)
+ token = self.reset_password_token(user, timestamp)
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],
@@ -307,27 +323,52 @@ class UserModel(BaseModel):
else:
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
+ 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'])
+ 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
from kallithea.lib import auth
- user_email = data['email']
- user = User.get_by_email(user_email)
- new_passwd = auth.PasswordGenerator().gen_password(
- 8, auth.PasswordGenerator.ALPHABETS_BIG_SMALL)
- if user:
- user.password = auth.get_crypt_password(new_passwd)
+ user = User.get_by_email(data['email'])
+ if not user:
+ log.debug("user with email %s not found" % data['email'])
+
+ if data['password']:
+ user.password = auth.get_crypt_password(data['password'])
Session().add(user)
Session().commit()
- log.info('change password for %s' % user_email)
- if new_passwd is None:
- raise Exception('unable to generate new password')
+ log.info('change password for %s' % user.username)
- 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)
+ run_task(tasks.send_email, [user.email],
+ _('Password change notification'),
+ _('Your Kallithea password has been changed'))
+ log.info('send password reset 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 reset the 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 reset the 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 reset 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>
More information about the kallithea-general
mailing list