[PATCH v2] auth: secure password reset implementation

Mads Kiilerich mads at kiilerich.com
Sun Aug 9 02:53:11 UTC 2015


On 08/08/2015 05:25 PM, Andrew Shadura wrote:
> @@ -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)

Here and above: It shouldn't use canonical_url, just a normal url.

If a site has multiple names/addresses, it is important that the URL 
points at the current name. If it points at another (canonical) 
location, the session id will be different and the link is not useful.

(The UI should perhaps also mention that it is important that the 
verification token from the mail must be used in the same browser 
session as it was requested in.)

/Mads


More information about the kallithea-general mailing list