Don't error on password reset if no email found, and translate error messages

This commit is contained in:
James Allen 2014-08-08 11:41:54 +01:00
parent 5ec1c59a31
commit 10021986c5
5 changed files with 26 additions and 13 deletions

View file

@ -1,6 +1,6 @@
PasswordResetHandler = require("./PasswordResetHandler") PasswordResetHandler = require("./PasswordResetHandler")
RateLimiter = require("../../infrastructure/RateLimiter") RateLimiter = require("../../infrastructure/RateLimiter")
logger = require "logger-sharelatex"
module.exports = module.exports =
@ -18,11 +18,13 @@ module.exports =
RateLimiter.addCount opts, (err, canCompile)-> RateLimiter.addCount opts, (err, canCompile)->
if !canCompile if !canCompile
return res.send 500, { message: req.i18n.translate("rate_limit_hit_wait")} return res.send 500, { message: req.i18n.translate("rate_limit_hit_wait")}
PasswordResetHandler.generateAndEmailResetToken email, (err)-> PasswordResetHandler.generateAndEmailResetToken email, (err, exists)->
if err? if err?
res.send 500, {message:err?.message} res.send 500, {message:err?.message}
else else if exists
res.send 200 res.send 200
else
res.send 404, {message: req.i18n.translate("cant_find_email")}
renderSetPasswordForm: (req, res)-> renderSetPasswordForm: (req, res)->
res.render "user/setPassword", res.render "user/setPassword",

View file

@ -8,18 +8,20 @@ logger = require("logger-sharelatex")
module.exports = module.exports =
generateAndEmailResetToken:(email, callback)-> generateAndEmailResetToken:(email, callback = (error, exists) ->)->
UserGetter.getUser email:email, (err, user)-> UserGetter.getUser email:email, (err, user)->
if err then return callback(err) if err then return callback(err)
if !user? if !user?
logger.err email:email, "user could not be found for password reset" logger.err email:email, "user could not be found for password reset"
return callback(message:"Can't find that email, sorry.") return callback(null, false)
PasswordResetTokenHandler.getNewToken user._id, (err, token)-> PasswordResetTokenHandler.getNewToken user._id, (err, token)->
if err then return callback(err) if err then return callback(err)
emailOptions = emailOptions =
to : email to : email
setNewPasswordUrl : "#{settings.siteUrl}/user/password/set?passwordResetToken=#{token}" setNewPasswordUrl : "#{settings.siteUrl}/user/password/set?passwordResetToken=#{token}"
EmailHandler.sendEmail "passwordResetRequested", emailOptions, callback EmailHandler.sendEmail "passwordResetRequested", emailOptions, (error) ->
return callback(error) if error?
callback null, true
setNewUserPassword: (token, password, callback)-> setNewUserPassword: (token, password, callback)->
PasswordResetTokenHandler.getUserIdFromTokenAndExpire token, (err, user_id)-> PasswordResetTokenHandler.getUserIdFromTokenAndExpire token, (err, user_id)->

View file

@ -43,7 +43,7 @@ define [
response.success = false response.success = false
response.error = true response.error = true
response.message = response.message =
text: data.message?.text or "Something went wrong talking to the server :(. Please try again." text: data.message?.text or data.message or "Something went wrong talking to the server :(. Please try again."
type: 'error' type: 'error'
ga('send', 'event', formName, 'failure', data.message) ga('send', 'event', formName, 'failure', data.message)
} }

View file

