[PATCH PoC] secure password reset implementation

Andrew Shadura andrew at shadura.me
Sat May 16 20:07:40 EDT 2015


# HG changeset patch
# User Andrew Shadura <andrew at shadura.me>
# Date 1431821238 -7200
#      Sun May 17 02:07:18 2015 +0200
# Node ID 8d43a8174c960779437c2d8de7a0906a8cd14128
# Parent  cb911e90e205bdb18fc2e2bd66549ea388d00413
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 idea behind is to generate a 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 the 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 not
implemented), 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
@@ -43,7 +43,8 @@ from kallithea.lib.auth_modules import i
 from kallithea.lib.base import BaseController, render
 from kallithea.lib.exceptions import UserCreationError
 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
 
@@ -241,12 +242,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,
@@ -258,17 +259,49 @@ 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(
+                username = request.GET.get('username'),
+                timestamp = request.GET.get('timestamp'),
+                token = request.GET.get('token')
+            )
+            if c.data['token']:
+                try:
+                    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'))
+                except Exception as e:
+                    log.error(e)
+                    return redirect(url('reset_password'))
+            else:
+                return render('/password_reset_confirmation.html')
+        elif request.POST:
+            password_reset_confirmation_form = PasswordResetConfirmationForm()()
+            c.data = dict(
+                username = request.POST.get('username'),
+                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/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,7 +27,11 @@ Original author and date, and relevant c
 
 
 import logging
+import hashlib
+import time
 import traceback
+from pylons import config
+from pylons import session
 from pylons.i18n.translation import _
 
 from sqlalchemy.exc import DatabaseError
@@ -296,10 +300,19 @@ 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 = hashlib.sha1('%s%s%s%s%s' % (user.user_id,
+                                               timestamp,
+                                               user.password,
+                                               session.id,
+                                               config.get('app_instance_uuid'))
+            ).hexdigest()
             link = h.canonical_url('reset_password_confirmation',
-                                   key=user.api_key)
+                                   username=user.username,
+                                   timestamp=timestamp,
+                                   token=token)
             reg_type = EmailNotificationModel.TYPE_PASSWORD_RESET
             body = EmailNotificationModel().get_email_tmpl(
                 reg_type, 'txt',
@@ -316,27 +329,45 @@ class UserModel(BaseModel):
         else:
             log.debug("password reset email %s not found" % user_email)
 
-        return True
+        return h.canonical_url('reset_password_confirmation',
+                               username=user.username,
+                               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_username(data['username'])
+        if not user:
+            log.debug("user %s not found" % data['username'])
+            return False
+
+        token = hashlib.sha1('%s%s%s%s%s' % (user.user_id,
+                                           data['timestamp'],
+                                           user.password,
+                                           session.id,
+                                           config.get('app_instance_uuid'))
+        ).hexdigest()
+        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_username(data['username'])
+        if not user:
+            log.debug("user %s not found" % data['username'])
+
+        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 has been changed'))
+        log.info('send password reset mail to %s' % user.email)
 
         return True
 
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['username']:
+        <p>${_('You are about to reset password for the user %s') % c.data['username']}</p>
+        ${h.form(h.canonical_url('reset_password_confirmation'), method='post' if c.data['token'] else 'get')}
+        <div style="display: none;">
+            ${h.hidden('username',value=c.data['username'])}
+            %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