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

Support for [bcrypt] #4

Closed
Vlix opened this issue Jan 21, 2020 · 4 comments · Fixed by #8
Closed

Support for [bcrypt] #4

Vlix opened this issue Jan 21, 2020 · 4 comments · Fixed by #8
Labels
enhancement New feature or request good first issue Good for newcomers help wanted Extra attention is needed

Comments

@Vlix
Copy link
Collaborator

Vlix commented Jan 21, 2020

Just found this library and it's not bad (surprised the password name wasn't used before), but would like to see more from this library for convenience.

bcrypt is still widely used and shouldn't be too difficult to add (we have an implementation of it very similar to this library)
Maybe a good idea to move functions specific to a certain encryption to their own modules so that it's easier to know what you're using. e.g.:

import Data.Password (Pass)
import qualified Data.Password.Scrypt as Scrypt
import qualified Data.Password.Bcrypt as Bcrypt

myCheck :: Pass -> Either Scrypt.PassHash Bcrypt.PassHash -> PassCheck
myCheck pass = either (Scrypt.checkPass pass) (Bcrypt.checkPass pass)
@cdepillabout cdepillabout added enhancement New feature or request good first issue Good for newcomers help wanted Extra attention is needed labels Jan 21, 2020
@cdepillabout
Copy link
Owner

This sounds like a good idea. I'd definitely accept a PR adding this.

Although I think it would probably just be easier to keep everything in Data.Password and just have checkPassScrypt, checkPassBcrypt, etc functions.

@cdepillabout
Copy link
Owner

cdepillabout commented Jan 21, 2020

Now that I think about this, you'd also need corresponding hashing functions for both Scrypt and Bcrypt, so maybe adding them to different modules would be easier!

Although I do think that Data.Password.Bcrypt and Data.Password.Scrypt should re-export everything from Data.Password (which should really only contain the Pass and PassHash types, and related functions). That way the end user generally won't have to import both Data.Password and Data.Password.Bcrypt.

@Vlix
Copy link
Collaborator Author

Vlix commented Jan 22, 2020

Yes, it'd be more user friendly to re-export the common data types too, though I'd opt for having the PassHash data type be changed to ScryptHash and BcryptHash, so just in case anyone uses both algorithms, you can't mix them up by using a Bcrypt-hashed PassHash in a check for Scrypt.

Might even be nice to also add PBKDF2, since then you'd have all 3 "generally" accepted standards of password hashing.

@cdepillabout
Copy link
Owner

I'd opt for having the PassHash data type be changed to ScryptHash and BcryptHash

I was thinking that just having phantom types on PassHash would be fine.

So we could have PassHash Scrypt and PassHash Bcrypt.

If anyone wanted to write instances for PassHash, they'd generally just have to write them for PassHash a and have them apply to both PassHash Scrypt and PassHash Bcrypt.

Might even be nice to also add PBKDF2, since then you'd have all 3 "generally" accepted standards of password hashing.

This definitely sounds like a good idea too!

@Vlix Vlix mentioned this issue Jan 26, 2020
@cdepillabout cdepillabout linked a pull request Apr 28, 2020 that will close this issue
@Vlix Vlix closed this as completed in #8 May 3, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request good first issue Good for newcomers help wanted Extra attention is needed
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants