Fix for #99 #100
Fix for #99 #100
Conversation
server/api/UserRoute.js
Outdated
| @@ -317,7 +317,7 @@ module.exports = (app) => { | |||
| }) | |||
| .catch((error) => { | |||
| if (error.message === "404") { | |||
| return res.status(404).send(error); | |||
| return res.status(200).send(error); | |||
There was a problem hiding this comment.
Returning a 200 is not enough. The email can still be guessed in two ways:
- The data that is returned to the client differs. Look at
resulton line 316 anderroron line 320 - The request will finish faster or slower depending on whether there is an error or not
So how to deal with these?
- Always return the same message. Something like
{ success: true }will do - Respond to the request right away and don't wait for the
userController.requestPasswordReset()promise
There was a problem hiding this comment.
Sorry for it, i understand and fixing right away
There was a problem hiding this comment.
I did what you asked for but if i dont wait promises how can i return 400 in real error ?
There was a problem hiding this comment.
There is no need to return an error here because this will tell the user if the email is in the system or not. So basically, someone could try out random emails to see if they have a Chartbrew account. This is a security issue.
Returning the same message all the time will stop attackers from finding valid email addresses.
server/api/UserRoute.js
Outdated
| return res.status(400).send(error); | ||
| }); | ||
| userController.requestPasswordReset(req.body.email) | ||
| return res.status(200).send({"success": true}); |
There was a problem hiding this comment.
Always recommended to use a code editor that supports linting highlighting. This way you can check wether your code is respecting the guidelines of the project.
In the first line, you're missing a semicolon and the second one is missing spaces around the curly brackets.
https://app.codacy.com/gh/chartbrew/chartbrew/pullRequest?bid=25774484&prid=8190654
There was a problem hiding this comment.
Saw it and fixed thank you for explanation and your patience
Thank you |
Fix #99
Always return code 200 for preventing email guessing.