-
Notifications
You must be signed in to change notification settings - Fork 114
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
changed password hash to bcrypt fixes #454 #457
changed password hash to bcrypt fixes #454 #457
Conversation
|
✅ Deploy Preview for endearing-brigadeiros-63f9d0 ready!
To edit notification comments on pull requests, go to your Netlify site configuration. |
Hi @singhotto , thanks for your PR, and welcome to FINOS! I did implement the fix on https://github.com/finos/git-proxy/pull/453/files ; would you mind reviewing it and rebasing your PR against the Also, please let me know if you encounter any issues with EasyCLA (comment above). |
Not sure I understand your question, sorry; we started looking at bcrypt mainly because password-hash is not maintained, and we don't want to bring security issues from unmaintained third-party libraries in our codebase; for this reason, we want to get rid of it entirely from the codebase. That said, I like the way you use
Long story short, to ensure that contributors are able to contribute their IP to us, and they're aware of it; this also leads to less legal risks for contributors. For the longer story, I suggest you reading https://osr.finos.org/docs/bok/artifacts/clas-and-dcos ; hope this answers your question. |
739a9d0
to
3a38dee
Compare
@singhotto - I just realised that I edited your comment, instead of replying. Apologies for that, I didn't realise it! I've refined the You raise a fair point about password already being hashed with I appreciate a lot your help on this; would you be interested to continue contributing to this project? If that's the case, I'd be happy to jump on a quick call, figure our the EasyCLA setup and maybe discuss few other activities we're planning to tackle as a team? Feel free to ping me on maoo@finos.org , eager to keep this collaboration going! |
Closing as addressed in #335 👍 Appreciate your support @singhotto and look forward to more of your support on the project ❤️ |
fixes #454
I included the bcrypt library for password hashing, but the database already contains passwords hashed with the password-hash library. Instead of removing the old library, I've kept it, and now the verification process utilizes both libraries.