ranisalt / node-argon2

Node.js bindings for Argon2 hashing algorithm
https://www.npmjs.com/package/argon2
MIT License
1.86k stars 92 forks source link

Incompatible with node 4.x #124

Closed simonepri closed 6 years ago

simonepri commented 6 years ago

The readme says that this package is compatible with node >= 4 but actually is compatible only with node >= 6. Since it's quite easy to support also node 4, @ranisalt would you consider a PR to add back the support for it?

ranisalt commented 6 years ago

The README is outdated. Node 4 is on maintenance support (until 4/30) and people should be pushed to move to 6 or, preferably, 8 as soon as possible.

I do not intend to support deprecated Node versions unless there's a good reason to.

simonepri commented 6 years ago

In general I agree with you about not supporting deprecated versions of node. But this package seems to be the only Argon2 bindings available and keeping a good support also for older node versions it may be reasonable.

The library I'm building upash wants to supports node >= 4 but without a slightly modification to this package I cannot achieve that.

Would you accept a PR for this?

ranisalt commented 6 years ago

Sure, I would, but also add to the README that deprecated Node versions are supported on a best effort basis.

ranisalt commented 6 years ago

Are you working on it still @simonepri? I'm looking at Node LTS release and version 6 starts maintenance support this month, so I was thinking on moving to async functions and classes and drop support for Node <8 altogether. Unfortunately npmjs.org doesn't provide analytics so I can check the % of users on those versions.

simonepri commented 6 years ago

Actually I didn't have the time so far. Sorry.

My aim is still to support node 4. (There's no need to use async functions when you can you promises without affecting code readability too much.) I'm working to unify all the password hashing algorithms to make them adhere to the PHC standard while using the same API interface for every one.

My idea for the node-argon2 repo is to provide raw only binding to the inner c++ implementation of the kdf function and then having another package that uses the kdf to compute the hash and encodes the hash string using some particular standard (PHC for instance).

Actually I've already done that for some algorithms: https://github.com/simonepri/phc-argon2 https://github.com/simonepri/phc-scrypt https://github.com/simonepri/phc-pbkdf2

In particular this is the argon2 one that uses node-argon2 raw hash methods and then take care of encoding the string.

I don't know if my aim fits well with your idea for this project but I thought was interesting discussing about it.

ranisalt commented 6 years ago

That would definitely reduce the codebase, surely. I think you could develop a protocol for KDF functions to work under the @phc namespace, say hash(plain[, {options}]) and verify(plain, digest) both returning promises, similar to what this package does.

I come from a Python background where stubborn devs insist on using Python 2 even though it's been deprecated for years and very close to EOL. Ditto for C++ pre-11. That's why I try to drop support for old versions :stuck_out_tongue_closed_eyes:

ranisalt commented 6 years ago

Node 4 has officialy EOL'd so don't expect support now if we need boilerplate code to support it.