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

User friendly error when input is incorrect in checkpw/2 #77

Closed
stevedomin opened this issue Feb 21, 2016 · 2 comments
Closed

User friendly error when input is incorrect in checkpw/2 #77

stevedomin opened this issue Feb 21, 2016 · 2 comments

Comments

@stevedomin
Copy link

Currently if you try to use checkpw/2 with a non-hashed password it fails with the following error:

Comeonin.Bcrypt.checkpw("password", "notahashedpw")
** (MatchError) no match of right hand side value: ['notahashedpw']
    (comeonin) lib/comeonin/bcrypt.ex:161: Comeonin.Bcrypt.hashpw/2
    (comeonin) lib/comeonin/bcrypt.ex:122: Comeonin.Bcrypt.checkpw/2

I'm wondering if it would be useful to return an {:error, reason} tuple or something to indicate that there was an error. Otherwise you need to wrap it try..catch block.

I ran into this issue because I was not hashing the password before inserting them in the DB, so when I tried to log in I got this error.

@riverrun
Copy link
Owner

Thanks for raising this issue. After some reflection, though, I've decided to leave this behavior as it is - I think it should raise an error in this situation. Sorry!
I don't really think you need to wrap the function in a try...catch block. Now that you realize that the password needs to be hashed, there's going to be no problem, and it's not going to be affected by any invalid user input because the developer is in control of the stored hash.

@stevedomin
Copy link
Author

@riverrun thanks for replying. No need to be sorry, it's not a real issue in that case.

Since you can go either way on it (raise or return a {:error, reason} tuple) I was wondering if it was an oversight so I opened this issue.

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

No branches or pull requests

2 participants