Stop using passlib #3729
Stop using passlib #3729
Conversation
bcrypt 5.x makes a change that explicitly warns about truncating long passwords. passlib tests for a bug of some previous version by testing with a long password, which raises this exception. We currently use passlib for bcrypt and for one-time-passwords (2fa). For the use of bcrypt, the change is straightforward. We can get rid of our `_cryptContext` object and replace any uses of it (two in model/user.py and one in the rest user.py). This will resolve the current issue with passlib, and won't affect current salts, etc. The OTP requires a bit more of a refactor, but we can switch to pyotp. I haven't investigated whether this can be done without any major change.
|
@zachmullen The changes to stop using bcrypt through passlib are pretty small. I could do that instead of pinning bcrypt. The work to refactor OTP is probably more, and would need a little investigation. If you think it would be better to stop using bcrypt through passlib right now rather than pinning bcrypt, I'll update this PR. |
|
If |
Stop using passlib for bcrypt. There is a slight change to how we speed up tests by using "plaintext" (this is mimicked from the passlib library, though much simpler). If we actually use bcrypt for everything, the tests take nearly twice as long.
|
So moving off of passlib is pretty straight-foward. I kept the I've tested the OTP code with Duo. None of the salts or codes have to change from the user's perspective. |
| # We seem to need a valid_window greater than zero -- in testing, | ||
| # Duo shows the otp that pyotp seems to want in the _next_ window; a | ||
| # value of 1 matches the default from the old passlib code | ||
| totpMatch = totp.verify(otpToken, valid_window=1) |
There was a problem hiding this comment.
It seems like ±1 is the best practice here so this is fine with me.
bcrypt 5.x makes a change that explicitly warns about truncating long passwords. passlib tests for a bug of some previous version by testing with a long password, which raises this exception.
We currently use passlib for bcrypt and for one-time-passwords (2fa). For the use of bcrypt, the change is straightforward. We can get rid of our
_cryptContextobject and replace any uses of it (two in model/user.py and one in the rest user.py). This will resolve the current issue with passlib, and won't affect current salts, etc.The OTP requires a bit more of a refactor, but we can switch to pyotp. I haven't investigated whether this can be done without any major change.