From 1717121f10d10b9d123c5c2ba95e3a9828ec4d1b Mon Sep 17 00:00:00 2001 From: Matthias Date: Sun, 27 Dec 2020 15:24:49 +0100 Subject: [PATCH] Properly use JWT secret key --- freqtrade/rpc/api_server2/api_auth.py | 43 ++++++++++++++------------ freqtrade/rpc/api_server2/deps.py | 4 +++ freqtrade/rpc/api_server2/webserver.py | 5 +++ tests/rpc/test_rpc_apiserver.py | 18 ++++++----- 4 files changed, 42 insertions(+), 28 deletions(-) diff --git a/freqtrade/rpc/api_server2/api_auth.py b/freqtrade/rpc/api_server2/api_auth.py index 595107acb..00ae60ed2 100644 --- a/freqtrade/rpc/api_server2/api_auth.py +++ b/freqtrade/rpc/api_server2/api_auth.py @@ -8,34 +8,32 @@ from fastapi.security.http import HTTPBasic, HTTPBasicCredentials from freqtrade.rpc.api_server2.api_models import AccessAndRefreshToken, AccessToken -from .deps import get_config +from .deps import get_api_config -SECRET_KEY = "09d25e094faa6ca2556c818166b7a9563b93f7099f6f0f4caa6cf63b88e8d3e7" ALGORITHM = "HS256" -ACCESS_TOKEN_EXPIRE_MINUTES = 30 router_login = APIRouter() -def verify_auth(config, username: str, password: str): +def verify_auth(api_config, username: str, password: str): """Verify username/password""" - return (secrets.compare_digest(username, config['api_server'].get('username')) and - secrets.compare_digest(password, config['api_server'].get('password'))) + return (secrets.compare_digest(username, api_config.get('username')) and + secrets.compare_digest(password, api_config.get('password'))) httpbasic = HTTPBasic(auto_error=False) oauth2_scheme = OAuth2PasswordBearer(tokenUrl="token", auto_error=False) -def get_user_from_token(token, token_type: str = "access"): +def get_user_from_token(token, secret_key: str, token_type: str = "access"): credentials_exception = HTTPException( status_code=status.HTTP_401_UNAUTHORIZED, detail="Could not validate credentials", headers={"WWW-Authenticate": "Bearer"}, ) try: - payload = jwt.decode(token, SECRET_KEY, algorithms=[ALGORITHM]) + payload = jwt.decode(token, secret_key, algorithms=[ALGORITHM]) username: str = payload.get("sub") if username is None: raise credentials_exception @@ -47,7 +45,7 @@ def get_user_from_token(token, token_type: str = "access"): return username -def create_token(data: dict, token_type: str = "access") -> str: +def create_token(data: dict, secret_key: str, token_type: str = "access") -> str: to_encode = data.copy() if token_type == "access": expire = datetime.utcnow() + timedelta(minutes=15) @@ -60,15 +58,16 @@ def create_token(data: dict, token_type: str = "access") -> str: "iat": datetime.utcnow(), "type": token_type, }) - encoded_jwt = jwt.encode(to_encode, SECRET_KEY, algorithm=ALGORITHM) + encoded_jwt = jwt.encode(to_encode, secret_key, algorithm=ALGORITHM) return encoded_jwt def http_basic_or_jwt_token(form_data: HTTPBasicCredentials = Depends(httpbasic), - token: str = Depends(oauth2_scheme), config=Depends(get_config)): + token: str = Depends(oauth2_scheme), + api_config=Depends(get_api_config)): if token: - return get_user_from_token(token) - elif form_data and verify_auth(config, form_data.username, form_data.password): + return get_user_from_token(token, api_config.get('jwt_secret_key', 'super-secret')) + elif form_data and verify_auth(api_config, form_data.username, form_data.password): return form_data.username raise HTTPException( @@ -78,12 +77,14 @@ def http_basic_or_jwt_token(form_data: HTTPBasicCredentials = Depends(httpbasic) @router_login.post('/token/login', response_model=AccessAndRefreshToken) -def token_login(form_data: HTTPBasicCredentials = Depends(HTTPBasic()), config=Depends(get_config)): +def token_login(form_data: HTTPBasicCredentials = Depends(HTTPBasic()), + api_config=Depends(get_api_config)): - if verify_auth(config, form_data.username, form_data.password): + if verify_auth(api_config, form_data.username, form_data.password): token_data = {'sub': form_data.username} - access_token = create_token(token_data) - refresh_token = create_token(token_data, token_type="refresh") + access_token = create_token(token_data, api_config.get('jwt_secret_key', 'super-secret')) + refresh_token = create_token(token_data, api_config.get('jwt_secret_key', 'super-secret'), + token_type="refresh") return { "access_token": access_token, "refresh_token": refresh_token, @@ -97,9 +98,11 @@ def token_login(form_data: HTTPBasicCredentials = Depends(HTTPBasic()), config=D @router_login.post('/token/refresh', response_model=AccessToken) -def token_refresh(token: str = Depends(oauth2_scheme)): +def token_refresh(token: str = Depends(oauth2_scheme), api_config=Depends(get_api_config)): # Refresh token - u = get_user_from_token(token, 'refresh') + u = get_user_from_token(token, api_config.get( + 'jwt_secret_key', 'super-secret'), 'refresh') token_data = {'sub': u} - access_token = create_token(token_data, token_type="access") + access_token = create_token(token_data, api_config.get('jwt_secret_key', 'super-secret'), + token_type="access") return {'access_token': access_token} diff --git a/freqtrade/rpc/api_server2/deps.py b/freqtrade/rpc/api_server2/deps.py index 60cc6b8fb..691759012 100644 --- a/freqtrade/rpc/api_server2/deps.py +++ b/freqtrade/rpc/api_server2/deps.py @@ -7,3 +7,7 @@ def get_rpc(): def get_config(): return ApiServer._config + + +def get_api_config(): + return ApiServer._config['api_server'] diff --git a/freqtrade/rpc/api_server2/webserver.py b/freqtrade/rpc/api_server2/webserver.py index 793e581ce..3956e52db 100644 --- a/freqtrade/rpc/api_server2/webserver.py +++ b/freqtrade/rpc/api_server2/webserver.py @@ -86,6 +86,11 @@ class ApiServer(RPCHandler): logger.warning("SECURITY WARNING - No password for local REST Server defined. " "Please make sure that this is intentional!") + if (self._config['api_server'].get('jwt_secret_key', 'super-secret') + in ('super-secret, somethingrandom')): + logger.warning("SECURITY WARNING - `jwt_secret_key` seems to be default." + "Others may be able to log into your bot.") + logger.info('Starting Local Rest Server.') uvconfig = uvicorn.Config(self.app, port=rest_port, diff --git a/tests/rpc/test_rpc_apiserver.py b/tests/rpc/test_rpc_apiserver.py index ee8976741..95789a85f 100644 --- a/tests/rpc/test_rpc_apiserver.py +++ b/tests/rpc/test_rpc_apiserver.py @@ -23,7 +23,7 @@ from freqtrade.persistence import PairLocks, Trade from freqtrade.rpc import RPC from freqtrade.rpc.api_server2 import ApiServer from freqtrade.state import RunMode, State -from tests.conftest import create_mock_trades, get_patched_freqtradebot, log_has, patch_get_signal +from tests.conftest import create_mock_trades, get_patched_freqtradebot, log_has, log_has_re, patch_get_signal BASE_URI = "/api/v1" @@ -91,22 +91,22 @@ def test_api_not_found(botclient): def test_api_auth(): with pytest.raises(ValueError): - create_token({'sub': 'Freqtrade'}, token_type="NotATokenType") + create_token({'sub': 'Freqtrade'}, 'secret1234', token_type="NotATokenType") - token = create_token({'sub': 'Freqtrade'}, ) + token = create_token({'sub': 'Freqtrade'}, 'secret1234') assert isinstance(token, bytes) - u = get_user_from_token(token) + u = get_user_from_token(token, 'secret1234') assert u == 'Freqtrade' with pytest.raises(HTTPException): - get_user_from_token(token, token_type='refresh') + get_user_from_token(token, 'secret1234', token_type='refresh') # Create invalid token - token = create_token({'sub`': 'Freqtrade'}, ) + token = create_token({'sub`': 'Freqtrade'}, 'secret1234') with pytest.raises(HTTPException): - get_user_from_token(token) + get_user_from_token(token, 'secret1234') with pytest.raises(HTTPException): - get_user_from_token(b'not_a_token') + get_user_from_token(b'not_a_token', 'secret1234') def test_api_unauthorized(botclient): @@ -279,6 +279,8 @@ def test_api_run(default_conf, mocker, caplog): "e.g 127.0.0.1 in config.json", caplog) assert log_has("SECURITY WARNING - No password for local REST Server defined. " "Please make sure that this is intentional!", caplog) + assert log_has_re("SECURITY WARNING - `jwt_secret_key` seems to be default.*", caplog) + # Test crashing flask caplog.clear()