Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Password can be reset wiithout new password confirmation. #15909

Closed
ilhamije opened this issue Sep 22, 2017 · 12 comments
Closed

Password can be reset wiithout new password confirmation. #15909

ilhamije opened this issue Sep 22, 2017 · 12 comments
Labels
first timers only Open for anyone doing contributions for first time. You do not need permission to work on these. help wanted Open for all. You do not need permission to work on these.

Comments

@ilhamije
Copy link

Issue Description

User can reset their password by only fiilling out new password field without filling out password confirmation.

How to reproduce it:

  1. Go to https://www.freecodecamp.org/forgot and try to reset your password.
  2. Open a confirmation email from FCC to be directed to reset password page.
  3. Fill out your new password on the New Password field and leave the Confirm Password blank.
  4. Hit Change Password button.
  5. Try to login with your new password.

Browser Information

  • Browser Name, Version: Google Chrome Version 58.0.3029.110 (64-bit)
  • Operating System: Linux Ubuntu 16.04
  • On Desktop

Screenshot

screenshot from 2017-09-23 01-35-43
screenshot from 2017-09-23 01-36-09
screenshot from 2017-09-23 01-36-34
screenshot from 2017-09-23 01-37-07
screenshot from 2017-09-23 01-37-23
screenshot from 2017-09-23 01-37-51
screenshot from 2017-09-23 01-38-10

@raisedadead
Copy link
Member

This essentially needs a client-side validation of the password to be same in both the inputs, and should be added here.

https://github.com/freeCodeCamp/freeCodeCamp/blob/backup/master/server/views/account/reset.jade#L15

The server-side code is all fine.

@raisedadead raisedadead added first timers only Open for anyone doing contributions for first time. You do not need permission to work on these. help wanted Open for all. You do not need permission to work on these. labels Sep 22, 2017
@raisedadead
Copy link
Member

Potential contributors, please add some jQuery based validation in the reset view mentioned above. Also note that this needs to go into the backup/master branch.

Happy fixing!

@shovanch
Copy link

@raisedadead Looking forward to fix this. This would be my first open source contribution to freeCodeCamp

@raisedadead
Copy link
Member

Yes please go ahead!

@msasad
Copy link

msasad commented Sep 24, 2017

All input validations must be carried out on the server side also. Just client side validation is not the solution. Server side code needs to be fixed as well.

@raisedadead
Copy link
Member

Server side code needs to be fixed as well.

Please elaborate, or point to code which is to be fixed.

@tracyalison11
Copy link

Are you looking for the change password button to be disabled until both inputs are filled and matched or do you want an error message thrown?

@raisedadead
Copy link
Member

@tracyalison11 we would like to display an error message (basic html5 validation), and of course blocking the submit until the form has validated.

some inspiration https://www.html5rocks.com/en/tutorials/forms/constraintvalidation/

@shovanch
Copy link

@raisedadead I have been working on this. Hoping to add PR before the end of the day

@msasad
Copy link

msasad commented Sep 26, 2017

Please elaborate, or point to code which is to be fixed.

If password can be changed without confirmation, this clearly means that there is no validation on the server side. The lack of validation at the server side itself is an issue which needs to be fixed.
The answer to this SF question might help in understanding why server side validation is necessary.
While it is better to have validation on both sides, if we ever choose to have validation in only one side, then it has to be at the server.

@raisedadead
Copy link
Member

If password can be changed without confirmation, this clearly means that there is no validation on the server side.

Did you get a chance to look at the server side code and confirm this? And that there is actually no validation?

What the SF article suggests is totally valid, and thanks for sharing it.

... Server-side validation is also crucial due to the fact that client-side validation can be completely bypassed by turning off JavaScript. ...

But that said, the use case we have here has one additional server-side check already. Hence my comments earlier. IMHO, I suggest you to please confirm this yourself if you so, please by actually checking the code. Should you face issues finding it, we would be glad to help you.

Yes, that said the client side view needs to be fixed, which again as per the article you shared:

... In practice, all it does is prevent your client (with JS enabled) to know whether the form is okay ...

And I totally agree.

Finally, this issue basically boils down to the fact that we had missed doing so in letting the user know that the form was not okay, as pointed out by OP.

@mohisen
Copy link

mohisen commented Sep 27, 2017 via email

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
first timers only Open for anyone doing contributions for first time. You do not need permission to work on these. help wanted Open for all. You do not need permission to work on these.
Projects
None yet
Development

No branches or pull requests

6 participants