TheOdinProject / curriculum

The open curriculum for learning web development
https://www.theodinproject.com/
Other
10.01k stars 13.4k forks source link

Bug: in Authentication basics #24804

Closed Cherifi-Houdaifa closed 1 year ago

Cherifi-Houdaifa commented 2 years ago

https://github.com/TheOdinProject/curriculum/blob/547c13f0a87c04fea485448c39ab0a0c400ee096/nodeJS/authentication/authentication_basics.md?plain=1#L329-L337 This code is asynchronous and then there is a return statement after it which will cause it to return two times which will throw an error: Error [ERR_HTTP_HEADERS_SENT]: Cannot set headers after they are sent to the client you may consider using promises with async / await to make sure that the async operation finishes before next return statement or you can just remove the last return statment In the LocalStrategy Function

Aliervo commented 2 years ago

I noticed this issue as well. Unless I am mistaken, using Promises won't actually prevent this from throwing an error, but rewriting it to use compareSync could.

I believe the simplest solution is modifying the sentence above the code block to mention deleting the double return.

Happy to send a PR if this solution sounds agreeable.

github-actions[bot] commented 1 year ago

This issue is stale because it has had no activity for the last 30 days.

github-actions[bot] commented 1 year ago

This issue is stale because it has had no activity for the last 30 days.

kxrn0 commented 1 year ago

instead use

//...
const matches = await bcrypt.compare(password, user.password);

if (!matches) return done(null, false, {message: "Incorrect password!"});
//...
wise-king-sullyman commented 1 year ago

It seems like something should be done here but I'm not certain what. @TheOdinProject/javascript any of yall familiar enough with using bcrypt to know what the best practice approach here is?

github-actions[bot] commented 1 year ago

This issue is stale because it has had no activity for the last 30 days.

ManonLef commented 1 year ago

I'm fairly sure this can be rewritten to another example using the bcrypt docs examples or from the many bcrypt examples online.

I will open this up for someone experienced enough with bcrypt to propose a new code sample

Lofty-Brambles commented 1 year ago

Will take a dig and open a PR in 24 hours.

ManonLef commented 1 year ago

assigned you @Lofty-Brambles thanks so much!

Lofty-Brambles commented 1 year ago

Minimum reproducible solution: https://codesandbox.io/p/sandbox/elegant-lewin-km7pj2