@ -39,7 +39,7 @@ describe "PasswordResetController", ->
describe "requestReset", -> describe "requestReset", ->
it "should error if the rate limit is hit", (done)-> it "should error if the rate limit is hit", (done)->
@PasswordResetHandler.generateAndEmailResetToken.callsArgWith(1) @PasswordResetHandler.generateAndEmailResetToken.callsArgWith(1, null, true)
@RateLimiter.addCount.callsArgWith(1, null, false) @RateLimiter.addCount.callsArgWith(1, null, false)
@res.send = (code)=> @res.send = (code)=>
code.should.equal 500 code.should.equal 500
@ -50,7 +50,7 @@ describe "PasswordResetController", ->
it "should tell the handler to process that email", (done)-> it "should tell the handler to process that email", (done)->
@RateLimiter.addCount.callsArgWith(1, null, true) @RateLimiter.addCount.callsArgWith(1, null, true)
@PasswordResetHandler.generateAndEmailResetToken.callsArgWith(1) @PasswordResetHandler.generateAndEmailResetToken.callsArgWith(1, null, true)
@res.send = (code)=> @res.send = (code)=>
code.should.equal 200 code.should.equal 200
@PasswordResetHandler.generateAndEmailResetToken.calledWith(@email.trim()).should.equal true @PasswordResetHandler.generateAndEmailResetToken.calledWith(@email.trim()).should.equal true
@ -65,11 +65,19 @@ describe "PasswordResetController", ->
done() done()
@PasswordResetController.requestReset @req, @res @PasswordResetController.requestReset @req, @res
it "should send a 404 if the email doesn't exist", (done)->
@RateLimiter.addCount.callsArgWith(1, null, true)
@PasswordResetHandler.generateAndEmailResetToken.callsArgWith(1, null, false)
@res.send = (code)=>
code.should.equal 404
done()
@PasswordResetController.requestReset @req, @res
it "should lowercase the email address", (done)-> it "should lowercase the email address", (done)->
@email = "UPerCaseEMAIL@example.Com" @email = "UPerCaseEMAIL@example.Com"
@req.body.email = @email @req.body.email = @email
@RateLimiter.addCount.callsArgWith(1, null, true) @RateLimiter.addCount.callsArgWith(1, null, true)
@PasswordResetHandler.generateAndEmailResetToken.callsArgWith(1) @PasswordResetHandler.generateAndEmailResetToken.callsArgWith(1, null, true)
@res.send = (code)=> @res.send = (code)=>
code.should.equal 200 code.should.equal 200
@PasswordResetHandler.generateAndEmailResetToken.calledWith(@email.toLowerCase()).should.equal true @PasswordResetHandler.generateAndEmailResetToken.calledWith(@email.toLowerCase()).should.equal true

View file

@ -42,8 +42,8 @@ describe "PasswordResetHandler", ->
it "should check the user exists", (done)-> it "should check the user exists", (done)->
@UserGetter.getUser.callsArgWith(1) @UserGetter.getUser.callsArgWith(1)
@PasswordResetTokenHandler.getNewToken.callsArgWith(1) @PasswordResetTokenHandler.getNewToken.callsArgWith(1)
@PasswordResetHandler.generateAndEmailResetToken @user.email, (err)=> @PasswordResetHandler.generateAndEmailResetToken @user.email, (err, exists)=>
should.exist(err) exists.should.equal false
done() done()
@ -52,8 +52,9 @@ describe "PasswordResetHandler", ->
@UserGetter.getUser.callsArgWith(1, null, @user) @UserGetter.getUser.callsArgWith(1, null, @user)
@PasswordResetTokenHandler.getNewToken.callsArgWith(1, null, @token) @PasswordResetTokenHandler.getNewToken.callsArgWith(1, null, @token)
@EmailHandler.sendEmail.callsArgWith(2) @EmailHandler.sendEmail.callsArgWith(2)
@PasswordResetHandler.generateAndEmailResetToken @user.email, (err)=> @PasswordResetHandler.generateAndEmailResetToken @user.email, (err, exists)=>
@EmailHandler.sendEmail.called.should.equal true @EmailHandler.sendEmail.called.should.equal true
exists.should.equal true
args = @EmailHandler.sendEmail.args[0] args = @EmailHandler.sendEmail.args[0]
args[0].should.equal "passwordResetRequested" args[0].should.equal "passwordResetRequested"
args[1].setNewPasswordUrl.should.equal "#{@settings.siteUrl}/user/password/set?passwordResetToken=#{@token}" args[1].setNewPasswordUrl.should.equal "#{@settings.siteUrl}/user/password/set?passwordResetToken=#{@token}"