Replace passlib with bcrypt

[LilDojd]

Summary:

This pull request replaces the usage of the passlib library with bcrypt for password hashing and verification.

Changes:

• remove passlib from conda environments and auth.py
• Added BcryptPasswordHandler with same defaults as in passlib.context.CryptContext

See #304 for motivation

[pep8speaks]

Hello @LilDojd! Thanks for opening this PR. We checked the lines you've touched for PEP 8 issues, and found:

• In the file alchemiscale/security/auth.py:

Line 37:63: E203 whitespace before ':' Line 41:80: E501 line too long (82 > 79 characters) Line 72:80: E501 line too long (83 > 79 characters)

[codecov[bot]]

Codecov Report

Attention: Patch coverage is 89.28571% with 3 lines in your changes missing coverage. Please review.

Project coverage is 80.21%. Comparing base (49182dc) to head (45c8418). Report is 180 commits behind head on main.

Files with missing lines	Patch %	Lines
alchemiscale/security/auth.py	89.28%	3 Missing :warning:

Additional details and impacted files

@@            Coverage Diff             @@
##             main     #306      +/-   ##
==========================================
+ Coverage   80.16%   80.21%   +0.04%     
==========================================
  Files          27       27              
  Lines        3484     3507      +23     
==========================================
+ Hits         2793     2813      +20     
- Misses        691      694       +3     

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

[dotsdl]

@LilDojd thanks again for doing this!

I simplified the implementation to avoid unnecessary state in the BcryptPasswordHandler, along with comments on the rationale for various bits. Let me know what you think of these changes.

We want any changes made here to allow existing alchemiscale deployments with existing identities/keys to still work, and I think this accomplishes that! Please do take a skeptical eye to it though. If it looks good to you, I think we're good to merge.

[LilDojd]

Thank you for your edits, it looks much better now!

I have ran property testing with hypothesis and it revealed a caveat. Here is the script https://gist.github.com/LilDojd/80931d429411320cfe28c5126ccddc54

From the passlib docs:

While not a security issue per-se, bcrypt does have one major limitation: password are truncated on the first NULL byte (if any), and only the first 72 bytes of a password are hashed… all the rest are ignored. Furthermore, bytes 55-72 are not fully mixed into the resulting hash (citation needed!). To work around both these issues, many applications first run the password through a message digest such as (HMAC-) SHA2-256. Passlib offers the premade passlib.hash.bcrypt_sha256 - BCrypt+SHA256 to take care of this issue.

But in the code it forbids nullbytes altogether:

    @classmethod
    def _norm_digest_args(cls, secret, ident, new=False):
		
		...

        # NOTE: especially important to forbid NULLs for bcrypt, since many
        # backends (bcryptor, bcrypt) happily accept them, and then
        # silently truncate the password at first NULL they encounter!
        if _BNULL in secret:
>           raise uh.exc.NullPasswordError(cls)

I suggest we add a similar check. Otherwise, from my testing, functionally nothing has changed and we should be good to go with this

[dotsdl]

Excellent, thank you for that sanity check @LilDojd, as well as the NULL check! I'll merge shortly once tests pass!