kallithea Changeset - f629e9a0c376

Changeset - f629e9a0c376

Parent rev.

Child rev.

[Not reviewed]

default

0 8 0

Andrew Shadura - 11 years ago 2015-05-17 02:07:18
andrew@shadura.me

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 is stateless.

The old implementation generated a new password and sent it
in clear text to whatever email assigned to the user currently,
so that any user, possibly unauthenticated, could request a reset
for any username or email. Apart from potential insecurity, this
made 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 browser 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.

The original work from Andrew has been thorougly reviewed and heavily
modified by Søren Løvborg.

8 files changed with 277 insertions and 57 deletions:

kallithea/controllers/login.py

kallithea/model/forms.py

kallithea/model/user.py

kallithea/templates/email_templates/password_reset.html

kallithea/templates/email_templates/password_reset.txt

kallithea/templates/password_reset.html

kallithea/templates/password_reset_confirmation.html

kallithea/tests/functional/test_login.py

0 comments (0 inline, 0 general)

kallithea/controllers/login.py

➞

Show inline comments

@@ @@ -31,7 +31,7 @@ import formencode @@
 import urlparse
 from formencode import htmlfill
 from webob.exc import HTTPFound
+from webob.exc import HTTPFound, HTTPBadRequest
 from pylons.i18n.translation import _
 from pylons.controllers.util import redirect
 from pylons import request, session, tmpl_context as c, url
@@ @@ -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, PasswordResetRequestForm, PasswordResetConfirmationForm
 from kallithea.model.user import UserModel
 from kallithea.model.meta import Session
@@ @@ -182,7 +183,7 @@ class LoginController(BaseController): @@
         c.captcha_public_key = settings.get('captcha_public_key')
         if request.POST:
-            password_reset_form = PasswordResetForm()()
+            password_reset_form = PasswordResetRequestForm()()
             try:
                 form_result = password_reset_form.to_python(dict(request.POST))
                 if c.captcha_active:
