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.

Sign up for GitHub

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

Jump to bottom

Never hash synchronously #47

Open
oliverjam opened this issue Apr 10, 2020 · 0 comments
Open

Never hash synchronously #47

oliverjam opened this issue Apr 10, 2020 · 0 comments
Labels
bug Something isn't working

Comments

@oliverjam
Copy link

week6-hihi/source/handlers.js

Line 112 in ddd7b68

data.password = bcryptjs.hashSync(data.password, salt);

Using the sync version of BCrypt is a very bad idea. Hashing is specifically designed to take a long time (for security). Doing this synchronously means your server gets blocked on this one line of code until the hashing finishes, unable to handle any other incoming requests. This is one case where it's super important to use the async promise version.
@hannahgooding hannahgooding added the bug Something isn't working label Apr 10, 2020
hannahgooding added a commit that referenced this issue Apr 10, 2020
@hannahgooding @HettieM @itsina96
Attempt to change bcryptSync to bcrypt with promise handling
73bcf34 
Relates #47

Co-authored-by: HettieM <hettie.mcconnell@outlook.com>
Co-authored-by: itsina96 <itsina96@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
bug Something isn't working
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
2 participants
@oliverjam @hannahgooding