claudioc / jingo

Node.js based Wiki
MIT License
1.02k stars 184 forks source link

Replace SHA1 with bcrypt for local authentication #143

Open cholling opened 8 years ago

cholling commented 8 years ago

I've made some simple changes to my fork to use bcrypt-nodejs to provide more secure, salted password hashes. I've verified that I can successfully create passwords and subsequently log in using this code, but haven't extensively tested it (or, indeed, know what testing needs to be done apart from simply verifying that I can log in).

The bcrypt-nodejs project uses a 3-clause BSD license, which I believe is compatible with jingo's MIT license, but I'd like to get confirmation from someone who understands these things better than I.

Any comments or suggestions for improvement before this is ready for a pull request?

claudioc commented 8 years ago

Hi,

I don't see any particular problem for that to happen, unless of course it could pose a backward compatibility problem. I try to maintain Jingo as simple as possible, for the user to install and maintain and for me to maintain it. Do you think that this would be a drop-in replacement for an existing (possibly 2 or 3 years old) installation or would it be a breaking change? Would we need to add a specific condition in the configuration file, something like "useCrypt: bcrypt" (defaulting to the previous one?)

Looking at your code it seems that this is the case (old password won't work anymore if you just update jingo).

cholling commented 8 years ago

You're right, this is the case. It would work fine for new installations, but break all the passwords for an existing one.

I could add the config option. Another possibility is to default to bcrypt for creating new passwords, but when verifying an existing password, try the old SHA1 hashing if bcrypt doesn't verify. That way, everyone's current password continues to work, but if/when they change their password, it will use the more secure hash. From the end user's standpoint it would be completely transparent.