@@ @@ -197,10 +198,10 @@ class LoginController(BaseController): @@
                         error_dict = {'recaptcha_field': _msg}
                         raise formencode.Invalid(_msg, _value, None,
                                                  error_dict=error_dict)
                 UserModel().reset_password_link(form_result)
                 h.flash(_('Your password reset link was sent'),
                 redirect_link = UserModel().send_reset_password_email(form_result)
                 h.flash(_('A password reset confirmation code has been sent'),
                             category='success')
-                return redirect(url('login_home'))
+                return redirect(redirect_link)
             except formencode.Invalid as errors:
                 return htmlfill.render(
@@ @@ -214,18 +215,45 @@ class LoginController(BaseController): @@
         return render('/password_reset.html')
     def password_reset_confirmation(self):
         if request.GET and request.GET.get('key'):
             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 as e:
                 log.error(e)
                 return redirect(url('reset_password'))
         # This controller handles both GET and POST requests, though we
         # only ever perform the actual password change on POST (since
         # GET requests are not allowed to have side effects, and do not
         # receive automatic CSRF protection).
         # The template needs the email address outside of the form.
         c.email = request.params.get('email')
         if not request.POST:
             return htmlfill.render(
                 render('/password_reset_confirmation.html'),
                 defaults=dict(request.params),
                 encoding='UTF-8')
         form = PasswordResetConfirmationForm()()
         try:
             form_result = form.to_python(dict(request.POST))
         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')
         if not UserModel().verify_reset_password_token(
             form_result['email'],
             form_result['timestamp'],
             form_result['token'],
         ):
             return htmlfill.render(
                 render('/password_reset_confirmation.html'),
                 defaults=form_result,
                 errors={'token': _('Invalid password reset token')},
                 prefix_error=False,
                 encoding='UTF-8')
         UserModel().reset_password(form_result['email'], form_result['password'])
         h.flash(_('Successfully updated password'), category='success')
         return redirect(url('login_home'))
     def logout(self):

kallithea/model/forms.py

➞

Show inline comments

@@ @@ -205,13 +205,27 @@ def RegisterForm(edit=False, old_data={} @@
     return _RegisterForm
 def PasswordResetForm():
     class _PasswordResetForm(formencode.Schema):
 def PasswordResetRequestForm():
     class _PasswordResetRequestForm(formencode.Schema):
         allow_extra_fields = True
         filter_extra_fields = True
         email = v.Email(not_empty=True)
     return _PasswordResetForm
     return _PasswordResetRequestForm
 def PasswordResetConfirmationForm():
     class _PasswordResetConfirmationForm(formencode.Schema):
         allow_extra_fields = True
         filter_extra_fields = True
         email = v.UnicodeString(strip=True, not_empty=True)
         timestamp = v.Number(strip=True, not_empty=True)
         token = v.UnicodeString(strip=True, not_empty=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=[]):

kallithea/model/user.py

➞

Show inline comments

@@ @@ -26,8 +26,12 @@ Original author and date, and relevant c @@
 """
 import hashlib
 import logging
 import time
 import traceback
 from pylons import config
 from pylons.i18n.translation import _
 from sqlalchemy.exc import DatabaseError
@@ @@ -47,6 +51,8 @@ log = logging.getLogger(__name__) @@
 class UserModel(BaseModel):
     password_reset_token_lifetime = 86400 # 24 hours
     cls = User
     def get(self, user_id, cache=False):
@@ @@ -271,25 +277,73 @@ class UserModel(BaseModel): @@
         from kallithea.lib.hooks import log_delete_user
         log_delete_user(user.get_dict(), cur_user)
     def reset_password_link(self, data):
     def get_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 hashlib.sha1('\0'.join([
             str(user.user_id),
             str(timestamp),
             user.password,
             session_id,
             config.get('app_instance_uuid'),
         ])).hexdigest()
     def send_reset_password_email(self, data):
         """
         Sends email with a password reset token and link to the password
         reset confirmation page with all information (including the token)
         pre-filled. Also returns URL of that page, only without the token,
         allowing users to copy-paste or manually enter the token from the
         email.
         """
         from kallithea.lib.celerylib import tasks, run_task
         from kallithea.model.notification import EmailNotificationModel
         import kallithea.lib.helpers as h
         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)
             link = h.canonical_url('reset_password_confirmation',
                                    key=user.api_key)
             log.debug('password reset user %s found', user)
             token = self.get_reset_password_token(user,
                                                   timestamp,
                                                   h.authentication_token())
             # URL must be fully qualified; but since the token is locked to
             # the current browser session, we must provide a URL with the
             # current scheme and hostname, rather than the canonical_url.
             link = h.url('reset_password_confirmation', qualified=True,
                          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],
@@ @@ -298,27 +352,52 @@ class UserModel(BaseModel): @@
         else:
             log.debug("password reset email %s not found", user_email)
         return True
         return h.url('reset_password_confirmation',
                      email=user_email,
                      timestamp=timestamp)
-    def reset_password(self, data):
+    def verify_reset_password_token(self, email, timestamp, token):
         from kallithea.lib.celerylib import tasks, run_task
         from kallithea.lib import auth
         user_email = data['email']
         import kallithea.lib.helpers as h
         user = User.get_by_email(email)
         if user is None:
             log.debug("user with email %s not found", email)
             return False
         token_age = int(time.time()) - int(timestamp)
         if token_age < 0:
             log.debug('timestamp is from the future')
             return False
         if token_age > UserModel.password_reset_token_lifetime:
             log.debug('password reset token expired')
             return False
         expected_token = self.get_reset_password_token(user,
                                                        timestamp,
                                                        h.authentication_token())
         log.debug('computed password reset token: %s', expected_token)
         log.debug('received password reset token: %s', token)
         return expected_token == token
     def reset_password(self, user_email, new_passwd):
         from kallithea.lib.celerylib import tasks, run_task
         from kallithea.lib import auth
         user = User.get_by_email(user_email)
         new_passwd = auth.PasswordGenerator().gen_password(
 , auth.PasswordGenerator.ALPHABETS_BIG_SMALL)
         if user is not None:
             user.password = auth.get_crypt_password(new_passwd)
             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')
+            raise Exception('unable to set 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)
                  _('Password reset notification'),
                  _('The password to your account %s has been changed using password reset form.') % (user.username,))
         log.info('send password reset mail to %s', user_email)
         return True

kallithea/templates/email_templates/password_reset.html

➞

Show inline comments

@@ @@ -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>

kallithea/templates/email_templates/password_reset.txt

➞

Show inline comments

@@ @@ -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}

kallithea/templates/password_reset.html

➞

Show inline comments

@@ @@ -44,7 +44,7 @@ @@
                 <div class="buttons">
                     <div class="nohighlight">
                       ${h.submit('send',_('Send Password Reset Email'),class_="btn")}
-                          <div class="activation_msg">${_('Password reset link will be sent to the email address matching your username.')}</div>
+                          <div class="activation_msg">${_('A password reset link will be sent to the specified email address if it is registered in the system.')}</div>
                     </div>
                 </div>
             </div>

kallithea/templates/password_reset_confirmation.html

➞

Show inline comments

 ## -*- coding: utf-8 -*-
 <%inherit file="base/root.html"/>
 <%block name="title">
     ${_('Reset Your Password')}
 </%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">
         ${h.form(h.url('reset_password_confirmation'), method='post')}
         <p>${_('You are about to set a new password for the email address %s.') % c.email}</p>
         <p>${_('Note that you must use the same browser session for this as the one used to request the password reset.')}</p>
         <div style="display: none;">
             ${h.hidden('email')}
             ${h.hidden('timestamp')}
         </div>
         <div class="form">
             <!-- fields -->
             <div class="fields">
                  <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>
                  <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>
                 <div class="buttons">
                     <div class="nohighlight">
                       ${h.submit('send',_('Confirm'),class_="btn")}
                     </div>
                 </div>
             </div>
         </div>
         ${h.end_form()}
     </div>
    </div>

kallithea/tests/functional/test_login.py

➞

Show inline comments

 # -*- coding: utf-8 -*-
 from __future__ import with_statement
 import re
 import time
 import mock
 from kallithea.tests import *
 from kallithea.tests.fixture import Fixture
 from kallithea.lib.utils2 import generate_api_key
@@ @@ -12,6 +14,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()
@@ @@ -326,6 +329,10 @@ class TestLoginController(TestController @@
         self.assertNotEqual(ret.api_key, None)
         self.assertEqual(ret.admin, False)
     #==========================================================================
     # PASSWORD RESET
     #==========================================================================
     def test_forgot_password_wrong_mail(self):
         bad_email = 'username%wrongmail.org'
         response = self.app.post(
@@ @@ -345,6 +352,7 @@ class TestLoginController(TestController @@
         email = 'username@python-works.com'
         name = 'passwd'
         lastname = 'reset'
         timestamp = int(time.time())
         new = User()
         new.username = username
@@ @@ -360,34 +368,58 @@ class TestLoginController(TestController @@
                                      action='password_reset'),
                                  {'email': email, })
-        self.checkSessionFlash(response, 'Your password reset link was sent')
+        self.checkSessionFlash(response, 'A password reset confirmation code has been sent')
         response = response.follow()
         # BAD KEY
         # BAD TOKEN
         token = "bad"
         key = "bad"
         response = self.app.post(url(controller='login',
                                      action='password_reset_confirmation'),
                                  {'email': email,
                                   'timestamp': timestamp,
                                   'password': "p@ssw0rd",
                                   'password_confirm': "p@ssw0rd",
                                   'token': token,
                                  })
         self.assertEqual(response.status, '200 OK')
         response.mustcontain('Invalid password reset token')
         # GOOD TOKEN
         # TODO: The token should ideally be taken from the mail sent
         # above, instead of being recalculated.
         token = UserModel().get_reset_password_token(
             User.get_by_username(username), timestamp, self.authentication_token())
         response = self.app.get(url(controller='login',
                                     action='password_reset_confirmation',
                                     key=key))
         self.assertEqual(response.status, '302 Found')
         self.assertTrue(response.location.endswith(url('reset_password')))
         # GOOD 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)
         key = User.get_by_username(username).api_key
         response = self.app.get(url(controller='login',
                                     action='password_reset_confirmation',
                                     key=key))
         response = self.app.post(url(controller='login',
                                      action='password_reset_confirmation'),
                                  {'email': email,
                                   'timestamp': timestamp,
                                   'password': "p@ssw0rd",
                                   'password_confirm': "p@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'))
         self.checkSessionFlash(response, 'Successfully updated password')
         response = response.follow()
     #==========================================================================
     # API
     #==========================================================================
     def _get_api_whitelist(self, values=None):
         config = {'api_access_controllers_whitelist': values or []}
         return config

0 comments (0 inline, 0 general)