Commits

Richard Jones  committed 90a34b4

improved password reset with no emailing of passwords

  • Participants
  • Parent commits 0174b2a

Comments (0)

Files changed (8)

 - raven
 - passlib
 - py-bcrypt (If using the recommended bcrypt hasher)
+- itsdangerous
 
 Quick development setup
 -----------------------
     c.execute('update comments_journal set submitted_by=%s where submitted_by=%s', (new, old))
     c.execute('delete from users where name=%s', (old,))
 
+def show_user(store, user):
+    c = store.get_cursor()
+    user = store.get_user()
+    if not user:
+        sys.exit('user %r does not exist' % user)
+    for key in user.keys():
+        print '%s: %s' % (key, user[key])
+    for p in store.get_user_packages(user):
+        print '%s: %s' % (p['package_name'], p['role_name'])
+
 def nuke_nested_lists(store, confirm=False):
     c = store.get_cursor()
     c.execute("""select name, version, summary from releases
             nuke_nested_lists(*args)
         elif command == 'keyrotate':
             keyrotate(config, *args)
+        elif command == 'user':
+            show_user(*args)
         else:
             print "unknown command '%s'!"%command
         st.changed()

File config.ini.template

 cheesecake_password = secret
 key_dir = .
 simple_sign_script = /serversig
+; this is the secret used to sign password reset efforts - keep it secret!
+; ''.join(random.choice(string.letters + string.digits) for n in range(64))
+reset_secret = secret
 
 [passlib]
 ; The first listed schemed will automatically be the default, see passlib
             self.sshkeys_update = c.get('webui', 'sshkeys_update')
         else:
             self.sshkeys_update = None
+        self.reset_secret = c.get('webui', 'reset_secret')
 
         self.logfile = c.get('logging', 'file')
         self.logging_mailhost = c.get('logging', 'mailhost')

File templates/password_reset.pt

       xmlns:metal="http://xml.zope.org/namespaces/metal"
       metal:use-macro="standard_template/macros/page">
   <metal:fill fill-slot="body">
-    <p>You have two options if you have forgotten your password. If you
-    know the email address you registered with, enter it below.</p>
+    <p>To reset your account password please enter the username below.</p>
 
-    <p tal:condition="data/retry | nothing"><strong>You must supply a username or
-    password!</strong></p>
+    <p tal:condition="data/retry | nothing"><strong>You must supply a
+      username!</strong></p>
 
     <form tal:attributes="action app/url_path" method="POST">
-      <input type="hidden" name=":action" value="password_reset" />
+      <input type="hidden" name=":action" value="pw_reset" />
       <table class="form">
-	<tr>
-	  <th>Email Address:</th>
-	  <td><input name="email" /></td>
-	</tr>
-	<tr>
-	  <td></td>
-	  <td><input type="submit" value="Reset password" /></td></tr>
+    <tr>
+      <th>Username:</th>
+      <td><input name="name" /></td>
+    </tr>
+    <tr>
+      <td></td>
+      <td><input type="submit" value="Reset password" /></td>
+    </tr>
       </table>
     </form>
 
-    <p>Or, if you know your username, then enter it below.</p>
+    <p>An email will be sent to you at the account registered address -
+    please follow the instructions within it to complete the reset process.</p>
 
-    <form tal:attributes="action app/url_path" method="POST">
-      <input type="hidden" name=":action" value="password_reset" />
-      <table class="form">
-	<tr>
-	  <th>Username:</th>
-	  <td><input name="name" /></td>
-	</tr>
-	<tr>
-	  <td></td>
-	  <td><input type="submit" value="Reset password" /></td>
-	</tr>
-      </table>
-    </form>
-
-    <p>A confirmation email will be sent to you - please follow the instructions
-    within it to complete the reset process.</p>
-    
   </metal:fill>
 </html>

File templates/password_reset_change.pt

+<?xml version="1.0" encoding="utf-8"?>
+<!DOCTYPE html PUBLIC "-//W3C//DTD XHTML 1.0 Transitional//EN" "http://www.w3.org/TR/xhtml1/DTD/xhtml1-transitional.dtd">
+<html xmlns:tal="http://xml.zope.org/namespaces/tal"
+      xmlns:metal="http://xml.zope.org/namespaces/metal"
+      metal:use-macro="standard_template/macros/page">
+  <metal:fill fill-slot="body">
+    <p>Please enter your new account password below.</p>
+
+    <p tal:condition="data/retry | nothing">
+      <strong tal:content="data/retry" />
+    </p>
+
+    <form tal:attributes="action app/url_path" method="POST">
+      <input type="hidden" name=":action" value="password_reset_change" />
+      <input type="hidden" name="otk" value="data/otk" />
+      <table class="form">
+    <tr>
+      <th>Password:</th>
+      <td><input type="password" name="password" /><br/>
+      Minimum 8 characters.
+      </td>
+    </tr>
+    <tr>
+      <th>Password again:</th>
+      <td><input type="password" name="confirm" /></td>
+    </tr>
+    <tr>
+      <td></td>
+      <td><input type="submit" value="Set Password" /></td>
+    </tr>
+      </table>
+    </form>
+  </metal:fill>
+</html>

File tools/sqlite_create.py

 "create index journals_version_idx on journals(version)",
 "create index packages_name_idx on packages(name)",
 "create index rego_otk_name_idx on rego_otk(name)",
+"create index reset_otk_name_idx on reset_otk(name)",
+"create index reset_otk_otk_idx on reset_otk(otk)",
 "create index rel_class_name_idx on release_classifiers(name)",
 "create index rel_class_trove_id_idx on "
     "release_classifiers(trove_id)",
 from distutils.util import rfc822_escape
 from distutils2.metadata import Metadata
 from xml.etree import cElementTree
+import itsdangerous
 
 try:
     import json
 username, "%(name)s". If you wish to proceed with the change, please follow
 the link below:
 
-  %(url)s?:action=password_reset&email=%(email)s
-
-You should then receive another email with the new password.
-
-'''
-
-# password reset email - indicates what the password is now
-password_message = '''Subject: PyPI password has been reset
-From: %(admin)s
-To: %(email)s
-
-Your login is: %(name)s
-Your password is now: %(password)s
+  %(url)s?:action=pw_reset&otk=%(otk)s
+
+This will present a form in which you may set your new password.
 '''
 
 _prov = '<p>You may also login or register using <a href="%(url_path)s?:action=openid">OpenID</a>'
         # handle the action
         if action in '''debug home browse rss index search submit doap
         display_pkginfo submit_pkg_info remove_pkg pkg_edit verify submit_form
-        display register_form user_form forgotten_password_form user
-        password_reset role role_form list_classifiers login logout files
+        display register_form user user_form
+        forgotten_password_form
+        password_reset pw_reset pw_reset_change
+        role role_form list_classifiers login logout files
         file_upload show_md5 doc_upload claim openid openid_return dropid
         clear_auth addkey delkey lasthour json gae_file about delete_user
         rss_regen openid_endpoint openid_decide_post packages_rss
                     nonce = qs['openid.response_nonce'][0]
             else:
                 claimed_id = None
-                if not info.has_key('confirm') or info['password'] <> info['confirm']:
-                    self.fail("password and confirm don't match", heading='Users')
-                    return
+                msg = self._verify_new_password(info['password'],
+                    info.get('confirm'))
+                if msg:
+                    return self.fail(msg, heading='Users')
 
             # validate a complete set of stuff
             # new user, create entry and email otk
                 password = None
             else:
                 # make sure the confirm matches
-                if password != info.get('confirm', ''):
-                    self.fail("password and confirm don't match",
-                        heading='User profile')
-                    return
+                msg = self._verify_new_password(password, info.get('confirm'),
+                    user)
+                if msg:
+                    return self.fail(msg, heading='User profile')
             email = info.get('email', user['email'])
             gpg_keyid = info.get('gpg_keyid', user['gpg_keyid'])
             self.store.store_user(self.username, password, email, gpg_keyid)
         self.update_sshkeys()
         return self.register_form()
 
-    def forgotten_password_form(self):
-        ''' Enable the user to reset their password.
-        '''
-        self.write_template("password_reset.pt", title="Request password reset")
-
     def password_reset(self):
-        """Reset the user's password and send an email to the address given.
+        """Send a password reset email to the user attached to the address
+        nominated.
+
+        This is a legacy interface used by distutils which supplies an email
+        address.
         """
-        def resend_otk():
-            info = {'otk':self.store.get_otk(user['name']), 'url':self.config.url,
-                    'admin':self.config.adminemail, 'email': user['email'],
-                    'name':user['name']}
+        email = self.form.get('email', '').strip()
+        user = self.store.get_user_by_email(email)
+        if not user:
+            return self.fail('email address unknown to me')
+
+        # check for existing registration-confirmation OTK
+        if self.store.get_otk(user['name']):
+            info = {'otk': self.store.get_otk(user['name']),
+                'url': self.config.url, 'admin': self.config.adminemail,
+                'email': user['email'], 'name':user['name']}
             self.send_email(info['email'], rego_message%info)
             response = 'Registration OK'
             message = 'You should receive a confirmation email shortly.'
             self.write_template('message.pt', title="Resending registration key",
                 message='Email with registration key resent')
 
-        if self.form.has_key('email') and self.form['email'].strip():
-            email = self.form['email'].strip()
-            user = self.store.get_user_by_email(email)
-            if not user:
-                self.fail('email address unknown to me')
-                return
-            if self.store.get_otk(user['name']):
-                return resend_otk()
-            pw = ''.join([random.choice(chars) for x in range(10)])
-            self.store.store_user(user['name'], pw, user['email'], None)
-            info = {'name': user['name'], 'password': pw,
-                'email':user['email']}
-            info['admin'] = self.config.adminemail
-            self.send_email(email, password_message%info)
-            self.write_template('message.pt', title="Request password reset",
-                message='Email sent with new password')
-        elif self.form.has_key('name') and self.form['name'].strip():
-            name = self.form['name'].strip()
-            user = self.store.get_user(name)
-            if not user:
-                self.fail('user name unknown to me')
-                return
-            if self.store.get_otk(user['name']):
-                return resend_otk()
-            info = {'name': user['name'], 'url': self.config.url,
-                 'email': urllib.quote(user['email'])}
-            info['admin'] = self.config.adminemail
-            self.send_email(user['email'], password_change_message%info)
-            self.write_template('message.pt', title="Request password reset",
-                message='Email sent to confirm password change')
-        else:
-            self.write_template("password_reset.pt", title="Request password reset",
-                retry=True)
+        # generate a reset OTK and mail the link
+        info = dict(name=user['name'], url=self.config.url,
+            otk=self._gen_reset_otk())
+        info['admin'] = self.config.adminemail
+        self.send_email(user['email'], password_change_message % info)
+        self.write_template('message.pt', title="Request password reset",
+            message='Email sent to confirm password change')
+
+    def forgotten_password_form(self):
+        ''' Enable the user to reset their password.
+
+        This is the first leg of a password reset and requires the user
+        identify themselves somehow by supplying their username or email
+        address.
+        '''
+        self.write_template("password_reset.pt",
+            title="Request password reset")
+
+    def forgotten_password(self):
+        '''Accept a user's submission of username and send a
+        reset email if it's valid.
+        '''
+        name = self.form.get('name', '').strip()
+        if not name:
+            self.write_template("password_reset.pt",
+                title="Request password reset", retry=True)
+
+        user = self.store.get_user(name)
+        # typically other systems would not indicate the username is invalid
+        # but in PyPI's case the username list is public so this is more
+        # user-friendly with no security penalty
+        if not user:
+            self.fail('user name unknown to me')
+            return
+
+        # existing registration OTK?
+        if self.store.get_otk(user['name']):
+            info = dict(
+                otk=self.store.get_otk(user['name']),
+                url=self.config.url,
+                admin=self.config.adminemail,
+                email=user['email'],
+                name=user['name'],
+            )
+            self.send_email(info['email'], rego_message % info)
+            return self.write_template('message.pt',
+                title="Resending registration key",
+                message='Email with registration key resent')
+
+        # generate a reset OTK and mail the link
+        info = dict(name=user['name'], url=self.config.url,
+            otk=self._gen_reset_otk())
+        info['admin'] = self.config.adminemail
+        self.send_email(user['email'], password_change_message % info)
+        self.write_template('message.pt', title="Request password reset",
+            message='Email sent to confirm password change')
+
+    def _gen_reset_otk(self):
+        # generate the reset key and sign it
+        reset_signer = itsdangerous.URLSafeTimedSerializer(
+            self.config[reset_secret], 'password-recovery')
+
+        # we include a snip of the current password hash so that the OTK can't
+        # be used again once the password is changed. And hash it to be extra
+        # obscure
+        return reset_signer.dumps((name, user['password'][-4:]))
+
+    def _decode_reset_otk(self, otk):
+        reset_signer = itsdangerous.URLSafeTimedSerializer(
+            self.config[reset_secret], 'password-recovery')
+        try:
+            # we allow 6 hours
+            name, x = reset_signer.loads(otk, max_age=6*60*60)
+        except itsdangerous.BadData:
+            return None
+        return self.store.get_user(name)
+
+    def pw_reset(self):
+        '''The user has clicked the reset link in the email we sent them.
+
+        Validate the OTK we are given and display a form for them to set their
+        new password.
+        '''
+        otk = self.form.get('otk', '').strip()
+        user = self._decode_reset_otk(otk)
+        if not user:
+            self.fail('invalid password reset token')
+            return
+        self.write_template('password_reset_change.pt', otk=otk,
+            title="Password reset")
+
+    def pw_reset_change(self):
+        '''The final leg in the password reset sequence: accept the new
+        password.'''
+        otk = self.form.get('otk', '').strip()
+        user = self._decode_reset_otk(otk)
+        if not user:
+            self.fail('invalid password reset token')
+            return
+
+        pw = self.form.get('password', '').strip()
+        confirm = self.form.get('confirm', '').strip()
+
+        msg = self._verify_new_password(pw, confirm, user)
+        if msg:
+            return self.write_template('password_reset_change.pt',
+                title="Password reset", retry=msg)
+
+        self.store.store_user(user['name'], pw, user['email'], None)
+        self.store.delete_reset_otk(otk)
+        self.write_template('message.pt', title="Password reset",
+            message='Password has been reset')
+
+    def _verify_new_password(self, pw, confirm, user=None):
+        '''Verify that the new password is good.
+
+        Returns a reason string if the verification fails.
+        '''
+        if user and self.config.passlib.verify(pw, user['password']):
+            return 'Please ensure the new password is not the same as the old.'
+
+        if user and pw == user['name']:
+            return 'Please make your password harder to guess.'
+
+        if pw != confirm:
+            return '''Please check you've entered the same password in
+                both fields.'''
+
+        if len(pw) < 8:
+            return "Please make your password at least 8 characters long."
+
+        if len(pw) < 16 and (pw.isdigit() or pw.isalpha() or pw.isupper()
+                or pw.islower()):
+            return '''Please use a mix of different-case letters and numbers
+                in your password.'''
+
+        return ''
 
     def delete_user(self):
         if not self.authenticated: