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

fix: trim whitespace from password field during login validation #1386

Closed
wants to merge 2 commits into from

Conversation

harshitg927
Copy link
Contributor

fixes issue#1382

Summary

Currently, users cannot log in when their passwords contain leading or trailing whitespace characters. This is causing failed login attempts even with correct passwords. The fix implements password string trimming during validation to allow successful authentication by removing unintentional spaces.

Changes

  1. Added whitespace trimming for password field
  2. Improves login form usability

Screen-recording

Screencast.from.09-01-25.21.42.13.webm

Related Issue

#1385

@harshitg927
Copy link
Contributor Author

@mozzy11 can you please review this pull request?

@yashgoyal0110
Copy link

would like to contribute on it

@Lemeri123
Copy link

Lemeri123 commented Jan 9, 2025

Well done @harshitg927. Now format your code correctly with the npm run format

@harshitg927
Copy link
Contributor Author

@Lemeri123 I have formated the code using npm run format... please review.

@Lemeri123
Copy link

Well done for solving the issue

Copy link
Contributor

@Bahati308 Bahati308 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

hello @harshitg927, thanks for the work, just a few changes and you will be there. The frontend/cypress/pages/ModifyOrderPage.js file is not responsible for login validation.

@harshitg927
Copy link
Contributor Author

@Bahati308 That's true, but these changes are a result of me running npm run format, should I still revert them?

@vsvishalsharma
Copy link
Contributor

@Bahati308 That's true, but these changes are a result of me running npm run format, should I still revert them?

Yeah Kindly revert them if you haven't intended to do these changes

@harshitg927
Copy link
Contributor Author

@vsvishalsharma done

@mozzy11 mozzy11 requested a review from CalebSLane January 13, 2025 13:02
@mozzy11
Copy link
Collaborator

mozzy11 commented Jan 13, 2025

@CalebSLane is it fine to trim the login passowrd ??

@harshitg927
Copy link
Contributor Author

Hey @CalebSLane, are there any updates for this Pull Request? Thank You.

@mozzy11
Copy link
Collaborator

mozzy11 commented Jan 20, 2025

Thanks @harshitg927 .
In general based on Security recomendations from OWASP ,NIST, RFC 7617, its not recomeneded to trim user passowrds since some paswords may contain white spaces intetionally.
User passwords should be treated as entered

@mozzy11 mozzy11 closed this Jan 20, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants