diff --git a/alchemiscale/security/auth.py b/alchemiscale/security/auth.py index f4782ea1..78c4ac0e 100644 --- a/alchemiscale/security/auth.py +++ b/alchemiscale/security/auth.py @@ -4,21 +4,68 @@ """ -from datetime import datetime, timedelta -from typing import Union, Optional import secrets +import base64 +import hashlib +from datetime import datetime, timedelta +from typing import Optional, Union +import bcrypt from fastapi import HTTPException, status from fastapi.security import OAuth2PasswordBearer from jose import JWTError, jwt -from passlib.context import CryptContext from pydantic import BaseModel -from .models import Token, TokenData, CredentialedEntity +from .models import CredentialedEntity, Token, TokenData + + +# we set a max size to avoid denial-of-service attacks +# since an extremely large secret attempted by an attacker can take +# increasing amounts of time or memory to validate; +# this is deliberately higher than any reasonable key length +# this is the same max size that `passlib` defaults to +MAX_SECRET_SIZE = 4096 +# Bcrypt truncates the secret at first NULL it encounters. For this reason, +# passlib forbids NULL bytes in the secret. This is not necessary for backwards +# compatibility, but we follow passlib's lead. +_BNULL = b"\x00" + + +class BcryptPasswordHandler(object): + + def __init__(self, rounds: int = 12, ident: str = "2b"): + self.rounds = rounds + self.ident = ident + + def hash(self, key: str) -> str: + validate_secret(key) + + # generate a salt unique to this key + salt = bcrypt.gensalt(rounds=self.rounds, prefix=self.ident.encode("ascii")) + hashed_salted = bcrypt.hashpw(key.encode("utf-8"), salt) + + return hashed_salted.decode("utf-8") + + def verify(self, key: str, hashed_salted: str) -> bool: + validate_secret(key) + + return bcrypt.checkpw(key.encode("utf-8"), hashed_salted.encode("utf-8")) -pwd_context = CryptContext(schemes=["bcrypt"], deprecated="auto") oauth2_scheme = OAuth2PasswordBearer(tokenUrl="token") +pwd_context = BcryptPasswordHandler() + + +def validate_secret(secret: str): + """ensure secret has correct type & size""" + if not isinstance(secret, str): + raise TypeError("secret must be a string") + if len(secret) > MAX_SECRET_SIZE: + raise ValueError( + f"secret is too long, maximum length is {MAX_SECRET_SIZE} characters" + ) + if _BNULL in secret.encode("utf-8"): + raise ValueError("secret contains NULL byte") def generate_secret_key(): diff --git a/alchemiscale/tests/unit/test_security.py b/alchemiscale/tests/unit/test_security.py index fca015e3..8e7050ca 100644 --- a/alchemiscale/tests/unit/test_security.py +++ b/alchemiscale/tests/unit/test_security.py @@ -30,3 +30,36 @@ def test_token_data(secret_key): token_data = auth.get_token_data(token=token, secret_key=secret_key) assert token_data.scopes == ["*-*-*"] + + +def test_bcrypt_password_handler(): + handler = auth.BcryptPasswordHandler() + hash_ = handler.hash("test") + assert handler.verify("test", hash_) + assert not handler.verify("deadbeef", hash_) + + +def test_bcrypt_against_passlib(): + """Test the our bcrypt handler has the same behavior as passlib did""" + + # pre-generated hash from + # `passlib.context.CryptContext(schemes=["bcrypt"], deprecated="auto").hash()` + test_password = "the quick brown fox jumps over the lazy dog" + test_hash = "$2b$12$QZTnjdx/sJS7jnEnCqhM3uS8mZ4mhLV5dDfM7ZBUT6LwDiNZ2p7S." + + # test that we get the same thing back from our bcrypt handler + handler = auth.BcryptPasswordHandler() + assert handler.verify(test_password, test_hash) + + +def test_bcrypt_against_passlib_long(): + """Test the our bcrypt handler has the same behavior as passlib did for passwords longer than 72 characters, in which bcrypt truncates""" + + # pre-generated hash from + # `passlib.context.CryptContext(schemes=["bcrypt"], deprecated="auto").hash()` + test_password = "this password is so long, it's longer than 72 characters; this should get truncated by bcrypt, so we can ensure we get the same verification behavior as passlib gives" + test_hash = "$2b$12$DQd5IPjlc8z4FZjBIdaquOlVc9whAqnkpZRsnuUUWvfHvwWy.dZ16" + + # test that we get the same thing back from our bcrypt handler + handler = auth.BcryptPasswordHandler() + assert handler.verify(test_password, test_hash) diff --git a/devtools/conda-envs/alchemiscale-server.yml b/devtools/conda-envs/alchemiscale-server.yml index 2a68451b..462361c1 100644 --- a/devtools/conda-envs/alchemiscale-server.yml +++ b/devtools/conda-envs/alchemiscale-server.yml @@ -29,7 +29,6 @@ dependencies: - uvicorn - gunicorn - python-jose - - passlib - bcrypt - python-multipart=0.0.12 # temporarily pinned due to broken 0.14 release on conda-forge - starlette diff --git a/devtools/conda-envs/test.yml b/devtools/conda-envs/test.yml index b4853df6..37021327 100644 --- a/devtools/conda-envs/test.yml +++ b/devtools/conda-envs/test.yml @@ -18,7 +18,7 @@ dependencies: - monotonic - docker-py # for grolt - # user client printing + # user client printing - rich ## object store @@ -29,7 +29,6 @@ dependencies: - uvicorn - gunicorn - python-jose - - passlib - bcrypt - starlette - httpx diff --git a/docs/conf.py b/docs/conf.py index 895c4f15..fdf43e2b 100644 --- a/docs/conf.py +++ b/docs/conf.py @@ -43,7 +43,6 @@ "jose", "networkx", "numpy", - "passlib", "py2neo", "pydantic", "starlette",