From 930c2512c16ef7a106c6daa1150e6b348ef2b6f3 Mon Sep 17 00:00:00 2001 From: Alexander Haas Date: Mon, 30 May 2022 12:22:03 +0200 Subject: [PATCH 01/94] chore: cleanup --- tardis/rest/app/security.py | 13 ++++++++----- 1 file changed, 8 insertions(+), 5 deletions(-) diff --git a/tardis/rest/app/security.py b/tardis/rest/app/security.py index 03e5318f..0b6f4992 100644 --- a/tardis/rest/app/security.py +++ b/tardis/rest/app/security.py @@ -64,7 +64,8 @@ def check_authorization( authenticate_value = "Bearer" try: - payload = jwt.decode(token, get_secret_key(), algorithms=[get_algorithm()]) + payload = jwt.decode(token, get_secret_key(), + algorithms=[get_algorithm()]) user_name: str = payload.get("sub") token_scopes = payload.get("scopes", []) token_data = TokenData(scopes=token_scopes, user_name=user_name) @@ -78,7 +79,7 @@ def check_authorization( for scope in security_scopes.scopes: if scope not in token_data.scopes: raise HTTPException( - status_code=status.HTTP_401_UNAUTHORIZED, + status_code=status.HTTP_403_FORBIDDEN, detail="Not enough permissions", headers={"WWW-Authenticate": authenticate_value}, ) from None @@ -89,11 +90,13 @@ def check_authorization( def check_authentication(user_name: str, password: str) -> UserCredentials: user = get_user(user_name) if not user: - raise HTTPException(status_code=400, detail="Incorrect username or password") + raise HTTPException( + status_code=status.HTTP_401_UNAUTHORIZED, detail="Incorrect username or password") if checkpw(password.encode(), user.hashed_password.encode()): return user else: - raise HTTPException(status_code=400, detail="Incorrect username or password") + raise HTTPException( + status_code=status.HTTP_401_UNAUTHORIZED, detail="Incorrect username or password") @lru_cache(maxsize=1) @@ -121,7 +124,7 @@ def get_secret_key() -> str: @lru_cache(maxsize=16) -def get_user(user_name: str) -> [None, UserCredentials]: +def get_user(user_name: str) -> Optional[UserCredentials]: try: rest_service = Configuration().Services.restapi except AttributeError: From 9201f2c9dc34df0a6ec3df33df5195ed65f9291d Mon Sep 17 00:00:00 2001 From: Alexander Haas Date: Mon, 30 May 2022 13:01:25 +0200 Subject: [PATCH 02/94] dependecy: fastapi-jwt-auth --- setup.py | 1 + 1 file changed, 1 insertion(+) diff --git a/setup.py b/setup.py index 9f84530c..0dceeacf 100644 --- a/setup.py +++ b/setup.py @@ -91,6 +91,7 @@ def get_cryptography_version(): "python-multipart", "typing_extensions", "backports.cached_property", + "fastapi-jwt-auth" ], extras_require={ "docs": [ From b87f5eed3aea526b775fe914e189a491f6622cc8 Mon Sep 17 00:00:00 2001 From: Alexander Haas Date: Mon, 30 May 2022 13:20:27 +0200 Subject: [PATCH 03/94] chore: replace static key from config with automatically generated one --- tardis/rest/service.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/tardis/rest/service.py b/tardis/rest/service.py index 2cb7973f..51d629e4 100644 --- a/tardis/rest/service.py +++ b/tardis/rest/service.py @@ -1,3 +1,4 @@ +import secrets from .app.security import UserCredentials from cobald.daemon import service from cobald.daemon.plugins import yaml_tag @@ -15,13 +16,12 @@ class RestService(object): def __init__( self, - secret_key: str, algorithm: str = "HS256", users: List = None, **fast_api_args, ): self._algorithm = algorithm - self._secret_key = secret_key + self._secret_key: str = secrets.token_hex(128) self._users = users or [] # necessary to avoid that the TARDIS' logger configuration is overwritten! From f2576040df121396b411b19643a7957a6dc99540 Mon Sep 17 00:00:00 2001 From: Alexander Haas Date: Mon, 30 May 2022 14:31:47 +0200 Subject: [PATCH 04/94] chore: introduce new User classes --- tardis/rest/app/security.py | 19 +++++++++---------- tardis/rest/service.py | 6 +++--- 2 files changed, 12 insertions(+), 13 deletions(-) diff --git a/tardis/rest/app/security.py b/tardis/rest/app/security.py index 0b6f4992..679738f8 100644 --- a/tardis/rest/app/security.py +++ b/tardis/rest/app/security.py @@ -12,18 +12,17 @@ from typing import List, Optional -class TokenData(BaseModel): - user_name: Optional[str] = None +class BaseUser(BaseModel): + user_name: str scopes: List[str] = [] -class UserCredentials(BaseModel): - user_name: str - hashed_password: str - scopes: List[str] +class LoginUser(BaseUser): + password: str - class Config: - extra = "forbid" + +class DatabaseUser(BaseUser): + hashed_password: str oauth2_scheme = OAuth2PasswordBearer( @@ -87,7 +86,7 @@ def check_authorization( return token_data -def check_authentication(user_name: str, password: str) -> UserCredentials: +def check_authentication(user_name: str, password: str) -> DatabaseUser: user = get_user(user_name) if not user: raise HTTPException( @@ -124,7 +123,7 @@ def get_secret_key() -> str: @lru_cache(maxsize=16) -def get_user(user_name: str) -> Optional[UserCredentials]: +def get_user(user_name: str) -> Optional[DatabaseUser]: try: rest_service = Configuration().Services.restapi except AttributeError: diff --git a/tardis/rest/service.py b/tardis/rest/service.py index 51d629e4..609ef6c1 100644 --- a/tardis/rest/service.py +++ b/tardis/rest/service.py @@ -1,5 +1,5 @@ import secrets -from .app.security import UserCredentials +from .app.security import DatabaseUser from cobald.daemon import service from cobald.daemon.plugins import yaml_tag @@ -40,10 +40,10 @@ def secret_key(self) -> str: return self._secret_key @lru_cache(maxsize=16) - def get_user(self, user_name: str) -> Optional[UserCredentials]: + def get_user(self, user_name: str) -> Optional[DatabaseUser]: for user in self._users: if user["user_name"] == user_name: - return UserCredentials(**user) + return DatabaseUser(**user) return None async def run(self) -> None: From 24a52e48030f434a38b594b5e93eb37a06f5d433 Mon Sep 17 00:00:00 2001 From: Alexander Haas Date: Mon, 30 May 2022 16:27:16 +0200 Subject: [PATCH 05/94] chore: secret key gets generated at runtime --- tardis/rest/app/security.py | 2 ++ tardis/rest/service.py | 6 ------ 2 files changed, 2 insertions(+), 6 deletions(-) diff --git a/tardis/rest/app/security.py b/tardis/rest/app/security.py index 679738f8..61626e41 100644 --- a/tardis/rest/app/security.py +++ b/tardis/rest/app/security.py @@ -12,6 +12,8 @@ from typing import List, Optional +class Settings(BaseModel): + authjwt_secret_key: str = secrets.token_hex(128) class BaseUser(BaseModel): user_name: str scopes: List[str] = [] diff --git a/tardis/rest/service.py b/tardis/rest/service.py index 609ef6c1..9707a489 100644 --- a/tardis/rest/service.py +++ b/tardis/rest/service.py @@ -21,7 +21,6 @@ def __init__( **fast_api_args, ): self._algorithm = algorithm - self._secret_key: str = secrets.token_hex(128) self._users = users or [] # necessary to avoid that the TARDIS' logger configuration is overwritten! @@ -34,11 +33,6 @@ def __init__( def algorithm(self) -> str: return self._algorithm - @property - @lru_cache(maxsize=16) - def secret_key(self) -> str: - return self._secret_key - @lru_cache(maxsize=16) def get_user(self, user_name: str) -> Optional[DatabaseUser]: for user in self._users: From 0dcfc0e07cb115da0f65dfd7ab8cbab512b46f77 Mon Sep 17 00:00:00 2001 From: Alexander Haas Date: Mon, 30 May 2022 16:29:10 +0200 Subject: [PATCH 06/94] feat: Auth by jwt in cookie + name change of route /login --- tardis/rest/app/main.py | 20 ++++- tardis/rest/app/routers/login.py | 18 ---- tardis/rest/app/routers/resources.py | 12 ++- tardis/rest/app/routers/user.py | 23 +++++ tardis/rest/app/security.py | 128 ++++++++++++--------------- 5 files changed, 103 insertions(+), 98 deletions(-) delete mode 100644 tardis/rest/app/routers/login.py create mode 100644 tardis/rest/app/routers/user.py diff --git a/tardis/rest/app/main.py b/tardis/rest/app/main.py index 88729cfc..7f618440 100644 --- a/tardis/rest/app/main.py +++ b/tardis/rest/app/main.py @@ -1,5 +1,8 @@ from ...__about__ import __version__ -from .routers import login, resources +from .routers import resources, user +from fastapi_jwt_auth.exceptions import AuthJWTException +from fastapi import Request +from fastapi.responses import JSONResponse from fastapi import FastAPI @@ -22,11 +25,20 @@ "description": "Information about the currently managed resources.", }, { - "name": "login", - "description": "Handles login and creation of limited duration tokens to access APIs.", # noqa B509 + "name": "user", + "description": "Handles login, refresh tokens, logout and anything related to the user.", # noqa B509 }, ], ) + +@app.exception_handler(AuthJWTException) +def authjwt_exception_handler(request: Request, exc: AuthJWTException): + return JSONResponse( + status_code=exc.status_code, + content={"detail": exc.message} + ) + + app.include_router(resources.router) -app.include_router(login.router) +app.include_router(user.router) diff --git a/tardis/rest/app/routers/login.py b/tardis/rest/app/routers/login.py deleted file mode 100644 index e8ca7e9c..00000000 --- a/tardis/rest/app/routers/login.py +++ /dev/null @@ -1,18 +0,0 @@ -from .. import security -from fastapi import APIRouter, Depends -from fastapi.security import OAuth2PasswordRequestForm - -from datetime import timedelta - -router = APIRouter(prefix="/login", tags=["login"]) - - -@router.post("/access-token", description="Get a limited duration access token") -async def login(form_data: OAuth2PasswordRequestForm = Depends()): - user = security.check_authentication(form_data.username, form_data.password) - return { - "access_token": security.create_access_token( - user.user_name, user.scopes, expires_delta=timedelta(days=1) - ), - "token_type": "bearer", - } diff --git a/tardis/rest/app/routers/resources.py b/tardis/rest/app/routers/resources.py index 8d764699..fa335497 100644 --- a/tardis/rest/app/routers/resources.py +++ b/tardis/rest/app/routers/resources.py @@ -1,7 +1,7 @@ from .. import security, crud, database from ....plugins.sqliteregistry import SqliteRegistry from fastapi import APIRouter, Depends, HTTPException, Path, Security - +from fastapi_jwt_auth import AuthJWT router = APIRouter(prefix="/resources", tags=["resources"]) @@ -10,8 +10,11 @@ async def get_resource_state( drone_uuid: str = Path(..., regex=r"^\S+-[A-Fa-f0-9]{10}$"), sql_registry: SqliteRegistry = Depends(database.get_sql_registry()), - _: str = Security(security.check_authorization, scopes=["resources:get"]), + Authorize: AuthJWT = Depends() + # _: str = Security(security.check_authorization, scopes=["resources:get"]), ): + Authorize.jwt_required() + query_result = await crud.get_resource_state(sql_registry, drone_uuid) try: query_result = query_result[0] @@ -23,7 +26,10 @@ async def get_resource_state( @router.get("/", description="Get list of managed resources") async def get_resources( sql_registry: SqliteRegistry = Depends(database.get_sql_registry()), - _: str = Security(security.check_authorization, scopes=["resources:get"]), + Authorize: AuthJWT = Depends() + # _: str = Security(security.check_authorization, scopes=["resources:get"]), ): + Authorize.jwt_required() + query_result = await crud.get_resources(sql_registry) return query_result diff --git a/tardis/rest/app/routers/user.py b/tardis/rest/app/routers/user.py new file mode 100644 index 00000000..6fa1d665 --- /dev/null +++ b/tardis/rest/app/routers/user.py @@ -0,0 +1,23 @@ +from .. import security +from fastapi import APIRouter, Depends +from fastapi_jwt_auth import AuthJWT + +router = APIRouter(prefix="/user", tags=["user"]) + +# TODO: Very important: Set the csrf cookie in the frontend. + + +@router.post("/login", description="Sets httponly access token in session cookie") +async def login(login_user: security.LoginUser, Authorize: AuthJWT = Depends()): + user = security.check_authentication( + login_user.user_name, login_user.password) + + scopes = {"scopes": user.scopes} + access_token = Authorize.create_access_token( + subject=user.user_name, user_claims=scopes) + refresh_token = Authorize.create_refresh_token(subject=user.user_name) + + Authorize.set_access_cookies(access_token) + Authorize.set_refresh_cookies(refresh_token) + + return {"msg": "Successfully logged in!"} diff --git a/tardis/rest/app/security.py b/tardis/rest/app/security.py index 61626e41..78e88a98 100644 --- a/tardis/rest/app/security.py +++ b/tardis/rest/app/security.py @@ -1,11 +1,13 @@ +import secrets from ...configuration.configuration import Configuration from ...exceptions.tardisexceptions import TardisError from bcrypt import checkpw, gensalt, hashpw from fastapi import Depends, HTTPException, status -from fastapi.security import OAuth2PasswordBearer, SecurityScopes +from fastapi.security import SecurityScopes from jose import JWTError, jwt -from pydantic import BaseModel, ValidationError +from pydantic import BaseModel, Field, ValidationError +from fastapi_jwt_auth import AuthJWT from datetime import datetime, timedelta from functools import lru_cache @@ -14,10 +16,27 @@ class Settings(BaseModel): authjwt_secret_key: str = secrets.token_hex(128) + authjwt_token_location: set = {"cookies"} + authjwt_cookie_samesite: str = "strict" + # TODO: change this to true in production so only https traffic is allowed + authjwt_cookie_secure: bool = False + # TODO: Set this too to True. But as soon as possible. Only disabled temporarily + authjwt_cookie_csrf_protect: bool = False + + +@AuthJWT.load_config +def get_config(): + return Settings() + + class BaseUser(BaseModel): user_name: str scopes: List[str] = [] +# TODO: Document the scopes manually +# "resources:get": "Allows to read resource database", +# "resources:put": "Allows to update resource database.", + class LoginUser(BaseUser): password: str @@ -27,72 +46,46 @@ class DatabaseUser(BaseUser): hashed_password: str -oauth2_scheme = OAuth2PasswordBearer( - tokenUrl="login/access-token", - scopes={ - "resources:get": "Allows to read resource database", - "resources:put": "Allows to update resource database.", - }, -) - - -def create_access_token( - user_name: str, - scopes: List[str], - expires_delta: Optional[timedelta] = None, - secret_key: Optional[str] = None, - algorithm: Optional[str] = None, -) -> str: - to_encode = {"sub": user_name, "scopes": scopes} - if expires_delta: - expire = datetime.utcnow() + expires_delta - to_encode.update({"exp": expire}) - encoded_jwt = jwt.encode( - to_encode, - secret_key or get_secret_key(), - algorithm=algorithm or get_algorithm(), - ) - - return encoded_jwt - - -def check_authorization( - security_scopes: SecurityScopes, token: str = Depends(oauth2_scheme) -) -> TokenData: - if security_scopes.scopes: - authenticate_value = f'Bearer scope="{security_scopes.scope_str}"' - else: - authenticate_value = "Bearer" - - try: - payload = jwt.decode(token, get_secret_key(), - algorithms=[get_algorithm()]) - user_name: str = payload.get("sub") - token_scopes = payload.get("scopes", []) - token_data = TokenData(scopes=token_scopes, user_name=user_name) - except (JWTError, ValidationError) as err: - raise HTTPException( - status_code=status.HTTP_401_UNAUTHORIZED, - detail="Could not validate credentials", - headers={"WWW-Authenticate": authenticate_value}, - ) from err - - for scope in security_scopes.scopes: - if scope not in token_data.scopes: - raise HTTPException( - status_code=status.HTTP_403_FORBIDDEN, - detail="Not enough permissions", - headers={"WWW-Authenticate": authenticate_value}, - ) from None - - return token_data +# TODO: Implement scopes properly when needed +# def check_authorization( +# security_scopes: SecurityScopes, token: str = Depends(oauth2_scheme) +# ) -> TokenData: +# if security_scopes.scopes: +# authenticate_value = f'Bearer scope="{security_scopes.scope_str}"' +# else: +# authenticate_value = "Bearer" + +# try: +# payload = jwt.decode(token, get_secret_key(), +# algorithms=[get_algorithm()]) +# user_name: str = payload.get("sub") +# token_scopes = payload.get("scopes", []) +# token_data = TokenData(scopes=token_scopes, user_name=user_name) +# except (JWTError, ValidationError) as err: +# raise HTTPException( +# status_code=status.HTTP_401_UNAUTHORIZED, +# detail="Could not validate credentials", +# headers={"WWW-Authenticate": authenticate_value}, +# ) from err + +# for scope in security_scopes.scopes: +# if scope not in token_data.scopes: +# raise HTTPException( +# status_code=status.HTTP_403_FORBIDDEN, +# detail="Not enough permissions", +# headers={"WWW-Authenticate": authenticate_value}, +# ) from None + +# return token_data def check_authentication(user_name: str, password: str) -> DatabaseUser: user = get_user(user_name) + if not user: raise HTTPException( status_code=status.HTTP_401_UNAUTHORIZED, detail="Incorrect username or password") + if checkpw(password.encode(), user.hashed_password.encode()): return user else: @@ -102,6 +95,7 @@ def check_authentication(user_name: str, password: str) -> DatabaseUser: @lru_cache(maxsize=1) def get_algorithm() -> str: + # This can probably be deprecated as fastapi_jwt_tokens sets it's algorithm by itself try: rest_service = Configuration().Services.restapi except AttributeError: @@ -112,18 +106,6 @@ def get_algorithm() -> str: return rest_service.algorithm -@lru_cache(maxsize=1) -def get_secret_key() -> str: - try: - rest_service = Configuration().Services.restapi - except AttributeError: - raise TardisError( - "TARDIS RestService not configured while accessing secret_key!" - ) from None - else: - return rest_service.secret_key - - @lru_cache(maxsize=16) def get_user(user_name: str) -> Optional[DatabaseUser]: try: From c43aceb486d121556fd8607bfb6996e919c8c939 Mon Sep 17 00:00:00 2001 From: Alexander Haas Date: Mon, 30 May 2022 16:29:28 +0200 Subject: [PATCH 07/94] feat: More user functions --- tardis/rest/app/routers/user.py | 27 +++++++++++++++++++++++++++ 1 file changed, 27 insertions(+) diff --git a/tardis/rest/app/routers/user.py b/tardis/rest/app/routers/user.py index 6fa1d665..640f03a5 100644 --- a/tardis/rest/app/routers/user.py +++ b/tardis/rest/app/routers/user.py @@ -21,3 +21,30 @@ async def login(login_user: security.LoginUser, Authorize: AuthJWT = Depends()): Authorize.set_refresh_cookies(refresh_token) return {"msg": "Successfully logged in!"} + + +@router.delete('/logout') +async def logout(Authorize: AuthJWT = Depends()): + Authorize.jwt_required() + + Authorize.unset_jwt_cookies() + return {"msg": "Successfully logged out!"} + + +@router.post('/refresh') +async def refresh(Authorize: AuthJWT = Depends()): + Authorize.jwt_refresh_token_required() + + current_user = Authorize.get_jwt_subject() + new_access_token = Authorize.create_access_token(subject=current_user) + + Authorize.set_access_cookies(new_access_token) + return {"msg": "Token successfully refreshed"} + + +@router.get("/me", response_model=security.BaseUser) +async def get_user_me(Authorize: AuthJWT = Depends()): + Authorize.jwt_required() + + user_name = Authorize.get_jwt_subject() + return security.get_user(user_name) From 8aabd1403edcfbb128d30dc67b2e2d4986af9197 Mon Sep 17 00:00:00 2001 From: Alexander Haas Date: Tue, 31 May 2022 08:30:41 +0200 Subject: [PATCH 08/94] fix: reformat to satisfy flake8 --- tardis/rest/app/routers/resources.py | 14 ++++++++------ tardis/rest/app/security.py | 16 +++++++++------- tardis/rest/service.py | 1 - 3 files changed, 17 insertions(+), 14 deletions(-) diff --git a/tardis/rest/app/routers/resources.py b/tardis/rest/app/routers/resources.py index fa335497..7bc914b1 100644 --- a/tardis/rest/app/routers/resources.py +++ b/tardis/rest/app/routers/resources.py @@ -1,17 +1,18 @@ -from .. import security, crud, database +from .. import crud, database from ....plugins.sqliteregistry import SqliteRegistry -from fastapi import APIRouter, Depends, HTTPException, Path, Security +from fastapi import APIRouter, Depends, HTTPException, Path from fastapi_jwt_auth import AuthJWT router = APIRouter(prefix="/resources", tags=["resources"]) -@router.get("/{drone_uuid}/state", description="Get current state of a resource") +@router.get("/{drone_uuid}/state", + description="Get current state of a resource") async def get_resource_state( drone_uuid: str = Path(..., regex=r"^\S+-[A-Fa-f0-9]{10}$"), sql_registry: SqliteRegistry = Depends(database.get_sql_registry()), Authorize: AuthJWT = Depends() - # _: str = Security(security.check_authorization, scopes=["resources:get"]), + # _: str = Security(security.check_authorization, scopes=["resources:get"]) ): Authorize.jwt_required() @@ -19,7 +20,8 @@ async def get_resource_state( try: query_result = query_result[0] except IndexError: - raise HTTPException(status_code=404, detail="Drone not found") from None + raise HTTPException( + status_code=404, detail="Drone not found") from None return query_result @@ -27,7 +29,7 @@ async def get_resource_state( async def get_resources( sql_registry: SqliteRegistry = Depends(database.get_sql_registry()), Authorize: AuthJWT = Depends() - # _: str = Security(security.check_authorization, scopes=["resources:get"]), + # _: str = Security(security.check_authorization, scopes=["resources:get"]) ): Authorize.jwt_required() diff --git a/tardis/rest/app/security.py b/tardis/rest/app/security.py index 78e88a98..54e1640a 100644 --- a/tardis/rest/app/security.py +++ b/tardis/rest/app/security.py @@ -4,14 +4,14 @@ from bcrypt import checkpw, gensalt, hashpw from fastapi import Depends, HTTPException, status -from fastapi.security import SecurityScopes -from jose import JWTError, jwt -from pydantic import BaseModel, Field, ValidationError +# from fastapi.security import SecurityScopes +# from jose import JWTError, jwt +from pydantic import BaseModel # , Field, ValidationError from fastapi_jwt_auth import AuthJWT -from datetime import datetime, timedelta +# from datetime import datetime, timedelta from functools import lru_cache -from typing import List, Optional +from typing import Optional, List class Settings(BaseModel): @@ -84,13 +84,15 @@ def check_authentication(user_name: str, password: str) -> DatabaseUser: if not user: raise HTTPException( - status_code=status.HTTP_401_UNAUTHORIZED, detail="Incorrect username or password") + status_code=status.HTTP_401_UNAUTHORIZED, + detail="Incorrect username or password") if checkpw(password.encode(), user.hashed_password.encode()): return user else: raise HTTPException( - status_code=status.HTTP_401_UNAUTHORIZED, detail="Incorrect username or password") + status_code=status.HTTP_401_UNAUTHORIZED, + detail="Incorrect username or password") @lru_cache(maxsize=1) diff --git a/tardis/rest/service.py b/tardis/rest/service.py index 9707a489..1aaf7e3c 100644 --- a/tardis/rest/service.py +++ b/tardis/rest/service.py @@ -1,4 +1,3 @@ -import secrets from .app.security import DatabaseUser from cobald.daemon import service from cobald.daemon.plugins import yaml_tag From 5a7da54bfc6246f0151de0abcba40a0a9eaf01b6 Mon Sep 17 00:00:00 2001 From: Alexander Haas Date: Tue, 31 May 2022 08:34:35 +0200 Subject: [PATCH 09/94] fix: second try at satisfying static check --- tardis/rest/app/security.py | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/tardis/rest/app/security.py b/tardis/rest/app/security.py index 54e1640a..75ec2fbc 100644 --- a/tardis/rest/app/security.py +++ b/tardis/rest/app/security.py @@ -3,7 +3,7 @@ from ...exceptions.tardisexceptions import TardisError from bcrypt import checkpw, gensalt, hashpw -from fastapi import Depends, HTTPException, status +from fastapi import HTTPException, status # from fastapi.security import SecurityScopes # from jose import JWTError, jwt from pydantic import BaseModel # , Field, ValidationError @@ -20,7 +20,7 @@ class Settings(BaseModel): authjwt_cookie_samesite: str = "strict" # TODO: change this to true in production so only https traffic is allowed authjwt_cookie_secure: bool = False - # TODO: Set this too to True. But as soon as possible. Only disabled temporarily + # TODO: Set this too to True. But as soon as possible. authjwt_cookie_csrf_protect: bool = False @@ -97,7 +97,8 @@ def check_authentication(user_name: str, password: str) -> DatabaseUser: @lru_cache(maxsize=1) def get_algorithm() -> str: - # This can probably be deprecated as fastapi_jwt_tokens sets it's algorithm by itself + # This can probably be deprecated as fastapi_jwt_tokens + # sets it's algorithm by itself try: rest_service = Configuration().Services.restapi except AttributeError: From 11135efad31f080791831a18dcb38be728572dbf Mon Sep 17 00:00:00 2001 From: Alexander Haas Date: Tue, 31 May 2022 09:04:40 +0200 Subject: [PATCH 10/94] added myself as contributor --- CONTRIBUTORS | 1 + 1 file changed, 1 insertion(+) diff --git a/CONTRIBUTORS b/CONTRIBUTORS index 644497da..2077d417 100644 --- a/CONTRIBUTORS +++ b/CONTRIBUTORS @@ -17,3 +17,4 @@ Matthias Schnepf Peter Wienemann rfvc PSchuhmacher +Alexander Haas \ No newline at end of file From 6b2b905f8294984fe6e8594ebf197f147c79b5a8 Mon Sep 17 00:00:00 2001 From: Alexander Haas Date: Tue, 31 May 2022 09:44:12 +0200 Subject: [PATCH 11/94] fix: remove get secret key tests as it isn't necessary to read it manually anymore as it is automatically generated at runtime and then managed by fastapi_jwt_auth --- tests/rest_t/app_t/test_security.py | 23 +---------------------- 1 file changed, 1 insertion(+), 22 deletions(-) diff --git a/tests/rest_t/app_t/test_security.py b/tests/rest_t/app_t/test_security.py index ca6f48d3..0bc8c4bd 100644 --- a/tests/rest_t/app_t/test_security.py +++ b/tests/rest_t/app_t/test_security.py @@ -1,21 +1,15 @@ from tardis.exceptions.tardisexceptions import TardisError from tardis.rest.app.security import ( - create_access_token, - check_authorization, + # check_authorization, check_authentication, get_algorithm, - get_secret_key, get_user, hash_password, - TokenData, ) from tardis.utilities.attributedict import AttributeDict from fastapi import HTTPException, status -from fastapi.security import SecurityScopes -from jose import JWTError -from datetime import datetime, timedelta from unittest import TestCase from unittest.mock import patch @@ -33,9 +27,6 @@ def tearDownClass(cls) -> None: cls.mock_config_patcher.stop() def setUp(self) -> None: - self.secret_key = ( - "689e7af69a70ad0d97f771371738be00452e81e128a876491c1d373dfbcca949" - ) self.algorithm = "HS256" self.infinite_resources_get_token = "eyJhbGciOiJIUzI1NiIsInR5cCI6IkpXVCJ9.eyJzdWIiOiJ0ZXN0Iiwic2NvcGVzIjpbInJlc291cmNlczpnZXQiXX0.FTzUlLfPgb2WXFUSPSoUsvqHI67QtSO2Boash_6eVBg" # noqa B950 @@ -52,14 +43,12 @@ def mocked_get_user(user_name): return None config = self.mock_config.return_value - config.Services.restapi.secret_key = self.secret_key config.Services.restapi.algorithm = self.algorithm config.Services.restapi.get_user.side_effect = mocked_get_user @staticmethod def clear_lru_cache(): get_algorithm.cache_clear() - get_secret_key.cache_clear() get_user.cache_clear() @patch("tardis.rest.app.security.datetime") @@ -179,16 +168,6 @@ def test_get_algorithm(self): get_algorithm() self.mock_config.side_effect = None - def test_get_secret_key(self): - self.clear_lru_cache() - self.assertEqual(get_secret_key(), self.secret_key) - - self.clear_lru_cache() - self.mock_config.side_effect = AttributeError - with self.assertRaises(TardisError): - get_secret_key() - self.mock_config.side_effect = None - def test_get_user(self): self.clear_lru_cache() self.assertEqual( From 99534a39780bf186178ccdc2598817fe5ff3d767 Mon Sep 17 00:00:00 2001 From: Alexander Haas Date: Tue, 31 May 2022 09:49:03 +0200 Subject: [PATCH 12/94] fix: delete create_access_token tests as they are generated automatically by the jwt package + comment out the authorization tests as it isn't clear if authorization is needed for now --- tests/rest_t/app_t/test_security.py | 125 ++++++++++------------------ 1 file changed, 43 insertions(+), 82 deletions(-) diff --git a/tests/rest_t/app_t/test_security.py b/tests/rest_t/app_t/test_security.py index 0bc8c4bd..f2cbb107 100644 --- a/tests/rest_t/app_t/test_security.py +++ b/tests/rest_t/app_t/test_security.py @@ -51,88 +51,49 @@ def clear_lru_cache(): get_algorithm.cache_clear() get_user.cache_clear() - @patch("tardis.rest.app.security.datetime") - def test_create_access_token(self, mocked_datetime): - self.clear_lru_cache() - - token = create_access_token(user_name="test", scopes=["resources:get"]) - - self.assertEqual(token, self.infinite_resources_get_token) - - self.clear_lru_cache() - - token = create_access_token( - user_name="test", - scopes=["resources:get"], - secret_key="c2ac5e498f6287c58fa941d0d2cfaf2dc271762a7ba03dcfc3ceb91bb1895d05", # noqa B950 - algorithm=self.algorithm, - ) - - self.assertEqual( - token, - "eyJhbGciOiJIUzI1NiIsInR5cCI6IkpXVCJ9.eyJzdWIiOiJ0ZXN0Iiwic2NvcGVzIjpbInJlc291cmNlczpnZXQiXX0.bpEZ-l6yC4A7mHV1OjK6WUBeFrjCGMQ7kN9ElXGORe4", # noqa B950 - ) - - self.clear_lru_cache() - mocked_datetime.utcnow.return_value = datetime.utcfromtimestamp(0) - token = create_access_token( - user_name="test", - scopes=["resources:get"], - expires_delta=timedelta(minutes=15), - ) - - self.assertEqual(token, self.limited_resources_get_token) - - self.clear_lru_cache() - token = create_access_token( - user_name="test", scopes=["resources:get", "resources:put"] - ) - - self.assertEqual(token, self.infinite_resources_get_update_token) - - def test_check_authorization(self): - self.clear_lru_cache() - security_scopes = SecurityScopes(["resources:get"]) - token_data = check_authorization( - security_scopes, self.infinite_resources_get_token - ) - - self.assertEqual( - token_data, TokenData(scopes=security_scopes.scopes, user_name="test") - ) - - security_scopes = SecurityScopes(["resources:put"]) - with self.assertRaises(HTTPException) as he: - check_authorization(security_scopes, self.infinite_resources_get_token) - self.assertEqual(he.exception.status_code, status.HTTP_401_UNAUTHORIZED) - self.assertEqual(he.exception.detail, "Not enough permissions") - - token_data = check_authorization( - security_scopes, self.infinite_resources_get_update_token - ) - self.assertEqual( - token_data, - TokenData(scopes=["resources:get", "resources:put"], user_name="test"), - ) - - security_scopes = SecurityScopes() - check_authorization(security_scopes, self.infinite_resources_get_token) - - with self.assertRaises(HTTPException) as he: - check_authorization(security_scopes, "1234567890abdcef") - self.assertEqual(he.exception.status_code, status.HTTP_401_UNAUTHORIZED) - self.assertEqual(he.exception.detail, "Could not validate credentials") - - @patch("tardis.rest.app.security.jwt") - def test_check_authorization_jwt_error(self, mocked_jwt): - mocked_jwt.decode.side_effect = JWTError - - with self.assertRaises(HTTPException) as he: - check_authorization(SecurityScopes(), self.infinite_resources_get_token) - self.assertEqual(he.exception.status_code, status.HTTP_401_UNAUTHORIZED) - self.assertEqual(he.exception.detail, "Could not validate credentials") - - mocked_jwt.decode.side_effect = None + # def test_check_authorization(self): + # self.clear_lru_cache() + # security_scopes = SecurityScopes(["resources:get"]) + # token_data = check_authorization( + # security_scopes, self.infinite_resources_get_token + # ) + + # self.assertEqual( + # token_data, TokenData(scopes=security_scopes.scopes, user_name="test") + # ) + + # security_scopes = SecurityScopes(["resources:put"]) + # with self.assertRaises(HTTPException) as he: + # check_authorization(security_scopes, self.infinite_resources_get_token) + # self.assertEqual(he.exception.status_code, status.HTTP_401_UNAUTHORIZED) + # self.assertEqual(he.exception.detail, "Not enough permissions") + + # token_data = check_authorization( + # security_scopes, self.infinite_resources_get_update_token + # ) + # self.assertEqual( + # token_data, + # TokenData(scopes=["resources:get", "resources:put"], user_name="test"), + # ) + + # security_scopes = SecurityScopes() + # check_authorization(security_scopes, self.infinite_resources_get_token) + + # with self.assertRaises(HTTPException) as he: + # check_authorization(security_scopes, "1234567890abdcef") + # self.assertEqual(he.exception.status_code, status.HTTP_401_UNAUTHORIZED) + # self.assertEqual(he.exception.detail, "Could not validate credentials") + + # @patch("tardis.rest.app.security.jwt") + # def test_check_authorization_jwt_error(self, mocked_jwt): + # mocked_jwt.decode.side_effect = JWTError + + # with self.assertRaises(HTTPException) as he: + # check_authorization(SecurityScopes(), self.infinite_resources_get_token) + # self.assertEqual(he.exception.status_code, status.HTTP_401_UNAUTHORIZED) + # self.assertEqual(he.exception.detail, "Could not validate credentials") + + # mocked_jwt.decode.side_effect = None def test_check_authentication(self): self.clear_lru_cache() From 8f55982500ddbc4b1181f3bc757ff4180a8f4324 Mon Sep 17 00:00:00 2001 From: Alexander Haas Date: Tue, 31 May 2022 09:49:31 +0200 Subject: [PATCH 13/94] fix: minor adjustements to reflect changed error codes --- tests/rest_t/app_t/test_security.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/tests/rest_t/app_t/test_security.py b/tests/rest_t/app_t/test_security.py index f2cbb107..f4392b72 100644 --- a/tests/rest_t/app_t/test_security.py +++ b/tests/rest_t/app_t/test_security.py @@ -100,13 +100,13 @@ def test_check_authentication(self): with self.assertRaises(HTTPException) as he: check_authentication(user_name="fails", password="test123") - self.assertEqual(he.exception.status_code, status.HTTP_400_BAD_REQUEST) + self.assertEqual(he.exception.status_code, status.HTTP_401_UNAUTHORIZED) self.assertEqual(he.exception.detail, "Incorrect username or password") self.clear_lru_cache() with self.assertRaises(HTTPException) as he: check_authentication(user_name="test", password="test123") - self.assertEqual(he.exception.status_code, status.HTTP_400_BAD_REQUEST) + self.assertEqual(he.exception.status_code, status.HTTP_401_UNAUTHORIZED) self.assertEqual(he.exception.detail, "Incorrect username or password") self.clear_lru_cache() From 777a09f901d629de6680f535f1bcfdb9781c1542 Mon Sep 17 00:00:00 2001 From: Alexander Haas Date: Tue, 31 May 2022 12:58:36 +0200 Subject: [PATCH 14/94] message: Non expiring tokens are a security risk and aren't really needed anymore with the introduction of refresh tokens, which provide a much safer alternative --- tests/rest_t/token_generator_t/test_generate_token.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/rest_t/token_generator_t/test_generate_token.py b/tests/rest_t/token_generator_t/test_generate_token.py index 663683c3..a5c6bbc1 100644 --- a/tests/rest_t/token_generator_t/test_generate_token.py +++ b/tests/rest_t/token_generator_t/test_generate_token.py @@ -6,7 +6,7 @@ from unittest import TestCase from unittest.mock import patch - +# !!! This can probably be deprecated as refresh tokens where introduced which are really easy to use and are much safer than non expiring tokens class TestGenerateToken(TestCase): mock_config_patcher = None mock_cobald_config_loader_patcher = None From 8afb2782ff9763a5076a1b129f4a082ff849a920 Mon Sep 17 00:00:00 2001 From: Alexander Haas Date: Tue, 31 May 2022 13:48:04 +0200 Subject: [PATCH 15/94] switch machine --- .vscode/settings.json | 4 ++ tardis/rest/app/main.py | 5 +- tardis/rest/app/routers/resources.py | 6 +- tardis/rest/app/routers/user.py | 10 ++-- tardis/rest/app/security.py | 8 ++- .../routers_t/base_test_case_routers.py | 11 ++-- tests/rest_t/routers_t/test_resources.py | 9 +++ .../routers_t/{test_login.py => test_user.py} | 56 ++++++++++--------- 8 files changed, 62 insertions(+), 47 deletions(-) create mode 100644 .vscode/settings.json rename tests/rest_t/routers_t/{test_login.py => test_user.py} (58%) diff --git a/.vscode/settings.json b/.vscode/settings.json new file mode 100644 index 00000000..33fe63f7 --- /dev/null +++ b/.vscode/settings.json @@ -0,0 +1,4 @@ +{ + "python.linting.flake8Enabled": true, + "python.linting.enabled": true +} \ No newline at end of file diff --git a/tardis/rest/app/main.py b/tardis/rest/app/main.py index 7f618440..f8cf89aa 100644 --- a/tardis/rest/app/main.py +++ b/tardis/rest/app/main.py @@ -34,10 +34,7 @@ @app.exception_handler(AuthJWTException) def authjwt_exception_handler(request: Request, exc: AuthJWTException): - return JSONResponse( - status_code=exc.status_code, - content={"detail": exc.message} - ) + return JSONResponse(status_code=exc.status_code, content={"detail": exc.message}) app.include_router(resources.router) diff --git a/tardis/rest/app/routers/resources.py b/tardis/rest/app/routers/resources.py index 7bc914b1..575eb519 100644 --- a/tardis/rest/app/routers/resources.py +++ b/tardis/rest/app/routers/resources.py @@ -6,8 +6,7 @@ router = APIRouter(prefix="/resources", tags=["resources"]) -@router.get("/{drone_uuid}/state", - description="Get current state of a resource") +@router.get("/{drone_uuid}/state", description="Get current state of a resource") async def get_resource_state( drone_uuid: str = Path(..., regex=r"^\S+-[A-Fa-f0-9]{10}$"), sql_registry: SqliteRegistry = Depends(database.get_sql_registry()), @@ -20,8 +19,7 @@ async def get_resource_state( try: query_result = query_result[0] except IndexError: - raise HTTPException( - status_code=404, detail="Drone not found") from None + raise HTTPException(status_code=404, detail="Drone not found") from None return query_result diff --git a/tardis/rest/app/routers/user.py b/tardis/rest/app/routers/user.py index 640f03a5..adaf703f 100644 --- a/tardis/rest/app/routers/user.py +++ b/tardis/rest/app/routers/user.py @@ -9,12 +9,12 @@ @router.post("/login", description="Sets httponly access token in session cookie") async def login(login_user: security.LoginUser, Authorize: AuthJWT = Depends()): - user = security.check_authentication( - login_user.user_name, login_user.password) + user = security.check_authentication(login_user.user_name, login_user.password) scopes = {"scopes": user.scopes} access_token = Authorize.create_access_token( - subject=user.user_name, user_claims=scopes) + subject=user.user_name, user_claims=scopes + ) refresh_token = Authorize.create_refresh_token(subject=user.user_name) Authorize.set_access_cookies(access_token) @@ -23,7 +23,7 @@ async def login(login_user: security.LoginUser, Authorize: AuthJWT = Depends()): return {"msg": "Successfully logged in!"} -@router.delete('/logout') +@router.delete("/logout") async def logout(Authorize: AuthJWT = Depends()): Authorize.jwt_required() @@ -31,7 +31,7 @@ async def logout(Authorize: AuthJWT = Depends()): return {"msg": "Successfully logged out!"} -@router.post('/refresh') +@router.post("/refresh") async def refresh(Authorize: AuthJWT = Depends()): Authorize.jwt_refresh_token_required() diff --git a/tardis/rest/app/security.py b/tardis/rest/app/security.py index 75ec2fbc..412e8361 100644 --- a/tardis/rest/app/security.py +++ b/tardis/rest/app/security.py @@ -4,6 +4,7 @@ from bcrypt import checkpw, gensalt, hashpw from fastapi import HTTPException, status + # from fastapi.security import SecurityScopes # from jose import JWTError, jwt from pydantic import BaseModel # , Field, ValidationError @@ -33,6 +34,7 @@ class BaseUser(BaseModel): user_name: str scopes: List[str] = [] + # TODO: Document the scopes manually # "resources:get": "Allows to read resource database", # "resources:put": "Allows to update resource database.", @@ -85,14 +87,16 @@ def check_authentication(user_name: str, password: str) -> DatabaseUser: if not user: raise HTTPException( status_code=status.HTTP_401_UNAUTHORIZED, - detail="Incorrect username or password") + detail="Incorrect username or password", + ) if checkpw(password.encode(), user.hashed_password.encode()): return user else: raise HTTPException( status_code=status.HTTP_401_UNAUTHORIZED, - detail="Incorrect username or password") + detail="Incorrect username or password", + ) @lru_cache(maxsize=1) diff --git a/tests/rest_t/routers_t/base_test_case_routers.py b/tests/rest_t/routers_t/base_test_case_routers.py index 16d2cb33..75202831 100644 --- a/tests/rest_t/routers_t/base_test_case_routers.py +++ b/tests/rest_t/routers_t/base_test_case_routers.py @@ -1,4 +1,4 @@ -from tardis.rest.app.security import get_algorithm, get_secret_key, get_user +from tardis.rest.app.security import get_algorithm, get_user from tardis.utilities.attributedict import AttributeDict from tests.utilities.utilities import run_async @@ -31,11 +31,9 @@ def tearDownClass(cls) -> None: cls.mock_config_patcher.stop() def setUp(self) -> None: - secret_key = "63328dc6b8524bf08b0ba151e287edb498852b77b97f837088de4d17247d032c" algorithm = "HS256" self.config = self.mock_config.return_value - self.config.Services.restapi.secret_key = secret_key self.config.Services.restapi.algorithm = algorithm self.config.Services.restapi.get_user.return_value = AttributeDict( user_name="test", @@ -55,12 +53,13 @@ def tearDown(self) -> None: @staticmethod def clear_lru_cache(): get_algorithm.cache_clear() - get_secret_key.cache_clear() get_user.cache_clear() @property def headers( self, - token="Bearer eyJhbGciOiJIUzI1NiIsInR5cCI6IkpXVCJ9.eyJzdWIiOiJ0ZXN0Iiwic2NvcGVzIjpbInJlc291cmNlczpnZXQiXX0.yybw7XC5GpTCHytNj2vl1nWLCgxaN7vpkIz4wF1-Pnc", # noqa B950 ): - return {"accept": "application/json", "Authorization": token} + return { + "accept": "application/json", + "Content-Type": "application/json", + } diff --git a/tests/rest_t/routers_t/test_resources.py b/tests/rest_t/routers_t/test_resources.py index e8283162..ecd8546b 100644 --- a/tests/rest_t/routers_t/test_resources.py +++ b/tests/rest_t/routers_t/test_resources.py @@ -5,6 +5,15 @@ class TestResources(TestCaseRouters): # Reminder: When defining `setUp`, `setUpClass`, `tearDown` and `tearDownClass` # in router tests the corresponding super().function() needs to be called as well. + def setUp(self): + super().setUp() + login = {"user_name": "test", "password": "test"} + # TODO: Create a static login token to make this test independent from /user/login + response = run_async( + self.client.post, "/user/login", headers=self.headers, json=login + ) + self.assertEqual(response.status_code, 200) + def test_get_resource_state(self): self.clear_lru_cache() self.mock_crud.get_resource_state.return_value = async_return( diff --git a/tests/rest_t/routers_t/test_login.py b/tests/rest_t/routers_t/test_user.py similarity index 58% rename from tests/rest_t/routers_t/test_login.py rename to tests/rest_t/routers_t/test_user.py index 0b6de8cd..221a4723 100644 --- a/tests/rest_t/routers_t/test_login.py +++ b/tests/rest_t/routers_t/test_user.py @@ -1,38 +1,46 @@ from tests.rest_t.routers_t.base_test_case_routers import TestCaseRouters from tests.utilities.utilities import run_async -from datetime import datetime -from unittest.mock import patch - class TestLogin(TestCaseRouters): # Reminder: When defining `setUp`, `setUpClass`, `tearDown` and `tearDownClass` # in router tests the corresponding super().function() needs to be called as well. - @patch("tardis.rest.app.security.datetime") - def test_get_access_token(self, mocked_datetime): - header = { - "accept": "application/json", - "Content-Type": "application/x-www-form-urlencoded", - } - + def test_login(self): data = { - "grant_type": "", - "username": "test1", + "user_name": "test1", "password": "test", "scope": "", - "client_id": "", - "client_secret": "", } + # No body and headers + self.clear_lru_cache() + response = run_async(self.client.post, "/user/login") + self.assertEqual(response.status_code, 422) + self.assertEqual( + response.json(), + { + "detail": [ + { + "loc": ["body"], + "msg": "field required", + "type": "value_error.missing", + } + ] + }, + ) + + # Invalid body but valid headers self.clear_lru_cache() - response = run_async(self.client.post, "/login/access-token") + response = run_async( + self.client.post, "/user/login", headers=self.headers, data="{}" + ) self.assertEqual(response.status_code, 422) self.assertEqual( response.json(), { "detail": [ { - "loc": ["body", "username"], + "loc": ["body", "user_name"], "msg": "field required", "type": "value_error.missing", }, @@ -46,32 +54,28 @@ def test_get_access_token(self, mocked_datetime): ) self.clear_lru_cache() - mocked_datetime.utcnow.return_value = datetime.utcfromtimestamp(0) response = run_async( - self.client.post, "/login/access-token", data=data, headers=header + self.client.post, "/user/login", json=data, headers=self.headers ) self.assertEqual(response.status_code, 200) self.assertEqual( response.json(), - { - "access_token": "eyJhbGciOiJIUzI1NiIsInR5cCI6IkpXVCJ9.eyJzdWIiOiJ0ZXN0Iiwic2NvcGVzIjpbInJlc291cmNlczpnZXQiXSwiZXhwIjo4NjQwMH0.FBozcHL4n21BMprTP8snzniaNClpPat3hlJ1b-glgJg", # noqa B509 - "token_type": "bearer", - }, + {"msg": "Successfully logged in!"}, ) self.clear_lru_cache() self.config.Services.restapi.get_user.side_effect = lambda user_name: None response = run_async( - self.client.post, "/login/access-token", data=data, headers=header + self.client.post, "/user/login", json=data, headers=self.headers ) - self.assertEqual(response.status_code, 400) + self.assertEqual(response.status_code, 401) self.assertEqual(response.json(), {"detail": "Incorrect username or password"}) self.config.Services.restapi.get_user.side_effect = None self.clear_lru_cache() data["password"] = "wrong" response = run_async( - self.client.post, "/login/access-token", data=data, headers=header + self.client.post, "/user/login", json=data, headers=self.headers ) - self.assertEqual(response.status_code, 400) + self.assertEqual(response.status_code, 401) self.assertEqual(response.json(), {"detail": "Incorrect username or password"}) From d5334560ccd817c51af8da14aca30e81ba67b069 Mon Sep 17 00:00:00 2001 From: Alexander Haas Date: Tue, 31 May 2022 20:44:09 +0200 Subject: [PATCH 16/94] fix: rename test_login to test_user + adjusting test to new api --- .../routers_t/{test_login.py => test_user.py} | 56 ++++++++++--------- 1 file changed, 30 insertions(+), 26 deletions(-) rename tests/rest_t/routers_t/{test_login.py => test_user.py} (58%) diff --git a/tests/rest_t/routers_t/test_login.py b/tests/rest_t/routers_t/test_user.py similarity index 58% rename from tests/rest_t/routers_t/test_login.py rename to tests/rest_t/routers_t/test_user.py index 0b6de8cd..221a4723 100644 --- a/tests/rest_t/routers_t/test_login.py +++ b/tests/rest_t/routers_t/test_user.py @@ -1,38 +1,46 @@ from tests.rest_t.routers_t.base_test_case_routers import TestCaseRouters from tests.utilities.utilities import run_async -from datetime import datetime -from unittest.mock import patch - class TestLogin(TestCaseRouters): # Reminder: When defining `setUp`, `setUpClass`, `tearDown` and `tearDownClass` # in router tests the corresponding super().function() needs to be called as well. - @patch("tardis.rest.app.security.datetime") - def test_get_access_token(self, mocked_datetime): - header = { - "accept": "application/json", - "Content-Type": "application/x-www-form-urlencoded", - } - + def test_login(self): data = { - "grant_type": "", - "username": "test1", + "user_name": "test1", "password": "test", "scope": "", - "client_id": "", - "client_secret": "", } + # No body and headers + self.clear_lru_cache() + response = run_async(self.client.post, "/user/login") + self.assertEqual(response.status_code, 422) + self.assertEqual( + response.json(), + { + "detail": [ + { + "loc": ["body"], + "msg": "field required", + "type": "value_error.missing", + } + ] + }, + ) + + # Invalid body but valid headers self.clear_lru_cache() - response = run_async(self.client.post, "/login/access-token") + response = run_async( + self.client.post, "/user/login", headers=self.headers, data="{}" + ) self.assertEqual(response.status_code, 422) self.assertEqual( response.json(), { "detail": [ { - "loc": ["body", "username"], + "loc": ["body", "user_name"], "msg": "field required", "type": "value_error.missing", }, @@ -46,32 +54,28 @@ def test_get_access_token(self, mocked_datetime): ) self.clear_lru_cache() - mocked_datetime.utcnow.return_value = datetime.utcfromtimestamp(0) response = run_async( - self.client.post, "/login/access-token", data=data, headers=header + self.client.post, "/user/login", json=data, headers=self.headers ) self.assertEqual(response.status_code, 200) self.assertEqual( response.json(), - { - "access_token": "eyJhbGciOiJIUzI1NiIsInR5cCI6IkpXVCJ9.eyJzdWIiOiJ0ZXN0Iiwic2NvcGVzIjpbInJlc291cmNlczpnZXQiXSwiZXhwIjo4NjQwMH0.FBozcHL4n21BMprTP8snzniaNClpPat3hlJ1b-glgJg", # noqa B509 - "token_type": "bearer", - }, + {"msg": "Successfully logged in!"}, ) self.clear_lru_cache() self.config.Services.restapi.get_user.side_effect = lambda user_name: None response = run_async( - self.client.post, "/login/access-token", data=data, headers=header + self.client.post, "/user/login", json=data, headers=self.headers ) - self.assertEqual(response.status_code, 400) + self.assertEqual(response.status_code, 401) self.assertEqual(response.json(), {"detail": "Incorrect username or password"}) self.config.Services.restapi.get_user.side_effect = None self.clear_lru_cache() data["password"] = "wrong" response = run_async( - self.client.post, "/login/access-token", data=data, headers=header + self.client.post, "/user/login", json=data, headers=self.headers ) - self.assertEqual(response.status_code, 400) + self.assertEqual(response.status_code, 401) self.assertEqual(response.json(), {"detail": "Incorrect username or password"}) From 5c3c790847b1fb1216e43984275693f742a86612 Mon Sep 17 00:00:00 2001 From: Alexander Haas Date: Tue, 31 May 2022 20:45:05 +0200 Subject: [PATCH 17/94] todo: maybe make test independent of login --- tests/rest_t/routers_t/test_resources.py | 1 + 1 file changed, 1 insertion(+) diff --git a/tests/rest_t/routers_t/test_resources.py b/tests/rest_t/routers_t/test_resources.py index e8283162..cf0f24fc 100644 --- a/tests/rest_t/routers_t/test_resources.py +++ b/tests/rest_t/routers_t/test_resources.py @@ -5,6 +5,7 @@ class TestResources(TestCaseRouters): # Reminder: When defining `setUp`, `setUpClass`, `tearDown` and `tearDownClass` # in router tests the corresponding super().function() needs to be called as well. + # TODO: Create a static login token to make this test independent from /user/login def test_get_resource_state(self): self.clear_lru_cache() self.mock_crud.get_resource_state.return_value = async_return( From cc0e44179ed2e9f07239243a01796f0df4825f9e Mon Sep 17 00:00:00 2001 From: Alexander Haas Date: Tue, 31 May 2022 20:45:44 +0200 Subject: [PATCH 18/94] fix: adjusting test_resources to new api --- tests/rest_t/routers_t/test_resources.py | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/tests/rest_t/routers_t/test_resources.py b/tests/rest_t/routers_t/test_resources.py index cf0f24fc..ecd8546b 100644 --- a/tests/rest_t/routers_t/test_resources.py +++ b/tests/rest_t/routers_t/test_resources.py @@ -5,7 +5,15 @@ class TestResources(TestCaseRouters): # Reminder: When defining `setUp`, `setUpClass`, `tearDown` and `tearDownClass` # in router tests the corresponding super().function() needs to be called as well. + def setUp(self): + super().setUp() + login = {"user_name": "test", "password": "test"} # TODO: Create a static login token to make this test independent from /user/login + response = run_async( + self.client.post, "/user/login", headers=self.headers, json=login + ) + self.assertEqual(response.status_code, 200) + def test_get_resource_state(self): self.clear_lru_cache() self.mock_crud.get_resource_state.return_value = async_return( From ac23677303fe0da3a774995ecb320e8c02375bcf Mon Sep 17 00:00:00 2001 From: Alexander Haas Date: Tue, 31 May 2022 20:46:21 +0200 Subject: [PATCH 19/94] fix: adjusting base test case routers to new api --- tests/rest_t/routers_t/base_test_case_routers.py | 11 +++++------ 1 file changed, 5 insertions(+), 6 deletions(-) diff --git a/tests/rest_t/routers_t/base_test_case_routers.py b/tests/rest_t/routers_t/base_test_case_routers.py index 16d2cb33..75202831 100644 --- a/tests/rest_t/routers_t/base_test_case_routers.py +++ b/tests/rest_t/routers_t/base_test_case_routers.py @@ -1,4 +1,4 @@ -from tardis.rest.app.security import get_algorithm, get_secret_key, get_user +from tardis.rest.app.security import get_algorithm, get_user from tardis.utilities.attributedict import AttributeDict from tests.utilities.utilities import run_async @@ -31,11 +31,9 @@ def tearDownClass(cls) -> None: cls.mock_config_patcher.stop() def setUp(self) -> None: - secret_key = "63328dc6b8524bf08b0ba151e287edb498852b77b97f837088de4d17247d032c" algorithm = "HS256" self.config = self.mock_config.return_value - self.config.Services.restapi.secret_key = secret_key self.config.Services.restapi.algorithm = algorithm self.config.Services.restapi.get_user.return_value = AttributeDict( user_name="test", @@ -55,12 +53,13 @@ def tearDown(self) -> None: @staticmethod def clear_lru_cache(): get_algorithm.cache_clear() - get_secret_key.cache_clear() get_user.cache_clear() @property def headers( self, - token="Bearer eyJhbGciOiJIUzI1NiIsInR5cCI6IkpXVCJ9.eyJzdWIiOiJ0ZXN0Iiwic2NvcGVzIjpbInJlc291cmNlczpnZXQiXX0.yybw7XC5GpTCHytNj2vl1nWLCgxaN7vpkIz4wF1-Pnc", # noqa B950 ): - return {"accept": "application/json", "Authorization": token} + return { + "accept": "application/json", + "Content-Type": "application/json", + } From f89b6f9bb97d99d6ee3b4c9653f217ac01f2e080 Mon Sep 17 00:00:00 2001 From: Alexander Haas Date: Tue, 31 May 2022 20:48:18 +0200 Subject: [PATCH 20/94] todo: deleted obvious todo --- tardis/rest/app/routers/user.py | 2 -- 1 file changed, 2 deletions(-) diff --git a/tardis/rest/app/routers/user.py b/tardis/rest/app/routers/user.py index 640f03a5..2483301d 100644 --- a/tardis/rest/app/routers/user.py +++ b/tardis/rest/app/routers/user.py @@ -4,8 +4,6 @@ router = APIRouter(prefix="/user", tags=["user"]) -# TODO: Very important: Set the csrf cookie in the frontend. - @router.post("/login", description="Sets httponly access token in session cookie") async def login(login_user: security.LoginUser, Authorize: AuthJWT = Depends()): From 54273501e607729c45095b10f7f3653cfc255e9e Mon Sep 17 00:00:00 2001 From: Alexander Haas Date: Tue, 31 May 2022 20:49:28 +0200 Subject: [PATCH 21/94] chore: deleted generate_token cli as it was deemed obscolete --- tardis/rest/token_generator/__init__.py | 0 tardis/rest/token_generator/__main__.py | 10 ----- tardis/rest/token_generator/generate_token.py | 43 ------------------- 3 files changed, 53 deletions(-) delete mode 100644 tardis/rest/token_generator/__init__.py delete mode 100644 tardis/rest/token_generator/__main__.py delete mode 100644 tardis/rest/token_generator/generate_token.py diff --git a/tardis/rest/token_generator/__init__.py b/tardis/rest/token_generator/__init__.py deleted file mode 100644 index e69de29b..00000000 diff --git a/tardis/rest/token_generator/__main__.py b/tardis/rest/token_generator/__main__.py deleted file mode 100644 index 4003385f..00000000 --- a/tardis/rest/token_generator/__main__.py +++ /dev/null @@ -1,10 +0,0 @@ -from .generate_token import generate_token -import typer - - -def generate_token_cli(): - typer.run(generate_token) - - -if __name__ == "__main__": - generate_token_cli() diff --git a/tardis/rest/token_generator/generate_token.py b/tardis/rest/token_generator/generate_token.py deleted file mode 100644 index 445c6bd8..00000000 --- a/tardis/rest/token_generator/generate_token.py +++ /dev/null @@ -1,43 +0,0 @@ -from ..app.security import create_access_token -from ...utilities.utils import disable_logging -from cobald.daemon.core.config import load - -from pathlib import Path -from typing import List -import logging -import typer - - -def generate_token( - user_name: str = typer.Option( - ..., help="User name to include in the generated token" - ), - scopes: List[str] = typer.Option( - ["resources:get"], help="Security scopes associated with the generated token" - ), - config_file: Path = typer.Option( - None, help="Path to the COBalD/TARDIS yaml configuration" - ), - secret_key: str = typer.Option(None, help="The secret key to generate a token"), - algorithm: str = typer.Option(None, help="The algorithm to generate a token"), -): - if config_file: - with disable_logging(logging.DEBUG): - with load(str(config_file)): # type hints of load expects a string - access_token = create_access_token(user_name=user_name, scopes=scopes) - elif algorithm and secret_key: - access_token = create_access_token( - user_name=user_name, - scopes=scopes, - secret_key=secret_key, - algorithm=algorithm, - ) - else: - typer.secho( - "Either a config-file or a secret-key and algorithm needs to be specified!", - bg=typer.colors.RED, - err=True, - ) - raise typer.Exit(code=1) - - typer.echo(access_token) From bba8c04d65a4ab47c9fc355d015b88884dbca698 Mon Sep 17 00:00:00 2001 From: Alexander Haas Date: Tue, 31 May 2022 20:50:15 +0200 Subject: [PATCH 22/94] chore: deleted generate_token tests --- tests/rest_t/token_generator_t/__init__.py | 0 .../token_generator_t/test_generate_token.py | 79 ------------------- tests/rest_t/token_generator_t/test_main.py | 21 ----- 3 files changed, 100 deletions(-) delete mode 100644 tests/rest_t/token_generator_t/__init__.py delete mode 100644 tests/rest_t/token_generator_t/test_generate_token.py delete mode 100644 tests/rest_t/token_generator_t/test_main.py diff --git a/tests/rest_t/token_generator_t/__init__.py b/tests/rest_t/token_generator_t/__init__.py deleted file mode 100644 index e69de29b..00000000 diff --git a/tests/rest_t/token_generator_t/test_generate_token.py b/tests/rest_t/token_generator_t/test_generate_token.py deleted file mode 100644 index a5c6bbc1..00000000 --- a/tests/rest_t/token_generator_t/test_generate_token.py +++ /dev/null @@ -1,79 +0,0 @@ -from tardis.rest.token_generator.generate_token import generate_token -from tardis.rest.app.security import get_algorithm, get_secret_key - -from typer.testing import CliRunner -from typer import Typer -from unittest import TestCase -from unittest.mock import patch - -# !!! This can probably be deprecated as refresh tokens where introduced which are really easy to use and are much safer than non expiring tokens -class TestGenerateToken(TestCase): - mock_config_patcher = None - mock_cobald_config_loader_patcher = None - - @classmethod - def setUpClass(cls) -> None: - cls.mock_config_patcher = patch("tardis.rest.app.security.Configuration") - cls.mock_config = cls.mock_config_patcher.start() - - cls.mock_cobald_config_loader_patcher = patch( - "tardis.rest.token_generator.generate_token.load" - ) - cls.mock_cobald_config_loader = cls.mock_cobald_config_loader_patcher.start() - - @classmethod - def tearDownClass(cls) -> None: - cls.mock_config_patcher.stop() - cls.mock_cobald_config_loader_patcher.stop() - - def setUp(self) -> None: - self.app = Typer() - self.app.command()(generate_token) - - self.runner = CliRunner() - - config = self.mock_config.return_value - config.Services.restapi.secret_key = ( - "752e003f636f402cc23728e185ce8c9eef27b7e02cf509b3015f7757e625b8e4" - ) - config.Services.restapi.algorithm = "HS256" - - @staticmethod - def clear_lru_cache(): - get_algorithm.cache_clear() - get_secret_key.cache_clear() - - def test_generate_token(self): - result = self.runner.invoke(self.app) - self.assertNotEqual(result.exit_code, 0) - self.assertTrue("Missing option '--user-name'" in result.stdout) - - result = self.runner.invoke(self.app, ["--user-name=test"]) - self.assertEqual(result.exit_code, 1) - self.assertTrue( - "Either a config-file or a secret-key and algorithm needs to be specified!" - in result.stdout - ) - - expected_token = "eyJhbGciOiJIUzI1NiIsInR5cCI6IkpXVCJ9.eyJzdWIiOiJ0ZXN0Iiwic2NvcGVzIjpbInJlc291cmNlczpnZXQiXX0.JtjroDVX2FlHI-DFO3XErRutDjvcZwbZF4Bqx736JTc" # noqa: B950 - - result = self.runner.invoke( - self.app, - [ - "--user-name=test", - "--algorithm=HS256", - "--secret-key=752e003f636f402cc23728e185ce8c9eef27b7e02cf509b3015f7757e625b8e4", # noqa: B950 - ], - ) - self.assertEqual(result.exit_code, 0) - self.assertEqual(result.stdout.strip(), expected_token) - - self.clear_lru_cache() - - result = self.runner.invoke( - self.app, ["--user-name=test", "--config-file=test.yml"] - ) - - self.mock_cobald_config_loader.assert_called_once_with("test.yml") - self.assertEqual(result.exit_code, 0) - self.assertEqual(result.stdout.strip(), expected_token) diff --git a/tests/rest_t/token_generator_t/test_main.py b/tests/rest_t/token_generator_t/test_main.py deleted file mode 100644 index c998673a..00000000 --- a/tests/rest_t/token_generator_t/test_main.py +++ /dev/null @@ -1,21 +0,0 @@ -from tardis.rest.token_generator.__main__ import generate_token, generate_token_cli - -from unittest import TestCase -from unittest.mock import patch - - -class TestTokenGeneratorMain(TestCase): - mock_typer_patcher = None - - @classmethod - def setUpClass(cls) -> None: - cls.mock_typer_patcher = patch("tardis.rest.token_generator.__main__.typer") - cls.mock_typer = cls.mock_typer_patcher.start() - - @classmethod - def tearDownClass(cls) -> None: - cls.mock_typer_patcher.stop() - - def testGenerateTokenCli(self): - generate_token_cli() - self.mock_typer.run.assert_called_with(generate_token) From 3112ccfadc9b4937106c4a089c0b1b59f5a0d90a Mon Sep 17 00:00:00 2001 From: Alexander Haas Date: Tue, 31 May 2022 20:53:07 +0200 Subject: [PATCH 23/94] fix: delete all traces of secret_key to fix test_service --- tests/rest_t/test_service.py | 8 +------- 1 file changed, 1 insertion(+), 7 deletions(-) diff --git a/tests/rest_t/test_service.py b/tests/rest_t/test_service.py index 26eec459..117b6fd2 100644 --- a/tests/rest_t/test_service.py +++ b/tests/rest_t/test_service.py @@ -8,10 +8,8 @@ class TestRestService(TestCase): def setUp(self) -> None: self.algorithm = "test_algorithm" - self.secret_key = "test_key" self.rest_service = RestService( algorithm=self.algorithm, - secret_key=self.secret_key, ) @patch("tardis.rest.service.Server") @@ -23,12 +21,9 @@ def test_run(self, mocked_server): run_async(self.rest_service.run) mocked_server.assert_called_with(config=self.rest_service._config) - def test_secret_key(self): - self.assertEqual(self.rest_service.secret_key, self.secret_key) - def test_algorithm(self): self.assertEqual(self.rest_service.algorithm, self.algorithm) - self.assertEqual(RestService(secret_key=self.secret_key).algorithm, "HS256") + self.assertEqual(RestService().algorithm, "HS256") def test_get_user(self): self.assertIsNone(self.rest_service.get_user(user_name="test")) @@ -41,7 +36,6 @@ def test_get_user(self): rest_service = RestService( algorithm=self.algorithm, - secret_key=self.secret_key, users=[user], ) From 01dfcaf27dac650579bc70990be2ab06c66f99fd Mon Sep 17 00:00:00 2001 From: Alexander Haas Date: Tue, 31 May 2022 21:26:10 +0200 Subject: [PATCH 24/94] todo: do some refactoring in the future --- tests/rest_t/routers_t/test_user.py | 1 + 1 file changed, 1 insertion(+) diff --git a/tests/rest_t/routers_t/test_user.py b/tests/rest_t/routers_t/test_user.py index 221a4723..82c42fc1 100644 --- a/tests/rest_t/routers_t/test_user.py +++ b/tests/rest_t/routers_t/test_user.py @@ -1,6 +1,7 @@ from tests.rest_t.routers_t.base_test_case_routers import TestCaseRouters from tests.utilities.utilities import run_async +# TODO: decrease code repetition by extracting basic auth test into separate function class TestLogin(TestCaseRouters): # Reminder: When defining `setUp`, `setUpClass`, `tearDown` and `tearDownClass` From 6ea50abc03402d3df61747dbe46209302db29d3b Mon Sep 17 00:00:00 2001 From: Alexander Haas Date: Tue, 31 May 2022 21:35:51 +0200 Subject: [PATCH 25/94] refactor: move the test_user into the base test class --- tests/rest_t/routers_t/base_test_case_routers.py | 4 ++++ tests/rest_t/routers_t/test_user.py | 6 ------ 2 files changed, 4 insertions(+), 6 deletions(-) diff --git a/tests/rest_t/routers_t/base_test_case_routers.py b/tests/rest_t/routers_t/base_test_case_routers.py index 75202831..6c7329b4 100644 --- a/tests/rest_t/routers_t/base_test_case_routers.py +++ b/tests/rest_t/routers_t/base_test_case_routers.py @@ -46,6 +46,10 @@ def setUp(self) -> None: ) # has to be imported after SqliteRegistry patch self.client = AsyncClient(app=app, base_url="http://test") + self.test_user = { + "user_name": "test1", + "password": "test", + } def tearDown(self) -> None: run_async(self.client.aclose) diff --git a/tests/rest_t/routers_t/test_user.py b/tests/rest_t/routers_t/test_user.py index 82c42fc1..3296bb20 100644 --- a/tests/rest_t/routers_t/test_user.py +++ b/tests/rest_t/routers_t/test_user.py @@ -7,12 +7,6 @@ class TestLogin(TestCaseRouters): # Reminder: When defining `setUp`, `setUpClass`, `tearDown` and `tearDownClass` # in router tests the corresponding super().function() needs to be called as well. def test_login(self): - data = { - "user_name": "test1", - "password": "test", - "scope": "", - } - # No body and headers self.clear_lru_cache() response = run_async(self.client.post, "/user/login") From 0464c2043e836eba13b714f9c188b10ecc5d8ffe Mon Sep 17 00:00:00 2001 From: Alexander Haas Date: Tue, 31 May 2022 21:37:16 +0200 Subject: [PATCH 26/94] refactor: move login into base test class --- tests/rest_t/routers_t/base_test_case_routers.py | 5 +++++ tests/rest_t/routers_t/test_user.py | 7 +++---- 2 files changed, 8 insertions(+), 4 deletions(-) diff --git a/tests/rest_t/routers_t/base_test_case_routers.py b/tests/rest_t/routers_t/base_test_case_routers.py index 6c7329b4..948772e3 100644 --- a/tests/rest_t/routers_t/base_test_case_routers.py +++ b/tests/rest_t/routers_t/base_test_case_routers.py @@ -54,6 +54,11 @@ def setUp(self) -> None: def tearDown(self) -> None: run_async(self.client.aclose) + def login(self): + self.clear_lru_cache() + response = run_async(self.client.post, "/user/login", json=self.test_user) + self.assertEqual(response.status_code, 200) + @staticmethod def clear_lru_cache(): get_algorithm.cache_clear() diff --git a/tests/rest_t/routers_t/test_user.py b/tests/rest_t/routers_t/test_user.py index 3296bb20..dd35d083 100644 --- a/tests/rest_t/routers_t/test_user.py +++ b/tests/rest_t/routers_t/test_user.py @@ -68,9 +68,8 @@ def test_login(self): self.config.Services.restapi.get_user.side_effect = None self.clear_lru_cache() - data["password"] = "wrong" - response = run_async( - self.client.post, "/user/login", json=data, headers=self.headers + self.login() ) + self.login() self.assertEqual(response.status_code, 401) - self.assertEqual(response.json(), {"detail": "Incorrect username or password"}) + self.login() From f1b95714e2fceb6b48973c0f65d6c9b3d68b07cc Mon Sep 17 00:00:00 2001 From: Alexander Haas Date: Tue, 31 May 2022 21:38:54 +0200 Subject: [PATCH 27/94] chore: delete headers as they aren't needed --- tests/rest_t/routers_t/base_test_case_routers.py | 9 --------- tests/rest_t/routers_t/test_user.py | 14 ++++---------- 2 files changed, 4 insertions(+), 19 deletions(-) diff --git a/tests/rest_t/routers_t/base_test_case_routers.py b/tests/rest_t/routers_t/base_test_case_routers.py index 948772e3..5a9e1bb4 100644 --- a/tests/rest_t/routers_t/base_test_case_routers.py +++ b/tests/rest_t/routers_t/base_test_case_routers.py @@ -63,12 +63,3 @@ def login(self): def clear_lru_cache(): get_algorithm.cache_clear() get_user.cache_clear() - - @property - def headers( - self, - ): - return { - "accept": "application/json", - "Content-Type": "application/json", - } diff --git a/tests/rest_t/routers_t/test_user.py b/tests/rest_t/routers_t/test_user.py index dd35d083..09aea3f7 100644 --- a/tests/rest_t/routers_t/test_user.py +++ b/tests/rest_t/routers_t/test_user.py @@ -24,11 +24,9 @@ def test_login(self): }, ) - # Invalid body but valid headers + # Empty body self.clear_lru_cache() - response = run_async( - self.client.post, "/user/login", headers=self.headers, data="{}" - ) + response = run_async(self.client.post, "/user/login", data="{}") self.assertEqual(response.status_code, 422) self.assertEqual( response.json(), @@ -49,9 +47,7 @@ def test_login(self): ) self.clear_lru_cache() - response = run_async( - self.client.post, "/user/login", json=data, headers=self.headers - ) + response = run_async(self.client.post, "/user/login", json=self.test_user) self.assertEqual(response.status_code, 200) self.assertEqual( response.json(), @@ -60,9 +56,7 @@ def test_login(self): self.clear_lru_cache() self.config.Services.restapi.get_user.side_effect = lambda user_name: None - response = run_async( - self.client.post, "/user/login", json=data, headers=self.headers - ) + response = run_async(self.client.post, "/user/login", json=self.test_user) self.assertEqual(response.status_code, 401) self.assertEqual(response.json(), {"detail": "Incorrect username or password"}) self.config.Services.restapi.get_user.side_effect = None From b69f865c6410e7aa785c2d88d560fb6cc38919e0 Mon Sep 17 00:00:00 2001 From: Alexander Haas Date: Tue, 31 May 2022 21:39:38 +0200 Subject: [PATCH 28/94] feat: added test for all /user functions --- tests/rest_t/routers_t/test_user.py | 50 +++++++++++++++++++++++++++++ 1 file changed, 50 insertions(+) diff --git a/tests/rest_t/routers_t/test_user.py b/tests/rest_t/routers_t/test_user.py index 09aea3f7..75d3a595 100644 --- a/tests/rest_t/routers_t/test_user.py +++ b/tests/rest_t/routers_t/test_user.py @@ -3,6 +3,7 @@ # TODO: decrease code repetition by extracting basic auth test into separate function + class TestLogin(TestCaseRouters): # Reminder: When defining `setUp`, `setUpClass`, `tearDown` and `tearDownClass` # in router tests the corresponding super().function() needs to be called as well. @@ -62,8 +63,57 @@ def test_login(self): self.config.Services.restapi.get_user.side_effect = None self.clear_lru_cache() + self.test_user["password"] = "wrong" + response = run_async(self.client.post, "/user/login", json=self.test_user) + self.assertEqual(response.status_code, 401) + self.assertEqual(response.json(), {"detail": "Incorrect username or password"}) + + def test_logout(self): + # Not logged in yet + self.clear_lru_cache() + response = run_async(self.client.delete, "/user/logout") + + self.assertEqual(response.status_code, 401) + self.assertEqual( + response.json(), {"detail": "Missing cookie access_token_cookie"} + ) + + # correct login self.login() + response = run_async(self.client.delete, "/user/logout") + self.assertEqual(response.status_code, 200) + + # prevent second logout + response = run_async(self.client.delete, "/user/logout") + self.assertEqual(response.status_code, 401) + + def test_refresh(self): + # Not logged in yet + self.clear_lru_cache() + response = run_async(self.client.post, "/user/refresh") + self.assertEqual(response.status_code, 401) + self.assertEqual( + response.json(), {"detail": "Missing cookie refresh_token_cookie"} ) + + # correct login self.login() + response = run_async(self.client.post, "/user/refresh") + self.assertEqual(response.status_code, 200) + + def test_user_me(self): + # Not logged in yet + self.clear_lru_cache() + response = run_async(self.client.get, "/user/me") self.assertEqual(response.status_code, 401) + self.assertEqual( + response.json(), {"detail": "Missing cookie access_token_cookie"} + ) + + # should work self.login() + response = run_async(self.client.get, "/user/me") + self.assertEqual(response.status_code, 200) + self.assertEqual( + response.json(), {"user_name": "test", "scopes": ["resources:get"]} + ) From fb26c24513cef1e1761f899262efadec8a3e9c7e Mon Sep 17 00:00:00 2001 From: Alexander Haas Date: Tue, 31 May 2022 21:55:23 +0200 Subject: [PATCH 29/94] fix: remove old headers --- tests/rest_t/routers_t/test_resources.py | 22 ++++++++++------------ 1 file changed, 10 insertions(+), 12 deletions(-) diff --git a/tests/rest_t/routers_t/test_resources.py b/tests/rest_t/routers_t/test_resources.py index ecd8546b..b2c47cb9 100644 --- a/tests/rest_t/routers_t/test_resources.py +++ b/tests/rest_t/routers_t/test_resources.py @@ -9,9 +9,7 @@ def setUp(self): super().setUp() login = {"user_name": "test", "password": "test"} # TODO: Create a static login token to make this test independent from /user/login - response = run_async( - self.client.post, "/user/login", headers=self.headers, json=login - ) + response = run_async(self.client.post, "/user/login", json=login) self.assertEqual(response.status_code, 200) def test_get_resource_state(self): @@ -21,7 +19,8 @@ def test_get_resource_state(self): ) response = run_async( - self.client.get, "/resources/test-0123456789/state", headers=self.headers + self.client.get, + "/resources/test-0123456789/state", ) self.assertEqual(response.status_code, 200) self.assertEqual( @@ -30,15 +29,11 @@ def test_get_resource_state(self): ) self.mock_crud.get_resource_state.return_value = async_return(return_value=[]) - response = run_async( - self.client.get, "/resources/test-1234567890/state", headers=self.headers - ) + response = run_async(self.client.get, "/resources/test-1234567890/state") self.assertEqual(response.status_code, 404) self.assertEqual(response.json(), {"detail": "Drone not found"}) - response = run_async( - self.client.get, "/resources/test-invalid/state", headers=self.headers - ) + response = run_async(self.client.get, "/resources/test-invalid/state") self.assertEqual(response.status_code, 422) self.assertEqual( response.json(), @@ -54,7 +49,10 @@ def test_get_resource_state(self): }, ) - response = run_async(self.client.get, "/resources/state", headers=self.headers) + response = run_async( + self.client.get, + "/resources/state", + ) self.assertEqual(response.status_code, 404) self.assertEqual(response.json(), {"detail": "Not Found"}) @@ -84,7 +82,7 @@ def test_get_resources(self): return_value=full_expected_resources ) - response = run_async(self.client.get, "/resources/", headers=self.headers) + response = run_async(self.client.get, "/resources/") self.assertEqual(response.status_code, 200) self.assertEqual( response.json(), From 815599535329d2398ff9ca5931ca6dad3917e488 Mon Sep 17 00:00:00 2001 From: Alexander Haas Date: Tue, 31 May 2022 21:55:37 +0200 Subject: [PATCH 30/94] formatting --- setup.py | 2 +- tardis/rest/app/main.py | 5 +---- tardis/rest/app/routers/resources.py | 6 ++---- tardis/rest/app/routers/user.py | 10 +++++----- tardis/rest/app/security.py | 8 ++++++-- 5 files changed, 15 insertions(+), 16 deletions(-) diff --git a/setup.py b/setup.py index 0dceeacf..4ae2a927 100644 --- a/setup.py +++ b/setup.py @@ -91,7 +91,7 @@ def get_cryptography_version(): "python-multipart", "typing_extensions", "backports.cached_property", - "fastapi-jwt-auth" + "fastapi-jwt-auth", ], extras_require={ "docs": [ diff --git a/tardis/rest/app/main.py b/tardis/rest/app/main.py index 7f618440..f8cf89aa 100644 --- a/tardis/rest/app/main.py +++ b/tardis/rest/app/main.py @@ -34,10 +34,7 @@ @app.exception_handler(AuthJWTException) def authjwt_exception_handler(request: Request, exc: AuthJWTException): - return JSONResponse( - status_code=exc.status_code, - content={"detail": exc.message} - ) + return JSONResponse(status_code=exc.status_code, content={"detail": exc.message}) app.include_router(resources.router) diff --git a/tardis/rest/app/routers/resources.py b/tardis/rest/app/routers/resources.py index 7bc914b1..575eb519 100644 --- a/tardis/rest/app/routers/resources.py +++ b/tardis/rest/app/routers/resources.py @@ -6,8 +6,7 @@ router = APIRouter(prefix="/resources", tags=["resources"]) -@router.get("/{drone_uuid}/state", - description="Get current state of a resource") +@router.get("/{drone_uuid}/state", description="Get current state of a resource") async def get_resource_state( drone_uuid: str = Path(..., regex=r"^\S+-[A-Fa-f0-9]{10}$"), sql_registry: SqliteRegistry = Depends(database.get_sql_registry()), @@ -20,8 +19,7 @@ async def get_resource_state( try: query_result = query_result[0] except IndexError: - raise HTTPException( - status_code=404, detail="Drone not found") from None + raise HTTPException(status_code=404, detail="Drone not found") from None return query_result diff --git a/tardis/rest/app/routers/user.py b/tardis/rest/app/routers/user.py index 2483301d..a99d39d3 100644 --- a/tardis/rest/app/routers/user.py +++ b/tardis/rest/app/routers/user.py @@ -7,12 +7,12 @@ @router.post("/login", description="Sets httponly access token in session cookie") async def login(login_user: security.LoginUser, Authorize: AuthJWT = Depends()): - user = security.check_authentication( - login_user.user_name, login_user.password) + user = security.check_authentication(login_user.user_name, login_user.password) scopes = {"scopes": user.scopes} access_token = Authorize.create_access_token( - subject=user.user_name, user_claims=scopes) + subject=user.user_name, user_claims=scopes + ) refresh_token = Authorize.create_refresh_token(subject=user.user_name) Authorize.set_access_cookies(access_token) @@ -21,7 +21,7 @@ async def login(login_user: security.LoginUser, Authorize: AuthJWT = Depends()): return {"msg": "Successfully logged in!"} -@router.delete('/logout') +@router.delete("/logout") async def logout(Authorize: AuthJWT = Depends()): Authorize.jwt_required() @@ -29,7 +29,7 @@ async def logout(Authorize: AuthJWT = Depends()): return {"msg": "Successfully logged out!"} -@router.post('/refresh') +@router.post("/refresh") async def refresh(Authorize: AuthJWT = Depends()): Authorize.jwt_refresh_token_required() diff --git a/tardis/rest/app/security.py b/tardis/rest/app/security.py index 75ec2fbc..412e8361 100644 --- a/tardis/rest/app/security.py +++ b/tardis/rest/app/security.py @@ -4,6 +4,7 @@ from bcrypt import checkpw, gensalt, hashpw from fastapi import HTTPException, status + # from fastapi.security import SecurityScopes # from jose import JWTError, jwt from pydantic import BaseModel # , Field, ValidationError @@ -33,6 +34,7 @@ class BaseUser(BaseModel): user_name: str scopes: List[str] = [] + # TODO: Document the scopes manually # "resources:get": "Allows to read resource database", # "resources:put": "Allows to update resource database.", @@ -85,14 +87,16 @@ def check_authentication(user_name: str, password: str) -> DatabaseUser: if not user: raise HTTPException( status_code=status.HTTP_401_UNAUTHORIZED, - detail="Incorrect username or password") + detail="Incorrect username or password", + ) if checkpw(password.encode(), user.hashed_password.encode()): return user else: raise HTTPException( status_code=status.HTTP_401_UNAUTHORIZED, - detail="Incorrect username or password") + detail="Incorrect username or password", + ) @lru_cache(maxsize=1) From eedc12d72eb5d228b48f89475995d39841fbc774 Mon Sep 17 00:00:00 2001 From: Alexander Haas Date: Tue, 31 May 2022 22:18:03 +0200 Subject: [PATCH 31/94] formatting --- tests/rest_t/routers_t/test_resources.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/tests/rest_t/routers_t/test_resources.py b/tests/rest_t/routers_t/test_resources.py index b2c47cb9..dc42e256 100644 --- a/tests/rest_t/routers_t/test_resources.py +++ b/tests/rest_t/routers_t/test_resources.py @@ -8,7 +8,8 @@ class TestResources(TestCaseRouters): def setUp(self): super().setUp() login = {"user_name": "test", "password": "test"} - # TODO: Create a static login token to make this test independent from /user/login + # TODO: Create a static login token to make this test + # independent from /user/login response = run_async(self.client.post, "/user/login", json=login) self.assertEqual(response.status_code, 200) From d665cbd377e295090b9d14695f117c7f63c941c0 Mon Sep 17 00:00:00 2001 From: Alexander Haas Date: Wed, 1 Jun 2022 15:49:17 +0200 Subject: [PATCH 32/94] feat: introducing a scope enum for better typesafety --- tardis/rest/app/scopes.py | 11 +++++++++++ 1 file changed, 11 insertions(+) create mode 100644 tardis/rest/app/scopes.py diff --git a/tardis/rest/app/scopes.py b/tardis/rest/app/scopes.py new file mode 100644 index 00000000..8b3bdfa5 --- /dev/null +++ b/tardis/rest/app/scopes.py @@ -0,0 +1,11 @@ +from enum import Enum + +# All available OAuth2 scopes +class Scope(str, Enum): + class Resources(str, Enum): + get = "resources:get" + put = "resources:put" + + class User(str, Enum): + get = "user:get" + put = "user:put" From 616ea9e7e6a4b2370f76de60ee500230454d7792 Mon Sep 17 00:00:00 2001 From: Alexander Haas Date: Wed, 1 Jun 2022 17:10:57 +0200 Subject: [PATCH 33/94] fix: scopes could't be set manually at login --- tardis/rest/app/routers/user.py | 7 ++++++- tardis/rest/app/security.py | 23 ++++++++--------------- 2 files changed, 14 insertions(+), 16 deletions(-) diff --git a/tardis/rest/app/routers/user.py b/tardis/rest/app/routers/user.py index a99d39d3..a6eab594 100644 --- a/tardis/rest/app/routers/user.py +++ b/tardis/rest/app/routers/user.py @@ -9,7 +9,12 @@ async def login(login_user: security.LoginUser, Authorize: AuthJWT = Depends()): user = security.check_authentication(login_user.user_name, login_user.password) - scopes = {"scopes": user.scopes} + if login_user.scopes == None: + scopes = {"scopes": user.scopes} + else: + security.check_scope_permissions(login_user.scopes, user.scopes) + scopes = {"scopes": login_user.scopes} + access_token = Authorize.create_access_token( subject=user.user_name, user_claims=scopes ) diff --git a/tardis/rest/app/security.py b/tardis/rest/app/security.py index 412e8361..ba1fc86c 100644 --- a/tardis/rest/app/security.py +++ b/tardis/rest/app/security.py @@ -48,14 +48,13 @@ class DatabaseUser(BaseUser): hashed_password: str -# TODO: Implement scopes properly when needed -# def check_authorization( -# security_scopes: SecurityScopes, token: str = Depends(oauth2_scheme) -# ) -> TokenData: -# if security_scopes.scopes: -# authenticate_value = f'Bearer scope="{security_scopes.scope_str}"' -# else: -# authenticate_value = "Bearer" +def check_scope_permissions(requested_scopes: List[str], allowed_scopes: List[str]): + for scope in requested_scopes: + if scope not in allowed_scopes: + raise HTTPException( + status_code=status.HTTP_403_FORBIDDEN, + detail="Not enough permissions", + ) from None # try: # payload = jwt.decode(token, get_secret_key(), @@ -70,13 +69,7 @@ class DatabaseUser(BaseUser): # headers={"WWW-Authenticate": authenticate_value}, # ) from err -# for scope in security_scopes.scopes: -# if scope not in token_data.scopes: -# raise HTTPException( -# status_code=status.HTTP_403_FORBIDDEN, -# detail="Not enough permissions", -# headers={"WWW-Authenticate": authenticate_value}, -# ) from None + check_scope_permissions(security_scopes.scopes, token_scopes) # return token_data From 6267e72401fe41dd40a3a8410b1175a1d82920ae Mon Sep 17 00:00:00 2001 From: Alexander Haas Date: Thu, 2 Jun 2022 10:33:44 +0200 Subject: [PATCH 34/94] chore: clean up imports --- tardis/rest/app/routers/resources.py | 4 ++-- tardis/rest/app/security.py | 8 ++++---- 2 files changed, 6 insertions(+), 6 deletions(-) diff --git a/tardis/rest/app/routers/resources.py b/tardis/rest/app/routers/resources.py index 575eb519..25a720c3 100644 --- a/tardis/rest/app/routers/resources.py +++ b/tardis/rest/app/routers/resources.py @@ -1,7 +1,7 @@ +from .. import security from .. import crud, database from ....plugins.sqliteregistry import SqliteRegistry -from fastapi import APIRouter, Depends, HTTPException, Path -from fastapi_jwt_auth import AuthJWT +from fastapi import APIRouter, Depends, HTTPException, Path, Security, status router = APIRouter(prefix="/resources", tags=["resources"]) diff --git a/tardis/rest/app/security.py b/tardis/rest/app/security.py index ba1fc86c..8e8843c2 100644 --- a/tardis/rest/app/security.py +++ b/tardis/rest/app/security.py @@ -3,11 +3,11 @@ from ...exceptions.tardisexceptions import TardisError from bcrypt import checkpw, gensalt, hashpw -from fastapi import HTTPException, status +from fastapi import HTTPException, status, Depends -# from fastapi.security import SecurityScopes -# from jose import JWTError, jwt -from pydantic import BaseModel # , Field, ValidationError +from fastapi.security import SecurityScopes + +from pydantic import BaseModel from fastapi_jwt_auth import AuthJWT # from datetime import datetime, timedelta From 95eb89c599d0f010cce29ff8d9562a6c221b1e84 Mon Sep 17 00:00:00 2001 From: Alexander Haas Date: Thu, 2 Jun 2022 10:35:00 +0200 Subject: [PATCH 35/94] chore: changed scopes to Optional parameter but this doesn't seem to have changed anything --- tardis/rest/app/security.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tardis/rest/app/security.py b/tardis/rest/app/security.py index 8e8843c2..b32fe039 100644 --- a/tardis/rest/app/security.py +++ b/tardis/rest/app/security.py @@ -32,7 +32,7 @@ def get_config(): class BaseUser(BaseModel): user_name: str - scopes: List[str] = [] + scopes: Optional[List[str]] = None # TODO: Document the scopes manually From ed0500da765c750ab9e8ed02953a68ddfbf7fe39 Mon Sep 17 00:00:00 2001 From: Alexander Haas Date: Thu, 2 Jun 2022 10:36:37 +0200 Subject: [PATCH 36/94] chore: rewrote check_authorization for cookie auth --- tardis/rest/app/security.py | 25 ++++++++++++------------- 1 file changed, 12 insertions(+), 13 deletions(-) diff --git a/tardis/rest/app/security.py b/tardis/rest/app/security.py index b32fe039..e0cdc349 100644 --- a/tardis/rest/app/security.py +++ b/tardis/rest/app/security.py @@ -56,22 +56,21 @@ def check_scope_permissions(requested_scopes: List[str], allowed_scopes: List[st detail="Not enough permissions", ) from None -# try: -# payload = jwt.decode(token, get_secret_key(), -# algorithms=[get_algorithm()]) -# user_name: str = payload.get("sub") -# token_scopes = payload.get("scopes", []) -# token_data = TokenData(scopes=token_scopes, user_name=user_name) -# except (JWTError, ValidationError) as err: -# raise HTTPException( -# status_code=status.HTTP_401_UNAUTHORIZED, -# detail="Could not validate credentials", -# headers={"WWW-Authenticate": authenticate_value}, -# ) from err + +def check_authorization( + security_scopes: SecurityScopes, Authorize: AuthJWT = Depends() +) -> AuthJWT: + # No authorization without authentication + Authorize.jwt_required() + + try: + token_scopes = Authorize.get_raw_jwt()["scopes"] + except KeyError: + raise TardisError("Scopes not defined in jwt token") from None check_scope_permissions(security_scopes.scopes, token_scopes) -# return token_data + return Authorize def check_authentication(user_name: str, password: str) -> DatabaseUser: From db0723f5e330f5bac56d47e1afb8802e36474d5f Mon Sep 17 00:00:00 2001 From: Alexander Haas Date: Thu, 2 Jun 2022 10:42:31 +0200 Subject: [PATCH 37/94] chore: replace int error with fastapi literal --- tardis/rest/app/routers/resources.py | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/tardis/rest/app/routers/resources.py b/tardis/rest/app/routers/resources.py index 25a720c3..e40279f9 100644 --- a/tardis/rest/app/routers/resources.py +++ b/tardis/rest/app/routers/resources.py @@ -19,7 +19,9 @@ async def get_resource_state( try: query_result = query_result[0] except IndexError: - raise HTTPException(status_code=404, detail="Drone not found") from None + raise HTTPException( + status_code=status.HTTP_404_NOT_FOUND, detail="Drone not found" + ) from None return query_result From 8df4ff8d7476d31bbecad25078a59a73304e2e24 Mon Sep 17 00:00:00 2001 From: Alexander Haas Date: Thu, 2 Jun 2022 10:43:53 +0200 Subject: [PATCH 38/94] chore: reintroduce authorization into api functions --- tardis/rest/app/routers/resources.py | 10 ++-------- tardis/rest/app/routers/user.py | 7 ++++--- 2 files changed, 6 insertions(+), 11 deletions(-) diff --git a/tardis/rest/app/routers/resources.py b/tardis/rest/app/routers/resources.py index e40279f9..d7fb9ee2 100644 --- a/tardis/rest/app/routers/resources.py +++ b/tardis/rest/app/routers/resources.py @@ -10,11 +10,8 @@ async def get_resource_state( drone_uuid: str = Path(..., regex=r"^\S+-[A-Fa-f0-9]{10}$"), sql_registry: SqliteRegistry = Depends(database.get_sql_registry()), - Authorize: AuthJWT = Depends() - # _: str = Security(security.check_authorization, scopes=["resources:get"]) + _: str = Security(security.check_authorization, scopes=["resources:get"]), ): - Authorize.jwt_required() - query_result = await crud.get_resource_state(sql_registry, drone_uuid) try: query_result = query_result[0] @@ -28,10 +25,7 @@ async def get_resource_state( @router.get("/", description="Get list of managed resources") async def get_resources( sql_registry: SqliteRegistry = Depends(database.get_sql_registry()), - Authorize: AuthJWT = Depends() - # _: str = Security(security.check_authorization, scopes=["resources:get"]) + _: str = Security(security.check_authorization, scopes=["resources:get"]), ): - Authorize.jwt_required() - query_result = await crud.get_resources(sql_registry) return query_result diff --git a/tardis/rest/app/routers/user.py b/tardis/rest/app/routers/user.py index a6eab594..162fd42d 100644 --- a/tardis/rest/app/routers/user.py +++ b/tardis/rest/app/routers/user.py @@ -1,5 +1,6 @@ from .. import security from fastapi import APIRouter, Depends +from fastapi.security import Security from fastapi_jwt_auth import AuthJWT router = APIRouter(prefix="/user", tags=["user"]) @@ -46,8 +47,8 @@ async def refresh(Authorize: AuthJWT = Depends()): @router.get("/me", response_model=security.BaseUser) -async def get_user_me(Authorize: AuthJWT = Depends()): - Authorize.jwt_required() - +async def get_user_me( + Authorize: AuthJWT = Security(security.check_authorization, scopes=["user:get"]), +): user_name = Authorize.get_jwt_subject() return security.get_user(user_name) From a2f364b34a6a5964162e8d19247db66eb8b00f72 Mon Sep 17 00:00:00 2001 From: Alexander Haas Date: Thu, 2 Jun 2022 10:45:16 +0200 Subject: [PATCH 39/94] todo: add functionality to /user/token_scopes --- tardis/rest/app/routers/user.py | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/tardis/rest/app/routers/user.py b/tardis/rest/app/routers/user.py index 162fd42d..04fd29fe 100644 --- a/tardis/rest/app/routers/user.py +++ b/tardis/rest/app/routers/user.py @@ -52,3 +52,8 @@ async def get_user_me( ): user_name = Authorize.get_jwt_subject() return security.get_user(user_name) + + +@router.get("/token_scopes") +async def get_token_scopes(): + pass From 3a96e554a0bd2e464f5498f46533fc4dbd18530d Mon Sep 17 00:00:00 2001 From: Alexander Haas Date: Thu, 2 Jun 2022 11:27:37 +0200 Subject: [PATCH 40/94] chore: introduced and fixed enum scopes into api --- tardis/rest/app/routers/resources.py | 5 +++-- tardis/rest/app/routers/user.py | 13 +++++++++---- tardis/rest/app/scopes.py | 18 ++++++++++-------- 3 files changed, 22 insertions(+), 14 deletions(-) diff --git a/tardis/rest/app/routers/resources.py b/tardis/rest/app/routers/resources.py index d7fb9ee2..ffcbe975 100644 --- a/tardis/rest/app/routers/resources.py +++ b/tardis/rest/app/routers/resources.py @@ -2,6 +2,7 @@ from .. import crud, database from ....plugins.sqliteregistry import SqliteRegistry from fastapi import APIRouter, Depends, HTTPException, Path, Security, status +from ..scopes import Resources router = APIRouter(prefix="/resources", tags=["resources"]) @@ -10,7 +11,7 @@ async def get_resource_state( drone_uuid: str = Path(..., regex=r"^\S+-[A-Fa-f0-9]{10}$"), sql_registry: SqliteRegistry = Depends(database.get_sql_registry()), - _: str = Security(security.check_authorization, scopes=["resources:get"]), + _=Security(security.check_authorization, scopes=[Resources.get]), ): query_result = await crud.get_resource_state(sql_registry, drone_uuid) try: @@ -25,7 +26,7 @@ async def get_resource_state( @router.get("/", description="Get list of managed resources") async def get_resources( sql_registry: SqliteRegistry = Depends(database.get_sql_registry()), - _: str = Security(security.check_authorization, scopes=["resources:get"]), + _=Security(security.check_authorization, scopes=[Resources.get]), ): query_result = await crud.get_resources(sql_registry) return query_result diff --git a/tardis/rest/app/routers/user.py b/tardis/rest/app/routers/user.py index 04fd29fe..b55e4713 100644 --- a/tardis/rest/app/routers/user.py +++ b/tardis/rest/app/routers/user.py @@ -1,13 +1,16 @@ +from ..scopes import User from .. import security -from fastapi import APIRouter, Depends -from fastapi.security import Security +from fastapi import APIRouter, Depends, Security from fastapi_jwt_auth import AuthJWT router = APIRouter(prefix="/user", tags=["user"]) @router.post("/login", description="Sets httponly access token in session cookie") -async def login(login_user: security.LoginUser, Authorize: AuthJWT = Depends()): +async def login( + login_user: security.LoginUser, + Authorize: AuthJWT = Depends(), +): user = security.check_authentication(login_user.user_name, login_user.password) if login_user.scopes == None: @@ -48,8 +51,10 @@ async def refresh(Authorize: AuthJWT = Depends()): @router.get("/me", response_model=security.BaseUser) async def get_user_me( - Authorize: AuthJWT = Security(security.check_authorization, scopes=["user:get"]), + Authorize: AuthJWT = Security(security.check_authorization, scopes=[User.get]), ): + Authorize.jwt_required() + user_name = Authorize.get_jwt_subject() return security.get_user(user_name) diff --git a/tardis/rest/app/scopes.py b/tardis/rest/app/scopes.py index 8b3bdfa5..591177ad 100644 --- a/tardis/rest/app/scopes.py +++ b/tardis/rest/app/scopes.py @@ -1,11 +1,13 @@ from enum import Enum # All available OAuth2 scopes -class Scope(str, Enum): - class Resources(str, Enum): - get = "resources:get" - put = "resources:put" - - class User(str, Enum): - get = "user:get" - put = "user:put" + + +class Resources(str, Enum): + get = "resources:get" + put = "resources:put" + + +class User(str, Enum): + get = "user:get" + put = "user:put" # usused From 0a2411af9575efde367013ac4d368e314a5b35f0 Mon Sep 17 00:00:00 2001 From: Alexander Haas Date: Thu, 2 Jun 2022 11:43:41 +0200 Subject: [PATCH 41/94] note: please check these two lines carefully in case I made an error --- tardis/rest/app/routers/user.py | 19 +++++++++++++++---- 1 file changed, 15 insertions(+), 4 deletions(-) diff --git a/tardis/rest/app/routers/user.py b/tardis/rest/app/routers/user.py index b55e4713..5d711061 100644 --- a/tardis/rest/app/routers/user.py +++ b/tardis/rest/app/routers/user.py @@ -16,6 +16,8 @@ async def login( if login_user.scopes == None: scopes = {"scopes": user.scopes} else: + # The next two lines are very critical as if wrongly implemented a user can give his token unlimited scopes. + # This functionality has to be tested thoroughly security.check_scope_permissions(login_user.scopes, user.scopes) scopes = {"scopes": login_user.scopes} @@ -30,7 +32,10 @@ async def login( return {"msg": "Successfully logged in!"} -@router.delete("/logout") +@router.delete( + "/logout", + description="Logout the current user by deleting all access token cookies", +) async def logout(Authorize: AuthJWT = Depends()): Authorize.jwt_required() @@ -38,7 +43,9 @@ async def logout(Authorize: AuthJWT = Depends()): return {"msg": "Successfully logged out!"} -@router.post("/refresh") +@router.post( + "/refresh", description="Use refresh token cookie to refresh expiration on cookie" +) async def refresh(Authorize: AuthJWT = Depends()): Authorize.jwt_refresh_token_required() @@ -49,7 +56,11 @@ async def refresh(Authorize: AuthJWT = Depends()): return {"msg": "Token successfully refreshed"} -@router.get("/me", response_model=security.BaseUser) +@router.get( + "/me", + response_model=security.BaseUser, + description="Get the user data how it's stored in the database (no password)", +) async def get_user_me( Authorize: AuthJWT = Security(security.check_authorization, scopes=[User.get]), ): @@ -59,6 +70,6 @@ async def get_user_me( return security.get_user(user_name) -@router.get("/token_scopes") +@router.get("/token_scopes", description="get scopes of CURRENT token (not of user)") async def get_token_scopes(): pass From b798388a9b14e7e4912a0e07559a1e7f35275bf7 Mon Sep 17 00:00:00 2001 From: Alexander Haas Date: Fri, 3 Jun 2022 00:34:12 +0200 Subject: [PATCH 42/94] feat: delete scope added --- tardis/rest/app/scopes.py | 1 + 1 file changed, 1 insertion(+) diff --git a/tardis/rest/app/scopes.py b/tardis/rest/app/scopes.py index 591177ad..351afa4e 100644 --- a/tardis/rest/app/scopes.py +++ b/tardis/rest/app/scopes.py @@ -6,6 +6,7 @@ class Resources(str, Enum): get = "resources:get" put = "resources:put" + delete = "resources:delete" class User(str, Enum): From dc01e5e07dacb556d3f1bd65032e5a78834e0d78 Mon Sep 17 00:00:00 2001 From: Alexander Haas Date: Fri, 3 Jun 2022 09:05:32 +0200 Subject: [PATCH 43/94] refactoring: minor --- tardis/rest/app/routers/resources.py | 9 +++++++++ tardis/rest/app/routers/user.py | 11 ++++++++--- tardis/rest/app/security.py | 14 +++++++++----- 3 files changed, 26 insertions(+), 8 deletions(-) diff --git a/tardis/rest/app/routers/resources.py b/tardis/rest/app/routers/resources.py index ffcbe975..2ce9a4e7 100644 --- a/tardis/rest/app/routers/resources.py +++ b/tardis/rest/app/routers/resources.py @@ -6,6 +6,8 @@ router = APIRouter(prefix="/resources", tags=["resources"]) +# TODO: Implement dependency for single drone operations + @router.get("/{drone_uuid}/state", description="Get current state of a resource") async def get_resource_state( @@ -30,3 +32,10 @@ async def get_resources( ): query_result = await crud.get_resources(sql_registry) return query_result + + +@router.delete('/{drone_uuid}/shutdown', description="Gently shut shown drone") +async def shutdown_drone( + _=Security(security.check_authorization, scopes=[Resources.delete]), +): + pass diff --git a/tardis/rest/app/routers/user.py b/tardis/rest/app/routers/user.py index 5d711061..d4171feb 100644 --- a/tardis/rest/app/routers/user.py +++ b/tardis/rest/app/routers/user.py @@ -6,14 +6,17 @@ router = APIRouter(prefix="/user", tags=["user"]) -@router.post("/login", description="Sets httponly access token in session cookie") +@router.post( + "/login", + description="Sets httponly access token in session cookie. The scopes are optional.", +) async def login( login_user: security.LoginUser, Authorize: AuthJWT = Depends(), ): user = security.check_authentication(login_user.user_name, login_user.password) - if login_user.scopes == None: + if login_user.scopes is None: scopes = {"scopes": user.scopes} else: # The next two lines are very critical as if wrongly implemented a user can give his token unlimited scopes. @@ -29,7 +32,9 @@ async def login( Authorize.set_access_cookies(access_token) Authorize.set_refresh_cookies(refresh_token) - return {"msg": "Successfully logged in!"} + # It is ok to return the user here because every user should have the user:get scope. + # Tokens don't have permissions to this function + return {"msg": "Successfully logged in!", "user": security.BaseUser(user_name=user.user_name, scopes=scopes["scopes"])} @router.delete( diff --git a/tardis/rest/app/security.py b/tardis/rest/app/security.py index e0cdc349..d67f9c34 100644 --- a/tardis/rest/app/security.py +++ b/tardis/rest/app/security.py @@ -63,11 +63,7 @@ def check_authorization( # No authorization without authentication Authorize.jwt_required() - try: - token_scopes = Authorize.get_raw_jwt()["scopes"] - except KeyError: - raise TardisError("Scopes not defined in jwt token") from None - + token_scopes = get_token_scopes(Authorize) check_scope_permissions(security_scopes.scopes, token_scopes) return Authorize @@ -91,6 +87,14 @@ def check_authentication(user_name: str, password: str) -> DatabaseUser: ) +def get_token_scopes(Authorize: AuthJWT) -> List[str]: + try: + token_scopes: List[str] = Authorize.get_raw_jwt()["scopes"] + except KeyError: + raise TardisError("Scopes not defined in jwt token") from None + return token_scopes + + @lru_cache(maxsize=1) def get_algorithm() -> str: # This can probably be deprecated as fastapi_jwt_tokens From 4b9a427200091951692973943ed1bbb201ef8ec9 Mon Sep 17 00:00:00 2001 From: Alexander Haas Date: Tue, 7 Jun 2022 21:17:20 +0200 Subject: [PATCH 44/94] feat: :sparkles: Added new crud functions to retrieve available types from DB --- tardis/rest/app/crud.py | 15 +++++++++++++++ 1 file changed, 15 insertions(+) diff --git a/tardis/rest/app/crud.py b/tardis/rest/app/crud.py index 960df596..c8dce19f 100644 --- a/tardis/rest/app/crud.py +++ b/tardis/rest/app/crud.py @@ -16,3 +16,18 @@ async def get_resources(sql_registry): JOIN Sites S ON R.site_id = S.site_id JOIN MachineTypes MT ON R.machine_type_id = MT.machine_type_id""" return await sql_registry.async_execute(sql_query, {}) + + +async def get_available_states(sql_registry): + sql_query = "SELECT RS.state FROM ResourceStates RS" + return await sql_registry.async_execute(sql_query, {}) + + +async def get_available_sites(sql_registry): + sql_query = "SELECT S.site_name FROM Sites S" + return await sql_registry.async_execute(sql_query, {}) + + +async def get_available_machine_types(sql_registry): + sql_query = "SELECT MT.machine_type FROM MachineTypes MT" + return await sql_registry.async_execute(sql_query, {}) From 81285e837163b98024039f0a2bba7ea99a4c9158 Mon Sep 17 00:00:00 2001 From: Alexander Haas Date: Tue, 7 Jun 2022 21:18:46 +0200 Subject: [PATCH 45/94] feat: :sparkles: New router with functions for retrieving types from DB All types like sites, machine types and status can now be retrieved from the api --- tardis/rest/app/main.py | 3 +- tardis/rest/app/routers/types.py | 47 ++++++++++++++++++++++++++++++++ 2 files changed, 49 insertions(+), 1 deletion(-) create mode 100644 tardis/rest/app/routers/types.py diff --git a/tardis/rest/app/main.py b/tardis/rest/app/main.py index f8cf89aa..59192d3e 100644 --- a/tardis/rest/app/main.py +++ b/tardis/rest/app/main.py @@ -1,5 +1,5 @@ from ...__about__ import __version__ -from .routers import resources, user +from .routers import resources, user, types from fastapi_jwt_auth.exceptions import AuthJWTException from fastapi import Request from fastapi.responses import JSONResponse @@ -39,3 +39,4 @@ def authjwt_exception_handler(request: Request, exc: AuthJWTException): app.include_router(resources.router) app.include_router(user.router) +app.include_router(types.router) diff --git a/tardis/rest/app/routers/types.py b/tardis/rest/app/routers/types.py new file mode 100644 index 00000000..25c52ade --- /dev/null +++ b/tardis/rest/app/routers/types.py @@ -0,0 +1,47 @@ +from typing import Dict, List + +from tardis.exceptions.tardisexceptions import TardisError +from .. import security +from .. import crud, database +from ....plugins.sqliteregistry import SqliteRegistry +from fastapi import APIRouter, Depends, Security +from ..scopes import Resources + +router = APIRouter(prefix="/types", tags=["types", "resources"]) + + +def sql_to_list(query_result: List[Dict]) -> List[str]: + out = [] + try: + for val in query_result: + out.append(list(val.values())[0]) + except Exception as e: + raise TardisError("Query result has invalid format") from e + return out + + +@router.get("/states", description="Get all available states") +async def get_resource_state( + sql_registry: SqliteRegistry = Depends(database.get_sql_registry()), + _=Security(security.check_authorization, scopes=[Resources.get]), +): + query_result = await crud.get_available_states(sql_registry) + return sql_to_list(query_result) + + +@router.get("/sites", description="Get all available sites") +async def get_resource_state( + sql_registry: SqliteRegistry = Depends(database.get_sql_registry()), + _=Security(security.check_authorization, scopes=[Resources.get]), +): + query_result = await crud.get_available_sites(sql_registry) + return sql_to_list(query_result) + + +@router.get("/machine_types", description="Get all available machine types") +async def get_resource_state( + sql_registry: SqliteRegistry = Depends(database.get_sql_registry()), + _=Security(security.check_authorization, scopes=[Resources.get]), +): + query_result = await crud.get_available_machine_types(sql_registry) + return sql_to_list(query_result) From 7ecce87c65226ff7d99e1747e09578734ddaf7d5 Mon Sep 17 00:00:00 2001 From: Alexander Haas Date: Tue, 7 Jun 2022 21:19:29 +0200 Subject: [PATCH 46/94] fix: :bug: Refresh token didn't update scopes/claims --- tardis/rest/app/routers/user.py | 15 ++++++++++++--- 1 file changed, 12 insertions(+), 3 deletions(-) diff --git a/tardis/rest/app/routers/user.py b/tardis/rest/app/routers/user.py index d4171feb..f4d5e95f 100644 --- a/tardis/rest/app/routers/user.py +++ b/tardis/rest/app/routers/user.py @@ -27,14 +27,19 @@ async def login( access_token = Authorize.create_access_token( subject=user.user_name, user_claims=scopes ) - refresh_token = Authorize.create_refresh_token(subject=user.user_name) + refresh_token = Authorize.create_refresh_token( + subject=user.user_name, user_claims=scopes + ) Authorize.set_access_cookies(access_token) Authorize.set_refresh_cookies(refresh_token) # It is ok to return the user here because every user should have the user:get scope. # Tokens don't have permissions to this function - return {"msg": "Successfully logged in!", "user": security.BaseUser(user_name=user.user_name, scopes=scopes["scopes"])} + return { + "msg": "Successfully logged in!", + "user": security.BaseUser(user_name=user.user_name, scopes=scopes["scopes"]), + } @router.delete( @@ -55,7 +60,11 @@ async def refresh(Authorize: AuthJWT = Depends()): Authorize.jwt_refresh_token_required() current_user = Authorize.get_jwt_subject() - new_access_token = Authorize.create_access_token(subject=current_user) + scopes = security.get_token_scopes(Authorize) + + new_access_token = Authorize.create_access_token( + subject=current_user, user_claims=({"scopes": scopes}) + ) Authorize.set_access_cookies(new_access_token) return {"msg": "Token successfully refreshed"} From c9c375eaa4febfb8e4500d54b9aa56b8b95e2c83 Mon Sep 17 00:00:00 2001 From: Alexander Haas Date: Fri, 24 Jun 2022 16:40:34 +0200 Subject: [PATCH 47/94] fix: :bug: jwt auth fastapi package was using 422 for Unauthorized responses??? Now returns 401 --- tardis/rest/app/main.py | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/tardis/rest/app/main.py b/tardis/rest/app/main.py index 59192d3e..6522c79e 100644 --- a/tardis/rest/app/main.py +++ b/tardis/rest/app/main.py @@ -34,7 +34,12 @@ @app.exception_handler(AuthJWTException) def authjwt_exception_handler(request: Request, exc: AuthJWTException): - return JSONResponse(status_code=exc.status_code, content={"detail": exc.message}) + detail = exc.message + + if detail == "Signature verification failed" or detail == "Signature has expired": + exc.status_code = 401 + + return JSONResponse(status_code=exc.status_code, content={"detail": detail}) app.include_router(resources.router) From 6c1ea0c25668442867babe6b371b9c43dcad8540 Mon Sep 17 00:00:00 2001 From: Alexander Haas Date: Thu, 28 Jul 2022 14:00:23 +0200 Subject: [PATCH 48/94] chore: :fire: Deleted SQL types --- tardis/rest/app/crud.py | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/tardis/rest/app/crud.py b/tardis/rest/app/crud.py index c8dce19f..1dfdc417 100644 --- a/tardis/rest/app/crud.py +++ b/tardis/rest/app/crud.py @@ -19,15 +19,15 @@ async def get_resources(sql_registry): async def get_available_states(sql_registry): - sql_query = "SELECT RS.state FROM ResourceStates RS" + sql_query = "SELECT state FROM ResourceStates" return await sql_registry.async_execute(sql_query, {}) async def get_available_sites(sql_registry): - sql_query = "SELECT S.site_name FROM Sites S" + sql_query = "SELECT site_name FROM Sites" return await sql_registry.async_execute(sql_query, {}) async def get_available_machine_types(sql_registry): - sql_query = "SELECT MT.machine_type FROM MachineTypes MT" + sql_query = "SELECT machine_type FROM MachineTypes" return await sql_registry.async_execute(sql_query, {}) From e26e40d674a88c92e6fcf4a4b5e361fbe72a3d01 Mon Sep 17 00:00:00 2001 From: Alexander Haas Date: Thu, 28 Jul 2022 14:00:30 +0200 Subject: [PATCH 49/94] comment --- tardis/rest/app/security.py | 1 + 1 file changed, 1 insertion(+) diff --git a/tardis/rest/app/security.py b/tardis/rest/app/security.py index d67f9c34..257029d4 100644 --- a/tardis/rest/app/security.py +++ b/tardis/rest/app/security.py @@ -22,6 +22,7 @@ class Settings(BaseModel): # TODO: change this to true in production so only https traffic is allowed authjwt_cookie_secure: bool = False # TODO: Set this too to True. But as soon as possible. + # Update: As 'same_site' is strict this is probably enough. authjwt_cookie_csrf_protect: bool = False From ef873c2ea1ac98d809a79b7e55d113941e2ca330 Mon Sep 17 00:00:00 2001 From: Alexander Haas Date: Thu, 28 Jul 2022 14:01:25 +0200 Subject: [PATCH 50/94] chore: :fire: Deleted TODO I don't know what I meant with this comment --- tardis/rest/app/routers/resources.py | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/tardis/rest/app/routers/resources.py b/tardis/rest/app/routers/resources.py index 2ce9a4e7..6d20d6ef 100644 --- a/tardis/rest/app/routers/resources.py +++ b/tardis/rest/app/routers/resources.py @@ -6,8 +6,6 @@ router = APIRouter(prefix="/resources", tags=["resources"]) -# TODO: Implement dependency for single drone operations - @router.get("/{drone_uuid}/state", description="Get current state of a resource") async def get_resource_state( @@ -34,8 +32,9 @@ async def get_resources( return query_result -@router.delete('/{drone_uuid}/shutdown', description="Gently shut shown drone") +@router.delete("/{drone_uuid}/drain", description="Gently shut shown drone") async def shutdown_drone( _=Security(security.check_authorization, scopes=[Resources.delete]), ): - pass + # TODO: Implement + return {"msg": "Not implemented"} From efc2bc918c277e3966e95b1e935f1d5ba057980b Mon Sep 17 00:00:00 2001 From: Alexander Haas Date: Thu, 28 Jul 2022 14:05:12 +0200 Subject: [PATCH 51/94] chore: New TODO comment in /token_scopes path --- tardis/rest/app/routers/user.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/tardis/rest/app/routers/user.py b/tardis/rest/app/routers/user.py index f4d5e95f..c32f5669 100644 --- a/tardis/rest/app/routers/user.py +++ b/tardis/rest/app/routers/user.py @@ -86,4 +86,5 @@ async def get_user_me( @router.get("/token_scopes", description="get scopes of CURRENT token (not of user)") async def get_token_scopes(): - pass + # TODO: Implement + return {"msg": "Not implemented"} From 52e757309915c09d362fa8c4a099f6ab4a86d1f8 Mon Sep 17 00:00:00 2001 From: Alexander Haas Date: Fri, 29 Jul 2022 11:44:55 +0200 Subject: [PATCH 52/94] chore: :fire: unused import --- tardis/rest/app/security.py | 1 - 1 file changed, 1 deletion(-) diff --git a/tardis/rest/app/security.py b/tardis/rest/app/security.py index 257029d4..33a366e0 100644 --- a/tardis/rest/app/security.py +++ b/tardis/rest/app/security.py @@ -10,7 +10,6 @@ from pydantic import BaseModel from fastapi_jwt_auth import AuthJWT -# from datetime import datetime, timedelta from functools import lru_cache from typing import Optional, List From ef58a8cc9559eda0ea8e1332a03f9424fe7b9a23 Mon Sep 17 00:00:00 2001 From: Alexander Haas Date: Fri, 29 Jul 2022 13:51:15 +0200 Subject: [PATCH 53/94] feat: Return required scopes if authorization fails --- tardis/rest/app/security.py | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/tardis/rest/app/security.py b/tardis/rest/app/security.py index 33a366e0..b8fe5ddb 100644 --- a/tardis/rest/app/security.py +++ b/tardis/rest/app/security.py @@ -53,7 +53,10 @@ def check_scope_permissions(requested_scopes: List[str], allowed_scopes: List[st if scope not in allowed_scopes: raise HTTPException( status_code=status.HTTP_403_FORBIDDEN, - detail="Not enough permissions", + detail={ + "msg": "Not enough permissions", + "scopesRequired": requested_scopes, + }, ) from None From cdafd4ac616cd4e77c45abb23ef4b848715e9e2f Mon Sep 17 00:00:00 2001 From: Alexander Haas Date: Fri, 29 Jul 2022 13:51:57 +0200 Subject: [PATCH 54/94] feat: Implemented the get_token_scopes function --- tardis/rest/app/routers/user.py | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/tardis/rest/app/routers/user.py b/tardis/rest/app/routers/user.py index c32f5669..8a6dc884 100644 --- a/tardis/rest/app/routers/user.py +++ b/tardis/rest/app/routers/user.py @@ -85,6 +85,7 @@ async def get_user_me( @router.get("/token_scopes", description="get scopes of CURRENT token (not of user)") -async def get_token_scopes(): - # TODO: Implement - return {"msg": "Not implemented"} +async def get_token_scopes(Authorize: AuthJWT = Depends()): + Authorize.jwt_required() + + return security.get_token_scopes(Authorize) From 235d64488bc2dda19906e04fa497a98d4ad31bae Mon Sep 17 00:00:00 2001 From: Alexander Haas Date: Fri, 29 Jul 2022 14:40:56 +0200 Subject: [PATCH 55/94] test: :white_check_mark: Added and updated tests to reflect the changes in the REST API --- tests/rest_t/app_t/test_security.py | 61 ++++++------------- .../routers_t/base_test_case_routers.py | 10 ++- tests/rest_t/routers_t/test_types.py | 56 +++++++++++++++++ tests/rest_t/routers_t/test_user.py | 30 +++++++-- 4 files changed, 106 insertions(+), 51 deletions(-) create mode 100644 tests/rest_t/routers_t/test_types.py diff --git a/tests/rest_t/app_t/test_security.py b/tests/rest_t/app_t/test_security.py index f4392b72..ef59897a 100644 --- a/tests/rest_t/app_t/test_security.py +++ b/tests/rest_t/app_t/test_security.py @@ -1,7 +1,9 @@ +from starlette.types import Scope from tardis.exceptions.tardisexceptions import TardisError +from tardis.plugins.sqliteregistry import SqliteRegistry from tardis.rest.app.security import ( - # check_authorization, check_authentication, + check_scope_permissions, get_algorithm, get_user, hash_password, @@ -10,6 +12,7 @@ from fastapi import HTTPException, status + from unittest import TestCase from unittest.mock import patch @@ -51,49 +54,19 @@ def clear_lru_cache(): get_algorithm.cache_clear() get_user.cache_clear() - # def test_check_authorization(self): - # self.clear_lru_cache() - # security_scopes = SecurityScopes(["resources:get"]) - # token_data = check_authorization( - # security_scopes, self.infinite_resources_get_token - # ) - - # self.assertEqual( - # token_data, TokenData(scopes=security_scopes.scopes, user_name="test") - # ) - - # security_scopes = SecurityScopes(["resources:put"]) - # with self.assertRaises(HTTPException) as he: - # check_authorization(security_scopes, self.infinite_resources_get_token) - # self.assertEqual(he.exception.status_code, status.HTTP_401_UNAUTHORIZED) - # self.assertEqual(he.exception.detail, "Not enough permissions") - - # token_data = check_authorization( - # security_scopes, self.infinite_resources_get_update_token - # ) - # self.assertEqual( - # token_data, - # TokenData(scopes=["resources:get", "resources:put"], user_name="test"), - # ) - - # security_scopes = SecurityScopes() - # check_authorization(security_scopes, self.infinite_resources_get_token) - - # with self.assertRaises(HTTPException) as he: - # check_authorization(security_scopes, "1234567890abdcef") - # self.assertEqual(he.exception.status_code, status.HTTP_401_UNAUTHORIZED) - # self.assertEqual(he.exception.detail, "Could not validate credentials") - - # @patch("tardis.rest.app.security.jwt") - # def test_check_authorization_jwt_error(self, mocked_jwt): - # mocked_jwt.decode.side_effect = JWTError - - # with self.assertRaises(HTTPException) as he: - # check_authorization(SecurityScopes(), self.infinite_resources_get_token) - # self.assertEqual(he.exception.status_code, status.HTTP_401_UNAUTHORIZED) - # self.assertEqual(he.exception.detail, "Could not validate credentials") - - # mocked_jwt.decode.side_effect = None + def test_check_scope_permissions(self): + self.assertRaises( + HTTPException, + check_scope_permissions, + ["resources:get"], + ["user:get"], + ) + + check_scope_permissions(["resources:get"], ["resources:get"]) + + def test_check_authorization(self): + # It is very hard mocking the AuthJWT object. However, authorization is tested implicitly as an integration test in the router tests. + pass def test_check_authentication(self): self.clear_lru_cache() diff --git a/tests/rest_t/routers_t/base_test_case_routers.py b/tests/rest_t/routers_t/base_test_case_routers.py index 5a9e1bb4..aeedffbb 100644 --- a/tests/rest_t/routers_t/base_test_case_routers.py +++ b/tests/rest_t/routers_t/base_test_case_routers.py @@ -18,9 +18,11 @@ def setUpClass(cls) -> None: cls.mock_sqlite_registry_patcher = patch( "tardis.rest.app.database.SqliteRegistry" ) + cls.mock_types_patcher = patch("tardis.rest.app.routers.types.crud") cls.mock_crud_patcher = patch("tardis.rest.app.routers.resources.crud") cls.mock_config_patcher = patch("tardis.rest.app.security.Configuration") cls.mock_sqlite_registry = cls.mock_sqlite_registry_patcher.start() + cls.mock_types = cls.mock_types_patcher.start() cls.mock_crud = cls.mock_crud_patcher.start() cls.mock_config = cls.mock_config_patcher.start() @@ -38,7 +40,7 @@ def setUp(self) -> None: self.config.Services.restapi.get_user.return_value = AttributeDict( user_name="test", hashed_password="$2b$12$Gkl8KYNGRMhx4kB0bKJnyuRuzOrx3LZlWf1CReIsDk9HyWoUGBihG", # noqa B509 - scopes=["resources:get"], + scopes=["resources:get", "user:get"], ) from tardis.rest.app.main import ( @@ -54,9 +56,11 @@ def setUp(self) -> None: def tearDown(self) -> None: run_async(self.client.aclose) - def login(self): + def login(self, user: dict = None): self.clear_lru_cache() - response = run_async(self.client.post, "/user/login", json=self.test_user) + response = run_async( + self.client.post, "/user/login", json=user or self.test_user + ) self.assertEqual(response.status_code, 200) @staticmethod diff --git a/tests/rest_t/routers_t/test_types.py b/tests/rest_t/routers_t/test_types.py new file mode 100644 index 00000000..494974dc --- /dev/null +++ b/tests/rest_t/routers_t/test_types.py @@ -0,0 +1,56 @@ +from tardis.utilities.attributedict import AttributeDict +from tests.rest_t.routers_t.base_test_case_routers import TestCaseRouters +from tests.utilities.utilities import async_return, run_async + + +def is_list_str(resp): + return isinstance(resp, list) and all(isinstance(x, str) for x in resp) + + +class TestTypes(TestCaseRouters): + def setUp(self) -> None: + super().setUp() + login = {"user_name": "test", "password": "test"} + # TODO: Create a static login token to make this test + # independent from /user/login + response = run_async(self.client.post, "/user/login", json=login) + self.assertEqual(response.status_code, 200) + + def test_types(self): + self.clear_lru_cache() + self.mock_types.get_available_states.return_value = async_return( + return_value=[{"state": "state"}, {"state": "state2"}] + ) + self.mock_types.get_available_sites.return_value = async_return( + return_value=[{"site": "site"}, {"site": "site2"}] + ) + self.mock_types.get_available_machine_types.return_value = async_return( + return_value=[{"machine_type": "type"}, {"machine_type": "type2"}] + ) + + self.clear_lru_cache() + response = run_async(self.client.get, "/types/states") + self.assertEqual(response.status_code, 200) + self.assertEqual(response.json(), ["state", "state2"]) + + self.clear_lru_cache() + response = run_async(self.client.get, "/types/sites") + self.assertEqual(response.status_code, 200) + self.assertEqual(response.json(), ["site", "site2"]) + + self.clear_lru_cache() + response = run_async(self.client.get, "/types/machine_types") + self.assertEqual(response.status_code, 200) + self.assertEqual(response.json(), ["type", "type2"]) + + # Invalid scope + self.clear_lru_cache() + self.login( + { + "user_name": "test1", + "password": "test", + "scopes": ["user:get"], + } + ) + response = run_async(self.client.get, "/types/states") + self.assertEqual(response.status_code, 403) diff --git a/tests/rest_t/routers_t/test_user.py b/tests/rest_t/routers_t/test_user.py index 75d3a595..0f947a09 100644 --- a/tests/rest_t/routers_t/test_user.py +++ b/tests/rest_t/routers_t/test_user.py @@ -4,7 +4,7 @@ # TODO: decrease code repetition by extracting basic auth test into separate function -class TestLogin(TestCaseRouters): +class TestUser(TestCaseRouters): # Reminder: When defining `setUp`, `setUpClass`, `tearDown` and `tearDownClass` # in router tests the corresponding super().function() needs to be called as well. def test_login(self): @@ -52,7 +52,10 @@ def test_login(self): self.assertEqual(response.status_code, 200) self.assertEqual( response.json(), - {"msg": "Successfully logged in!"}, + { + "msg": "Successfully logged in!", + "user": {"user_name": "test", "scopes": ["resources:get", "user:get"]}, + }, ) self.clear_lru_cache() @@ -101,6 +104,12 @@ def test_refresh(self): response = run_async(self.client.post, "/user/refresh") self.assertEqual(response.status_code, 200) + # invalid access token but valid refresh token + self.clear_lru_cache() + self.client.cookies["access_token_cookie"] = "invalid" + response = run_async(self.client.post, "/user/refresh") + self.assertEqual(response.status_code, 200) + def test_user_me(self): # Not logged in yet self.clear_lru_cache() @@ -110,10 +119,23 @@ def test_user_me(self): response.json(), {"detail": "Missing cookie access_token_cookie"} ) - # should work self.login() response = run_async(self.client.get, "/user/me") self.assertEqual(response.status_code, 200) self.assertEqual( - response.json(), {"user_name": "test", "scopes": ["resources:get"]} + response.json(), + {"user_name": "test", "scopes": ["resources:get", "user:get"]}, + ) + + def test_get_token_scopes(self): + self.clear_lru_cache() + self.login( + { + "user_name": "test1", + "password": "test", + "scopes": ["resources:get"], + } ) + response = run_async(self.client.get, "/user/token_scopes") + self.assertEqual(response.status_code, 200) + self.assertEqual(response.json(), ["resources:get"]) From d8b64a27e3d075770785feb7b22ee712c67e836d Mon Sep 17 00:00:00 2001 From: Alexander Haas Date: Fri, 29 Jul 2022 14:48:38 +0200 Subject: [PATCH 56/94] style: :art: Fixed linting and format errors --- tardis/rest/app/main.py | 2 +- tardis/rest/app/routers/types.py | 4 ++-- tests/rest_t/app_t/test_security.py | 2 -- tests/rest_t/routers_t/test_types.py | 1 - 4 files changed, 3 insertions(+), 6 deletions(-) diff --git a/tardis/rest/app/main.py b/tardis/rest/app/main.py index 6522c79e..8f7e9632 100644 --- a/tardis/rest/app/main.py +++ b/tardis/rest/app/main.py @@ -38,7 +38,7 @@ def authjwt_exception_handler(request: Request, exc: AuthJWTException): if detail == "Signature verification failed" or detail == "Signature has expired": exc.status_code = 401 - + return JSONResponse(status_code=exc.status_code, content={"detail": detail}) diff --git a/tardis/rest/app/routers/types.py b/tardis/rest/app/routers/types.py index 25c52ade..f219538d 100644 --- a/tardis/rest/app/routers/types.py +++ b/tardis/rest/app/routers/types.py @@ -30,7 +30,7 @@ async def get_resource_state( @router.get("/sites", description="Get all available sites") -async def get_resource_state( +async def get_resource_sites( sql_registry: SqliteRegistry = Depends(database.get_sql_registry()), _=Security(security.check_authorization, scopes=[Resources.get]), ): @@ -39,7 +39,7 @@ async def get_resource_state( @router.get("/machine_types", description="Get all available machine types") -async def get_resource_state( +async def get_resource_types( sql_registry: SqliteRegistry = Depends(database.get_sql_registry()), _=Security(security.check_authorization, scopes=[Resources.get]), ): diff --git a/tests/rest_t/app_t/test_security.py b/tests/rest_t/app_t/test_security.py index ef59897a..fc303831 100644 --- a/tests/rest_t/app_t/test_security.py +++ b/tests/rest_t/app_t/test_security.py @@ -1,6 +1,4 @@ -from starlette.types import Scope from tardis.exceptions.tardisexceptions import TardisError -from tardis.plugins.sqliteregistry import SqliteRegistry from tardis.rest.app.security import ( check_authentication, check_scope_permissions, diff --git a/tests/rest_t/routers_t/test_types.py b/tests/rest_t/routers_t/test_types.py index 494974dc..98027ca4 100644 --- a/tests/rest_t/routers_t/test_types.py +++ b/tests/rest_t/routers_t/test_types.py @@ -1,4 +1,3 @@ -from tardis.utilities.attributedict import AttributeDict from tests.rest_t.routers_t.base_test_case_routers import TestCaseRouters from tests.utilities.utilities import async_return, run_async From f38c884e2de7b098f60a68ffb0867eaf93a3e4e1 Mon Sep 17 00:00:00 2001 From: Alexander Haas Date: Fri, 29 Jul 2022 14:54:11 +0200 Subject: [PATCH 57/94] style: B950 --- tardis/rest/app/routers/user.py | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/tardis/rest/app/routers/user.py b/tardis/rest/app/routers/user.py index 8a6dc884..845a4b8b 100644 --- a/tardis/rest/app/routers/user.py +++ b/tardis/rest/app/routers/user.py @@ -8,7 +8,7 @@ @router.post( "/login", - description="Sets httponly access token in session cookie. The scopes are optional.", + description="Sets httponly access token in session cookie. The scopes are optional.", # noqa B950 ) async def login( login_user: security.LoginUser, @@ -19,7 +19,7 @@ async def login( if login_user.scopes is None: scopes = {"scopes": user.scopes} else: - # The next two lines are very critical as if wrongly implemented a user can give his token unlimited scopes. + # The next two lines are very critical as if wrongly implemented a user can give his token unlimited scopes. # noqa B950 # This functionality has to be tested thoroughly security.check_scope_permissions(login_user.scopes, user.scopes) scopes = {"scopes": login_user.scopes} @@ -34,7 +34,7 @@ async def login( Authorize.set_access_cookies(access_token) Authorize.set_refresh_cookies(refresh_token) - # It is ok to return the user here because every user should have the user:get scope. + # It is ok to return the user here because every user should have the user:get scope. # noqa B950 # Tokens don't have permissions to this function return { "msg": "Successfully logged in!", @@ -54,7 +54,8 @@ async def logout(Authorize: AuthJWT = Depends()): @router.post( - "/refresh", description="Use refresh token cookie to refresh expiration on cookie" + "/refresh", + description="Use refresh token cookie to refresh expiration on cookie", # noqa B950 ) async def refresh(Authorize: AuthJWT = Depends()): Authorize.jwt_refresh_token_required() From ba68bb792fd3b37e9b98be7c4a7884bf5ac44e0d Mon Sep 17 00:00:00 2001 From: Alexander Haas Date: Fri, 29 Jul 2022 14:56:19 +0200 Subject: [PATCH 58/94] flake8 --- tests/rest_t/app_t/test_security.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/rest_t/app_t/test_security.py b/tests/rest_t/app_t/test_security.py index fc303831..e8ee9167 100644 --- a/tests/rest_t/app_t/test_security.py +++ b/tests/rest_t/app_t/test_security.py @@ -63,7 +63,7 @@ def test_check_scope_permissions(self): check_scope_permissions(["resources:get"], ["resources:get"]) def test_check_authorization(self): - # It is very hard mocking the AuthJWT object. However, authorization is tested implicitly as an integration test in the router tests. + # It is very hard mocking the AuthJWT object. However, authorization is tested implicitly as an integration test in the router tests. # noqa B950 pass def test_check_authentication(self): From ebf5fbb2fe7231951bda211936b41f65731d1947 Mon Sep 17 00:00:00 2001 From: Alexander Haas Date: Tue, 2 Aug 2022 12:31:56 +0200 Subject: [PATCH 59/94] chore: :adhesive_bandage: changed delete logout to post --- tardis/rest/app/routers/user.py | 2 +- tests/rest_t/routers_t/test_user.py | 6 +++--- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/tardis/rest/app/routers/user.py b/tardis/rest/app/routers/user.py index 845a4b8b..3905cdaf 100644 --- a/tardis/rest/app/routers/user.py +++ b/tardis/rest/app/routers/user.py @@ -42,7 +42,7 @@ async def login( } -@router.delete( +@router.post( "/logout", description="Logout the current user by deleting all access token cookies", ) diff --git a/tests/rest_t/routers_t/test_user.py b/tests/rest_t/routers_t/test_user.py index 0f947a09..47ab7d7e 100644 --- a/tests/rest_t/routers_t/test_user.py +++ b/tests/rest_t/routers_t/test_user.py @@ -74,7 +74,7 @@ def test_login(self): def test_logout(self): # Not logged in yet self.clear_lru_cache() - response = run_async(self.client.delete, "/user/logout") + response = run_async(self.client.post, "/user/logout") self.assertEqual(response.status_code, 401) self.assertEqual( @@ -83,11 +83,11 @@ def test_logout(self): # correct login self.login() - response = run_async(self.client.delete, "/user/logout") + response = run_async(self.client.post, "/user/logout") self.assertEqual(response.status_code, 200) # prevent second logout - response = run_async(self.client.delete, "/user/logout") + response = run_async(self.client.post, "/user/logout") self.assertEqual(response.status_code, 401) def test_refresh(self): From a402613299b0d13ad93955430e05408ec5057dc8 Mon Sep 17 00:00:00 2001 From: Alexander Haas Date: Tue, 2 Aug 2022 12:34:43 +0200 Subject: [PATCH 60/94] chore: :heavy_plus_sign: Added the feature 'rest' as an extra. --- setup.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/setup.py b/setup.py index 6163c8de..a4087fc9 100644 --- a/setup.py +++ b/setup.py @@ -91,7 +91,6 @@ def get_cryptography_version(): "python-multipart", "typing_extensions", "backports.cached_property", - "fastapi-jwt-auth", ], extras_require={ "docs": [ @@ -100,6 +99,7 @@ def get_cryptography_version(): "sphinxcontrib-contentui", "myst_parser", ], + "rest": ["fastapi-jwt-auth"], "test": TESTS_REQUIRE, "contrib": ["flake8", "flake8-bugbear", "black; implementation_name=='cpython'"] + TESTS_REQUIRE, From b91cfa3d5d055fc305ed12edf55f73896e94180f Mon Sep 17 00:00:00 2001 From: Alexander Haas Date: Tue, 2 Aug 2022 12:39:49 +0200 Subject: [PATCH 61/94] fix: :heavy_plus_sign: Added dependency to TESTS_REQUIRE --- setup.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/setup.py b/setup.py index a4087fc9..93c0da7f 100644 --- a/setup.py +++ b/setup.py @@ -12,7 +12,7 @@ with open(os.path.join(repo_base_dir, "README.md"), "r") as read_me: long_description = read_me.read() -TESTS_REQUIRE = ["flake8", "httpx"] +TESTS_REQUIRE = ["flake8", "httpx", "fastapi-jwt-auth"] def get_cryptography_version(): From c58b6dc78b0044acd96882a83b99b7a28d2940c8 Mon Sep 17 00:00:00 2001 From: Alexander Haas Date: Tue, 16 Aug 2022 15:57:50 +0200 Subject: [PATCH 62/94] docs: Explanation for custom jwt exception handler --- tardis/rest/app/main.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/tardis/rest/app/main.py b/tardis/rest/app/main.py index 8f7e9632..54b98651 100644 --- a/tardis/rest/app/main.py +++ b/tardis/rest/app/main.py @@ -31,7 +31,8 @@ ], ) - +# the jwt auth package returns 422 on failed auth which doens't make sense +# This code changes that. There is currently an issue running: https://github.com/IndominusByte/fastapi-jwt-auth/issues/20 @app.exception_handler(AuthJWTException) def authjwt_exception_handler(request: Request, exc: AuthJWTException): detail = exc.message From cfd7eb5326e58970b206b631b1cbcd861f56139a Mon Sep 17 00:00:00 2001 From: Alexander Haas Date: Tue, 16 Aug 2022 15:58:08 +0200 Subject: [PATCH 63/94] chore: deleted unused scope --- tardis/rest/app/scopes.py | 1 - 1 file changed, 1 deletion(-) diff --git a/tardis/rest/app/scopes.py b/tardis/rest/app/scopes.py index 351afa4e..bbca86a0 100644 --- a/tardis/rest/app/scopes.py +++ b/tardis/rest/app/scopes.py @@ -11,4 +11,3 @@ class Resources(str, Enum): class User(str, Enum): get = "user:get" - put = "user:put" # usused From 05d0e45c35e6476d6845dab07f1800acbdac80b1 Mon Sep 17 00:00:00 2001 From: Alexander Haas Date: Tue, 16 Aug 2022 16:01:21 +0200 Subject: [PATCH 64/94] fix: changed import order --- tardis/rest/app/security.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tardis/rest/app/security.py b/tardis/rest/app/security.py index b8fe5ddb..b78c168f 100644 --- a/tardis/rest/app/security.py +++ b/tardis/rest/app/security.py @@ -11,7 +11,7 @@ from fastapi_jwt_auth import AuthJWT from functools import lru_cache -from typing import Optional, List +from typing import List, Optional class Settings(BaseModel): From 34b586a5ea082e9ec80ff65efc1b70edc3467696 Mon Sep 17 00:00:00 2001 From: Alexander Haas Date: Tue, 16 Aug 2022 16:01:37 +0200 Subject: [PATCH 65/94] fix: added type hints --- tardis/rest/app/routers/resources.py | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/tardis/rest/app/routers/resources.py b/tardis/rest/app/routers/resources.py index 6d20d6ef..a1adcb80 100644 --- a/tardis/rest/app/routers/resources.py +++ b/tardis/rest/app/routers/resources.py @@ -1,8 +1,8 @@ -from .. import security -from .. import crud, database +from .. import security, crud, database from ....plugins.sqliteregistry import SqliteRegistry from fastapi import APIRouter, Depends, HTTPException, Path, Security, status from ..scopes import Resources +from fastapi_jwt_auth import AuthJWT router = APIRouter(prefix="/resources", tags=["resources"]) @@ -11,7 +11,7 @@ async def get_resource_state( drone_uuid: str = Path(..., regex=r"^\S+-[A-Fa-f0-9]{10}$"), sql_registry: SqliteRegistry = Depends(database.get_sql_registry()), - _=Security(security.check_authorization, scopes=[Resources.get]), + _: AuthJWT = Security(security.check_authorization, scopes=[Resources.get]), ): query_result = await crud.get_resource_state(sql_registry, drone_uuid) try: @@ -26,7 +26,7 @@ async def get_resource_state( @router.get("/", description="Get list of managed resources") async def get_resources( sql_registry: SqliteRegistry = Depends(database.get_sql_registry()), - _=Security(security.check_authorization, scopes=[Resources.get]), + _: AuthJWT = Security(security.check_authorization, scopes=[Resources.get]), ): query_result = await crud.get_resources(sql_registry) return query_result @@ -34,7 +34,7 @@ async def get_resources( @router.delete("/{drone_uuid}/drain", description="Gently shut shown drone") async def shutdown_drone( - _=Security(security.check_authorization, scopes=[Resources.delete]), + _: AuthJWT = Security(security.check_authorization, scopes=[Resources.delete]), ): # TODO: Implement return {"msg": "Not implemented"} From e2dbabd50c8d48fa5d0f8e7a560c4c24ae37fe77 Mon Sep 17 00:00:00 2001 From: Alexander Haas Date: Tue, 16 Aug 2022 16:02:08 +0200 Subject: [PATCH 66/94] fix: added type hints in types.py --- tardis/rest/app/routers/types.py | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/tardis/rest/app/routers/types.py b/tardis/rest/app/routers/types.py index f219538d..28b6b26a 100644 --- a/tardis/rest/app/routers/types.py +++ b/tardis/rest/app/routers/types.py @@ -6,6 +6,7 @@ from ....plugins.sqliteregistry import SqliteRegistry from fastapi import APIRouter, Depends, Security from ..scopes import Resources +from fastapi_jwt_auth import AuthJWT router = APIRouter(prefix="/types", tags=["types", "resources"]) @@ -23,7 +24,7 @@ def sql_to_list(query_result: List[Dict]) -> List[str]: @router.get("/states", description="Get all available states") async def get_resource_state( sql_registry: SqliteRegistry = Depends(database.get_sql_registry()), - _=Security(security.check_authorization, scopes=[Resources.get]), + _: AuthJWT = Security(security.check_authorization, scopes=[Resources.get]), ): query_result = await crud.get_available_states(sql_registry) return sql_to_list(query_result) @@ -32,7 +33,7 @@ async def get_resource_state( @router.get("/sites", description="Get all available sites") async def get_resource_sites( sql_registry: SqliteRegistry = Depends(database.get_sql_registry()), - _=Security(security.check_authorization, scopes=[Resources.get]), + _: AuthJWT = Security(security.check_authorization, scopes=[Resources.get]), ): query_result = await crud.get_available_sites(sql_registry) return sql_to_list(query_result) @@ -41,7 +42,7 @@ async def get_resource_sites( @router.get("/machine_types", description="Get all available machine types") async def get_resource_types( sql_registry: SqliteRegistry = Depends(database.get_sql_registry()), - _=Security(security.check_authorization, scopes=[Resources.get]), + _: AuthJWT = Security(security.check_authorization, scopes=[Resources.get]), ): query_result = await crud.get_available_machine_types(sql_registry) return sql_to_list(query_result) From 3d47622a7bb0b0dbfac78e662053643d8fff1fc4 Mon Sep 17 00:00:00 2001 From: Alexander Haas Date: Tue, 16 Aug 2022 16:16:02 +0200 Subject: [PATCH 67/94] chore: deprectate the get_algorithm() function --- tardis/rest/app/security.py | 14 -------------- tests/rest_t/app_t/test_security.py | 12 ------------ tests/rest_t/routers_t/base_test_case_routers.py | 3 +-- 3 files changed, 1 insertion(+), 28 deletions(-) diff --git a/tardis/rest/app/security.py b/tardis/rest/app/security.py index b78c168f..a069bdf6 100644 --- a/tardis/rest/app/security.py +++ b/tardis/rest/app/security.py @@ -98,20 +98,6 @@ def get_token_scopes(Authorize: AuthJWT) -> List[str]: return token_scopes -@lru_cache(maxsize=1) -def get_algorithm() -> str: - # This can probably be deprecated as fastapi_jwt_tokens - # sets it's algorithm by itself - try: - rest_service = Configuration().Services.restapi - except AttributeError: - raise TardisError( - "TARDIS RestService not configured while accessing algorithm!" - ) from None - else: - return rest_service.algorithm - - @lru_cache(maxsize=16) def get_user(user_name: str) -> Optional[DatabaseUser]: try: diff --git a/tests/rest_t/app_t/test_security.py b/tests/rest_t/app_t/test_security.py index e8ee9167..066ed773 100644 --- a/tests/rest_t/app_t/test_security.py +++ b/tests/rest_t/app_t/test_security.py @@ -2,7 +2,6 @@ from tardis.rest.app.security import ( check_authentication, check_scope_permissions, - get_algorithm, get_user, hash_password, ) @@ -49,7 +48,6 @@ def mocked_get_user(user_name): @staticmethod def clear_lru_cache(): - get_algorithm.cache_clear() get_user.cache_clear() def test_check_scope_permissions(self): @@ -90,16 +88,6 @@ def test_check_authentication(self): }, ) - def test_get_algorithm(self): - self.clear_lru_cache() - self.assertEqual(get_algorithm(), self.algorithm) - - self.clear_lru_cache() - self.mock_config.side_effect = AttributeError - with self.assertRaises(TardisError): - get_algorithm() - self.mock_config.side_effect = None - def test_get_user(self): self.clear_lru_cache() self.assertEqual( diff --git a/tests/rest_t/routers_t/base_test_case_routers.py b/tests/rest_t/routers_t/base_test_case_routers.py index aeedffbb..e11ffdea 100644 --- a/tests/rest_t/routers_t/base_test_case_routers.py +++ b/tests/rest_t/routers_t/base_test_case_routers.py @@ -1,4 +1,4 @@ -from tardis.rest.app.security import get_algorithm, get_user +from tardis.rest.app.security import get_user from tardis.utilities.attributedict import AttributeDict from tests.utilities.utilities import run_async @@ -65,5 +65,4 @@ def login(self, user: dict = None): @staticmethod def clear_lru_cache(): - get_algorithm.cache_clear() get_user.cache_clear() From b3565ed77adaee006ef3f8b45f7b621cdad3a60e Mon Sep 17 00:00:00 2001 From: Alexander Haas Date: Tue, 16 Aug 2022 16:45:23 +0200 Subject: [PATCH 68/94] fix: code quality in sql_to_list() --- tardis/rest/app/routers/types.py | 6 +----- 1 file changed, 1 insertion(+), 5 deletions(-) diff --git a/tardis/rest/app/routers/types.py b/tardis/rest/app/routers/types.py index 28b6b26a..7cfe93e9 100644 --- a/tardis/rest/app/routers/types.py +++ b/tardis/rest/app/routers/types.py @@ -1,5 +1,4 @@ from typing import Dict, List - from tardis.exceptions.tardisexceptions import TardisError from .. import security from .. import crud, database @@ -12,13 +11,10 @@ def sql_to_list(query_result: List[Dict]) -> List[str]: - out = [] try: - for val in query_result: - out.append(list(val.values())[0]) + return [list(pair.values())[0] for pair in query_result] except Exception as e: raise TardisError("Query result has invalid format") from e - return out @router.get("/states", description="Get all available states") From e2670f9cde513ec905551f999b2e7b6c149e7ed8 Mon Sep 17 00:00:00 2001 From: Alexander Haas Date: Tue, 16 Aug 2022 16:46:08 +0200 Subject: [PATCH 69/94] feat: scope guard for user:get scope in login func --- tardis/rest/app/routers/user.py | 19 +++++++++++++------ 1 file changed, 13 insertions(+), 6 deletions(-) diff --git a/tardis/rest/app/routers/user.py b/tardis/rest/app/routers/user.py index 3905cdaf..630030ab 100644 --- a/tardis/rest/app/routers/user.py +++ b/tardis/rest/app/routers/user.py @@ -16,6 +16,7 @@ async def login( ): user = security.check_authentication(login_user.user_name, login_user.password) + # set and check the scopes that are applied to the returned token if login_user.scopes is None: scopes = {"scopes": user.scopes} else: @@ -34,12 +35,18 @@ async def login( Authorize.set_access_cookies(access_token) Authorize.set_refresh_cookies(refresh_token) - # It is ok to return the user here because every user should have the user:get scope. # noqa B950 - # Tokens don't have permissions to this function - return { - "msg": "Successfully logged in!", - "user": security.BaseUser(user_name=user.user_name, scopes=scopes["scopes"]), - } + # If the user doesn't have the user:get scope, he can't get the user data. # noqa B950 + if User.get not in user.scopes: + return { + "msg": "Successfully logged in", + } + else: + return { + "msg": "Successfully logged in!", + "user": security.BaseUser( + user_name=user.user_name, scopes=scopes["scopes"] + ), + } @router.post( From 986290759a9bfe5656e709e3abc5792f74cbb24e Mon Sep 17 00:00:00 2001 From: Alexander Haas Date: Tue, 16 Aug 2022 17:08:27 +0200 Subject: [PATCH 70/94] fix: added rest as a feature properly --- setup.py | 28 ++++++++++++++++++---------- 1 file changed, 18 insertions(+), 10 deletions(-) diff --git a/setup.py b/setup.py index 93c0da7f..2e565b95 100644 --- a/setup.py +++ b/setup.py @@ -12,7 +12,16 @@ with open(os.path.join(repo_base_dir, "README.md"), "r") as read_me: long_description = read_me.read() -TESTS_REQUIRE = ["flake8", "httpx", "fastapi-jwt-auth"] +TESTS_REQUIRE = ["flake8", "httpx"] +REST_REQUIRES = [ + "fastapi-jwt-auth", + "fastapi", + "python-jose", + "uvicorn[standard]<=0.14.0", # to support python3.6 (Centos 7) + "typer", + "bcrypt", + "python-multipart", +] def get_cryptography_version(): @@ -83,12 +92,6 @@ def get_cryptography_version(): "kubernetes_asyncio", "pydantic", "asyncstdlib", - "fastapi", - "python-jose", - "uvicorn[standard]<=0.14.0", # to support python3.6 (Centos 7) - "typer", - "bcrypt", - "python-multipart", "typing_extensions", "backports.cached_property", ], @@ -99,10 +102,15 @@ def get_cryptography_version(): "sphinxcontrib-contentui", "myst_parser", ], - "rest": ["fastapi-jwt-auth"], + "rest": REST_REQUIRES, "test": TESTS_REQUIRE, - "contrib": ["flake8", "flake8-bugbear", "black; implementation_name=='cpython'"] - + TESTS_REQUIRE, + "contrib": [ + "flake8", + "flake8-bugbear", + "black; implementation_name=='cpython'", + ] + + TESTS_REQUIRE + + REST_REQUIRES, }, tests_require=TESTS_REQUIRE, zip_safe=False, From 8ffaf2fc5e8b4f8ea3c0a5e8446f396402a5158b Mon Sep 17 00:00:00 2001 From: Alexander Haas Date: Tue, 16 Aug 2022 17:08:50 +0200 Subject: [PATCH 71/94] fix: DELETE -> PATCH in /drain --- tardis/rest/app/routers/resources.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tardis/rest/app/routers/resources.py b/tardis/rest/app/routers/resources.py index a1adcb80..068d2eb3 100644 --- a/tardis/rest/app/routers/resources.py +++ b/tardis/rest/app/routers/resources.py @@ -32,7 +32,7 @@ async def get_resources( return query_result -@router.delete("/{drone_uuid}/drain", description="Gently shut shown drone") +@router.patch("/{drone_uuid}/drain", description="Gently shut shown drone") async def shutdown_drone( _: AuthJWT = Security(security.check_authorization, scopes=[Resources.delete]), ): From 2996a5e88972f1e760f5f8b26c7294c76c34d58a Mon Sep 17 00:00:00 2001 From: Alexander Haas Date: Wed, 17 Aug 2022 09:45:57 +0200 Subject: [PATCH 72/94] chore: properly deprecated algorithm config --- tardis/rest/service.py | 8 +------- 1 file changed, 1 insertion(+), 7 deletions(-) diff --git a/tardis/rest/service.py b/tardis/rest/service.py index 1aaf7e3c..e739d768 100644 --- a/tardis/rest/service.py +++ b/tardis/rest/service.py @@ -15,22 +15,16 @@ class RestService(object): def __init__( self, - algorithm: str = "HS256", users: List = None, **fast_api_args, ): - self._algorithm = algorithm self._users = users or [] # necessary to avoid that the TARDIS' logger configuration is overwritten! if "log_config" not in fast_api_args: fast_api_args["log_config"] = None - self._config = Config("tardis.rest.app.main:app", **fast_api_args) - @property - @lru_cache(maxsize=16) - def algorithm(self) -> str: - return self._algorithm + self._config = Config("tardis.rest.app.main:app", **fast_api_args) @lru_cache(maxsize=16) def get_user(self, user_name: str) -> Optional[DatabaseUser]: From 7648a0351ae47bbce065f9f8da884b4ef1172660 Mon Sep 17 00:00:00 2001 From: Alexander Haas Date: Wed, 17 Aug 2022 09:46:31 +0200 Subject: [PATCH 73/94] feat: implemented the shutdown_drone() API --- tardis/rest/app/crud.py | 8 ++++++++ tardis/rest/app/routers/resources.py | 8 +++++--- 2 files changed, 13 insertions(+), 3 deletions(-) diff --git a/tardis/rest/app/crud.py b/tardis/rest/app/crud.py index 1dfdc417..ee87e8e1 100644 --- a/tardis/rest/app/crud.py +++ b/tardis/rest/app/crud.py @@ -31,3 +31,11 @@ async def get_available_sites(sql_registry): async def get_available_machine_types(sql_registry): sql_query = "SELECT machine_type FROM MachineTypes" return await sql_registry.async_execute(sql_query, {}) + + +async def set_state_to_draining(sql_registry, drone_uuid: str): + sql_query = """ + UPDATE Resources + SET state_id = (SELECT state_id FROM ResourceStates WHERE state = 'DrainState') + WHERE drone_uuid = :drone_uuid""" + return await sql_registry.async_execute(sql_query, dict(drone_uuid=drone_uuid)) \ No newline at end of file diff --git a/tardis/rest/app/routers/resources.py b/tardis/rest/app/routers/resources.py index 068d2eb3..f4e73f4b 100644 --- a/tardis/rest/app/routers/resources.py +++ b/tardis/rest/app/routers/resources.py @@ -34,7 +34,9 @@ async def get_resources( @router.patch("/{drone_uuid}/drain", description="Gently shut shown drone") async def shutdown_drone( - _: AuthJWT = Security(security.check_authorization, scopes=[Resources.delete]), + drone_uuid: str = Path(..., regex=r"^\S+-[A-Fa-f0-9]{10}$"), + sql_registry: SqliteRegistry = Depends(database.get_sql_registry()), + _: AuthJWT = Security(security.check_authorization, scopes=[Resources.patch]), ): - # TODO: Implement - return {"msg": "Not implemented"} + await crud.set_state_to_draining(sql_registry, drone_uuid) + return {"msg": "Drone set to DrainState"} From 0955055cef6e7a2ea6208ac1bed8a62b05bcc3b1 Mon Sep 17 00:00:00 2001 From: Alexander Haas Date: Wed, 17 Aug 2022 09:46:57 +0200 Subject: [PATCH 74/94] fix: replaced TardisError with HTTPException --- tardis/rest/app/security.py | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/tardis/rest/app/security.py b/tardis/rest/app/security.py index a069bdf6..8c48a9b2 100644 --- a/tardis/rest/app/security.py +++ b/tardis/rest/app/security.py @@ -94,7 +94,10 @@ def get_token_scopes(Authorize: AuthJWT) -> List[str]: try: token_scopes: List[str] = Authorize.get_raw_jwt()["scopes"] except KeyError: - raise TardisError("Scopes not defined in jwt token") from None + raise HTTPException( + status_code=status.HTTP_400_BAD_REQUEST, + detail="Invalid token/no scopes in token", + ) return token_scopes @@ -110,6 +113,7 @@ def get_user(user_name: str) -> Optional[DatabaseUser]: return rest_service.get_user(user_name) + def hash_password(password: str) -> bytes: salt = gensalt() return hashpw(password.encode(), salt) From 00ae2031fa7888af7fc8c140f1a70751ba0897d1 Mon Sep 17 00:00:00 2001 From: Alexander Haas Date: Wed, 17 Aug 2022 09:47:27 +0200 Subject: [PATCH 75/94] chore: removed unused scope enum variants --- tardis/rest/app/scopes.py | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/tardis/rest/app/scopes.py b/tardis/rest/app/scopes.py index bbca86a0..fc401cb0 100644 --- a/tardis/rest/app/scopes.py +++ b/tardis/rest/app/scopes.py @@ -5,8 +5,7 @@ class Resources(str, Enum): get = "resources:get" - put = "resources:put" - delete = "resources:delete" + patch = "resources:patch" class User(str, Enum): From 43bb02d76d732c4284a9851c98253b2da1570119 Mon Sep 17 00:00:00 2001 From: Alexander Haas Date: Thu, 18 Aug 2022 13:04:00 +0200 Subject: [PATCH 76/94] some comments --- tardis/rest/app/security.py | 13 +++++++++---- 1 file changed, 9 insertions(+), 4 deletions(-) diff --git a/tardis/rest/app/security.py b/tardis/rest/app/security.py index 8c48a9b2..e406a2e4 100644 --- a/tardis/rest/app/security.py +++ b/tardis/rest/app/security.py @@ -19,14 +19,18 @@ class Settings(BaseModel): authjwt_token_location: set = {"cookies"} authjwt_cookie_samesite: str = "strict" # TODO: change this to true in production so only https traffic is allowed + # Service meant to be used with https proxy authjwt_cookie_secure: bool = False - # TODO: Set this too to True. But as soon as possible. - # Update: As 'same_site' is strict this is probably enough. + # As 'same_site' is strict this is probably enough. authjwt_cookie_csrf_protect: bool = False + def __init__(self): + self.authjwt_cookie_secure = Configuration().Services.restapi._dev + @AuthJWT.load_config def get_config(): + # TODO: Solve - AttributeError: Configuration().Services return Settings() @@ -49,13 +53,15 @@ class DatabaseUser(BaseUser): def check_scope_permissions(requested_scopes: List[str], allowed_scopes: List[str]): + # All requested scopes must be contained in allowed_scopes for scope in requested_scopes: if scope not in allowed_scopes: raise HTTPException( status_code=status.HTTP_403_FORBIDDEN, detail={ "msg": "Not enough permissions", - "scopesRequired": requested_scopes, + "failedAt": scope, + "allowedScopes": allowed_scopes, }, ) from None @@ -113,7 +119,6 @@ def get_user(user_name: str) -> Optional[DatabaseUser]: return rest_service.get_user(user_name) - def hash_password(password: str) -> bytes: salt = gensalt() return hashpw(password.encode(), salt) From 0a631fca0eea52bccaed46c3650ce483da0ec64a Mon Sep 17 00:00:00 2001 From: Alexander Haas Date: Thu, 18 Aug 2022 14:06:23 +0200 Subject: [PATCH 77/94] fix: deleted init to settings --- tardis/rest/app/security.py | 3 --- 1 file changed, 3 deletions(-) diff --git a/tardis/rest/app/security.py b/tardis/rest/app/security.py index e406a2e4..71e64bcc 100644 --- a/tardis/rest/app/security.py +++ b/tardis/rest/app/security.py @@ -24,9 +24,6 @@ class Settings(BaseModel): # As 'same_site' is strict this is probably enough. authjwt_cookie_csrf_protect: bool = False - def __init__(self): - self.authjwt_cookie_secure = Configuration().Services.restapi._dev - @AuthJWT.load_config def get_config(): From 1c96bd15bd8703305094d2a0dde52671527103ad Mon Sep 17 00:00:00 2001 From: Alexander Haas Date: Thu, 18 Aug 2022 14:18:56 +0200 Subject: [PATCH 78/94] feat: expiration param in /login --- tardis/rest/app/routers/user.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/tardis/rest/app/routers/user.py b/tardis/rest/app/routers/user.py index 630030ab..e5a6b934 100644 --- a/tardis/rest/app/routers/user.py +++ b/tardis/rest/app/routers/user.py @@ -12,6 +12,7 @@ ) async def login( login_user: security.LoginUser, + expires_delta: int | None = None, Authorize: AuthJWT = Depends(), ): user = security.check_authentication(login_user.user_name, login_user.password) @@ -26,7 +27,7 @@ async def login( scopes = {"scopes": login_user.scopes} access_token = Authorize.create_access_token( - subject=user.user_name, user_claims=scopes + subject=user.user_name, user_claims=scopes, expires_time=expires_delta ) refresh_token = Authorize.create_refresh_token( subject=user.user_name, user_claims=scopes From 5fe3347b5cf175f548930d998fdec862d0d5d789 Mon Sep 17 00:00:00 2001 From: Alexander Haas Date: Thu, 18 Aug 2022 14:41:14 +0200 Subject: [PATCH 79/94] test: fixed failing algo tests --- tests/rest_t/test_service.py | 10 +--------- 1 file changed, 1 insertion(+), 9 deletions(-) diff --git a/tests/rest_t/test_service.py b/tests/rest_t/test_service.py index 117b6fd2..eac18a62 100644 --- a/tests/rest_t/test_service.py +++ b/tests/rest_t/test_service.py @@ -7,10 +7,7 @@ class TestRestService(TestCase): def setUp(self) -> None: - self.algorithm = "test_algorithm" - self.rest_service = RestService( - algorithm=self.algorithm, - ) + self.rest_service = RestService() @patch("tardis.rest.service.Server") def test_run(self, mocked_server): @@ -21,10 +18,6 @@ def test_run(self, mocked_server): run_async(self.rest_service.run) mocked_server.assert_called_with(config=self.rest_service._config) - def test_algorithm(self): - self.assertEqual(self.rest_service.algorithm, self.algorithm) - self.assertEqual(RestService().algorithm, "HS256") - def test_get_user(self): self.assertIsNone(self.rest_service.get_user(user_name="test")) @@ -35,7 +28,6 @@ def test_get_user(self): } rest_service = RestService( - algorithm=self.algorithm, users=[user], ) From 427afe31487cfc5720d0bd6f80857646cff977f3 Mon Sep 17 00:00:00 2001 From: Alexander Haas Date: Fri, 19 Aug 2022 16:08:46 +0200 Subject: [PATCH 80/94] fix: added '!' for consistency --- tardis/rest/app/routers/user.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tardis/rest/app/routers/user.py b/tardis/rest/app/routers/user.py index e5a6b934..1df71f06 100644 --- a/tardis/rest/app/routers/user.py +++ b/tardis/rest/app/routers/user.py @@ -39,7 +39,7 @@ async def login( # If the user doesn't have the user:get scope, he can't get the user data. # noqa B950 if User.get not in user.scopes: return { - "msg": "Successfully logged in", + "msg": "Successfully logged in!", } else: return { From f6606f05c18ace1426f0ce9bdccbeac847247859 Mon Sep 17 00:00:00 2001 From: Alexander Haas Date: Fri, 19 Aug 2022 16:09:41 +0200 Subject: [PATCH 81/94] test: made HTTPException more specific --- tests/rest_t/app_t/test_security.py | 21 +++++++++++++++------ 1 file changed, 15 insertions(+), 6 deletions(-) diff --git a/tests/rest_t/app_t/test_security.py b/tests/rest_t/app_t/test_security.py index 066ed773..cda79089 100644 --- a/tests/rest_t/app_t/test_security.py +++ b/tests/rest_t/app_t/test_security.py @@ -51,12 +51,21 @@ def clear_lru_cache(): get_user.cache_clear() def test_check_scope_permissions(self): - self.assertRaises( - HTTPException, - check_scope_permissions, - ["resources:get"], - ["user:get"], - ) + with self.assertRaises(HTTPException) as cm: + check_scope_permissions( + ["user:get", "resources:get"], ["user:get", "user:put"] + ) + self.assertEqual( + cm.exception, + HTTPException( + status_code=status.HTTP_403_FORBIDDEN, + detail={ + "msg": "Not enough permissions", + "failedAt": "resources:get", + "allowedScopes": ["user:get", "user:put"], + }, + ), + ) check_scope_permissions(["resources:get"], ["resources:get"]) From cd700f616e86a382740b76c9e7d5b9f909308ac2 Mon Sep 17 00:00:00 2001 From: Alexander Haas Date: Fri, 19 Aug 2022 16:10:04 +0200 Subject: [PATCH 82/94] feat: utility functions and fixed scopes --- tests/rest_t/routers_t/base_test_case_routers.py | 14 ++++++++++---- 1 file changed, 10 insertions(+), 4 deletions(-) diff --git a/tests/rest_t/routers_t/base_test_case_routers.py b/tests/rest_t/routers_t/base_test_case_routers.py index e11ffdea..ec2d65a6 100644 --- a/tests/rest_t/routers_t/base_test_case_routers.py +++ b/tests/rest_t/routers_t/base_test_case_routers.py @@ -33,14 +33,11 @@ def tearDownClass(cls) -> None: cls.mock_config_patcher.stop() def setUp(self) -> None: - algorithm = "HS256" - self.config = self.mock_config.return_value - self.config.Services.restapi.algorithm = algorithm self.config.Services.restapi.get_user.return_value = AttributeDict( user_name="test", hashed_password="$2b$12$Gkl8KYNGRMhx4kB0bKJnyuRuzOrx3LZlWf1CReIsDk9HyWoUGBihG", # noqa B509 - scopes=["resources:get", "user:get"], + scopes=["resources:get", "user:get", "resources:patch"], ) from tardis.rest.app.main import ( @@ -53,6 +50,15 @@ def setUp(self) -> None: "password": "test", } + def set_scopes(self, scopes: list): + self.config.Services.restapi.get_user.return_value.scopes = scopes + + def reset_scopes(self): + self.set_scopes(["resources:get", "user:get", "resources:patch"]) + + def get_scopes(self): + return self.config.Services.restapi.get_user.return_value.scopes + def tearDown(self) -> None: run_async(self.client.aclose) From d44f526fc5d6bb45bd2475e3926dbff293ac3205 Mon Sep 17 00:00:00 2001 From: Alexander Haas Date: Fri, 19 Aug 2022 16:11:00 +0200 Subject: [PATCH 83/94] test: added passing test for draining drones --- tests/rest_t/routers_t/test_resources.py | 14 ++++++++++++++ 1 file changed, 14 insertions(+) diff --git a/tests/rest_t/routers_t/test_resources.py b/tests/rest_t/routers_t/test_resources.py index dc42e256..81f2ee60 100644 --- a/tests/rest_t/routers_t/test_resources.py +++ b/tests/rest_t/routers_t/test_resources.py @@ -89,3 +89,17 @@ def test_get_resources(self): response.json(), full_expected_resources, ) + + def test_shutdown_drone(self): + self.clear_lru_cache() + self.mock_crud.set_state_to_draining.return_value = async_return() + + response = run_async(self.client.patch, "/resources/test-0125bc9fd8/drain") + self.assertEqual(response.status_code, 200) + self.assertEqual(response.json(), {"msg": "Drone set to DrainState"}) + + # missing scope + self.set_scopes(["resources:get"]) + self.login() + response = run_async(self.client.patch, "/resources/test-0125bc9fd8/drain") + self.assertEqual(response.status_code, 403) From 6d7fbef352a81e8069d264beab506c0825105a7b Mon Sep 17 00:00:00 2001 From: Alexander Haas Date: Fri, 19 Aug 2022 16:12:57 +0200 Subject: [PATCH 84/94] test: improved authorization tests --- tests/rest_t/routers_t/test_resources.py | 19 ++++++++++++++----- tests/rest_t/routers_t/test_types.py | 16 ++++------------ tests/rest_t/routers_t/test_user.py | 24 ++++++++++++++++++++---- 3 files changed, 38 insertions(+), 21 deletions(-) diff --git a/tests/rest_t/routers_t/test_resources.py b/tests/rest_t/routers_t/test_resources.py index 81f2ee60..134fc08f 100644 --- a/tests/rest_t/routers_t/test_resources.py +++ b/tests/rest_t/routers_t/test_resources.py @@ -7,11 +7,8 @@ class TestResources(TestCaseRouters): # in router tests the corresponding super().function() needs to be called as well. def setUp(self): super().setUp() - login = {"user_name": "test", "password": "test"} - # TODO: Create a static login token to make this test - # independent from /user/login - response = run_async(self.client.post, "/user/login", json=login) - self.assertEqual(response.status_code, 200) + self.reset_scopes() + self.login() def test_get_resource_state(self): self.clear_lru_cache() @@ -57,6 +54,12 @@ def test_get_resource_state(self): self.assertEqual(response.status_code, 404) self.assertEqual(response.json(), {"detail": "Not Found"}) + # missing scope + self.set_scopes(["resources:patch"]) + self.login() + response = run_async(self.client.get, "/resources/test-0123456789/state") + self.assertEqual(response.status_code, 403) + def test_get_resources(self): self.clear_lru_cache() full_expected_resources = [ @@ -90,6 +93,12 @@ def test_get_resources(self): full_expected_resources, ) + # missing scope + self.set_scopes(["resources:patch"]) + self.login() + response = run_async(self.client.get, "/resources/") + self.assertEqual(response.status_code, 403) + def test_shutdown_drone(self): self.clear_lru_cache() self.mock_crud.set_state_to_draining.return_value = async_return() diff --git a/tests/rest_t/routers_t/test_types.py b/tests/rest_t/routers_t/test_types.py index 98027ca4..309df159 100644 --- a/tests/rest_t/routers_t/test_types.py +++ b/tests/rest_t/routers_t/test_types.py @@ -9,11 +9,8 @@ def is_list_str(resp): class TestTypes(TestCaseRouters): def setUp(self) -> None: super().setUp() - login = {"user_name": "test", "password": "test"} - # TODO: Create a static login token to make this test - # independent from /user/login - response = run_async(self.client.post, "/user/login", json=login) - self.assertEqual(response.status_code, 200) + self.reset_scopes() + self.login() def test_types(self): self.clear_lru_cache() @@ -44,12 +41,7 @@ def test_types(self): # Invalid scope self.clear_lru_cache() - self.login( - { - "user_name": "test1", - "password": "test", - "scopes": ["user:get"], - } - ) + self.set_scopes(["resources:patch"]) + self.login() response = run_async(self.client.get, "/types/states") self.assertEqual(response.status_code, 403) diff --git a/tests/rest_t/routers_t/test_user.py b/tests/rest_t/routers_t/test_user.py index 47ab7d7e..f29796e4 100644 --- a/tests/rest_t/routers_t/test_user.py +++ b/tests/rest_t/routers_t/test_user.py @@ -1,8 +1,6 @@ from tests.rest_t.routers_t.base_test_case_routers import TestCaseRouters from tests.utilities.utilities import run_async -# TODO: decrease code repetition by extracting basic auth test into separate function - class TestUser(TestCaseRouters): # Reminder: When defining `setUp`, `setUpClass`, `tearDown` and `tearDownClass` @@ -54,7 +52,19 @@ def test_login(self): response.json(), { "msg": "Successfully logged in!", - "user": {"user_name": "test", "scopes": ["resources:get", "user:get"]}, + "user": {"user_name": "test", "scopes": self.get_scopes()}, + }, + ) + + # missing scopes + self.clear_lru_cache() + self.set_scopes(["resources:get"]) + response = run_async(self.client.post, "/user/login", json=self.test_user) + self.assertEqual(response.status_code, 200) + self.assertEqual( + response.json(), + { + "msg": "Successfully logged in!", }, ) @@ -124,9 +134,15 @@ def test_user_me(self): self.assertEqual(response.status_code, 200) self.assertEqual( response.json(), - {"user_name": "test", "scopes": ["resources:get", "user:get"]}, + {"user_name": "test", "scopes": self.get_scopes()}, ) + # missing scope + self.set_scopes(["resources:get"]) + self.login() + response = run_async(self.client.get, "/user/me") + self.assertEqual(response.status_code, 403) + def test_get_token_scopes(self): self.clear_lru_cache() self.login( From 7f54f0fe5ee8d9277b0c69476b89888387e4b682 Mon Sep 17 00:00:00 2001 From: Alexander Haas Date: Fri, 19 Aug 2022 16:15:41 +0200 Subject: [PATCH 85/94] style: formatting and linting fixes --- tardis/rest/app/crud.py | 2 +- tardis/rest/app/main.py | 3 ++- tardis/rest/app/security.py | 2 +- 3 files changed, 4 insertions(+), 3 deletions(-) diff --git a/tardis/rest/app/crud.py b/tardis/rest/app/crud.py index ee87e8e1..37a3035d 100644 --- a/tardis/rest/app/crud.py +++ b/tardis/rest/app/crud.py @@ -38,4 +38,4 @@ async def set_state_to_draining(sql_registry, drone_uuid: str): UPDATE Resources SET state_id = (SELECT state_id FROM ResourceStates WHERE state = 'DrainState') WHERE drone_uuid = :drone_uuid""" - return await sql_registry.async_execute(sql_query, dict(drone_uuid=drone_uuid)) \ No newline at end of file + return await sql_registry.async_execute(sql_query, dict(drone_uuid=drone_uuid)) diff --git a/tardis/rest/app/main.py b/tardis/rest/app/main.py index 54b98651..28ebe1f7 100644 --- a/tardis/rest/app/main.py +++ b/tardis/rest/app/main.py @@ -31,8 +31,9 @@ ], ) + # the jwt auth package returns 422 on failed auth which doens't make sense -# This code changes that. There is currently an issue running: https://github.com/IndominusByte/fastapi-jwt-auth/issues/20 +# This code changes that. There is currently an issue running: https://github.com/IndominusByte/fastapi-jwt-auth/issues/20 # noqa B950 @app.exception_handler(AuthJWTException) def authjwt_exception_handler(request: Request, exc: AuthJWTException): detail = exc.message diff --git a/tardis/rest/app/security.py b/tardis/rest/app/security.py index 71e64bcc..b8c13790 100644 --- a/tardis/rest/app/security.py +++ b/tardis/rest/app/security.py @@ -100,7 +100,7 @@ def get_token_scopes(Authorize: AuthJWT) -> List[str]: raise HTTPException( status_code=status.HTTP_400_BAD_REQUEST, detail="Invalid token/no scopes in token", - ) + ) from None return token_scopes From 071aac4ebb03c31bf706db193a40c6ea98f3f402 Mon Sep 17 00:00:00 2001 From: Alexander Haas Date: Fri, 19 Aug 2022 16:20:09 +0200 Subject: [PATCH 86/94] fix: adjusted type hint for 3.8 --- tardis/rest/app/routers/user.py | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/tardis/rest/app/routers/user.py b/tardis/rest/app/routers/user.py index 1df71f06..d560b378 100644 --- a/tardis/rest/app/routers/user.py +++ b/tardis/rest/app/routers/user.py @@ -1,3 +1,5 @@ +from optparse import Option +from typing import Optional from ..scopes import User from .. import security from fastapi import APIRouter, Depends, Security @@ -12,7 +14,7 @@ ) async def login( login_user: security.LoginUser, - expires_delta: int | None = None, + expires_delta: Optional[int] = None, Authorize: AuthJWT = Depends(), ): user = security.check_authentication(login_user.user_name, login_user.password) From dc16742c9d9c0ec91b3a4ce36801b6be918c52d2 Mon Sep 17 00:00:00 2001 From: Alexander Haas Date: Fri, 19 Aug 2022 16:21:44 +0200 Subject: [PATCH 87/94] chore: removed unused import --- drone_registry.db-shm | Bin 0 -> 32768 bytes drone_registry.db-wal | 0 tardis/rest/app/routers/user.py | 1 - 3 files changed, 1 deletion(-) create mode 100644 drone_registry.db-shm create mode 100644 drone_registry.db-wal diff --git a/drone_registry.db-shm b/drone_registry.db-shm new file mode 100644 index 0000000000000000000000000000000000000000..fe9ac2845eca6fe6da8a63cd096d9cf9e24ece10 GIT binary patch literal 32768 zcmeIuAr62r3 Date: Tue, 23 Aug 2022 16:24:22 +0200 Subject: [PATCH 88/94] chore: renamed shutdown_drone to drain_drone --- tardis/rest/app/routers/resources.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tardis/rest/app/routers/resources.py b/tardis/rest/app/routers/resources.py index f4e73f4b..6d9c71b4 100644 --- a/tardis/rest/app/routers/resources.py +++ b/tardis/rest/app/routers/resources.py @@ -33,7 +33,7 @@ async def get_resources( @router.patch("/{drone_uuid}/drain", description="Gently shut shown drone") -async def shutdown_drone( +async def drain_drone( drone_uuid: str = Path(..., regex=r"^\S+-[A-Fa-f0-9]{10}$"), sql_registry: SqliteRegistry = Depends(database.get_sql_registry()), _: AuthJWT = Security(security.check_authorization, scopes=[Resources.patch]), From 2d493ea0391c18a0cb30d3560fe8281f985906b8 Mon Sep 17 00:00:00 2001 From: Alexander Haas Date: Tue, 23 Aug 2022 17:00:04 +0200 Subject: [PATCH 89/94] fix: narrowed Exception handling in sql_to_list --- tardis/rest/app/routers/types.py | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/tardis/rest/app/routers/types.py b/tardis/rest/app/routers/types.py index 7cfe93e9..82ebab7d 100644 --- a/tardis/rest/app/routers/types.py +++ b/tardis/rest/app/routers/types.py @@ -13,8 +13,10 @@ def sql_to_list(query_result: List[Dict]) -> List[str]: try: return [list(pair.values())[0] for pair in query_result] - except Exception as e: - raise TardisError("Query result has invalid format") from e + except (AttributeError, IndexError, TypeError) as e: + raise TardisError( + f"Query result has invalid type/format: {type(query_result)}. Must be List[Dict]" + ) from e @router.get("/states", description="Get all available states") From 97320d1731e8d7b4719020abaa4c86eab0b0053a Mon Sep 17 00:00:00 2001 From: Alexander Haas Date: Tue, 23 Aug 2022 17:00:30 +0200 Subject: [PATCH 90/94] tests: asserted that mock_crud was called --- tests/rest_t/routers_t/test_resources.py | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/tests/rest_t/routers_t/test_resources.py b/tests/rest_t/routers_t/test_resources.py index 134fc08f..cf92a5ff 100644 --- a/tests/rest_t/routers_t/test_resources.py +++ b/tests/rest_t/routers_t/test_resources.py @@ -1,3 +1,4 @@ +from unittest.mock import ANY from tests.rest_t.routers_t.base_test_case_routers import TestCaseRouters from tests.utilities.utilities import async_return, run_async @@ -99,7 +100,7 @@ def test_get_resources(self): response = run_async(self.client.get, "/resources/") self.assertEqual(response.status_code, 403) - def test_shutdown_drone(self): + def test_drain_drone(self): self.clear_lru_cache() self.mock_crud.set_state_to_draining.return_value = async_return() @@ -107,6 +108,11 @@ def test_shutdown_drone(self): self.assertEqual(response.status_code, 200) self.assertEqual(response.json(), {"msg": "Drone set to DrainState"}) + # Using ANY because th SQLiteRegistry id changes every time + self.mock_crud.set_state_to_draining.assert_called_once_with( + ANY, "test-0125bc9fd8" + ) + # missing scope self.set_scopes(["resources:get"]) self.login() From ac0ebb9c2a4ed9ff2f2236925f601d1b0d3f9f8c Mon Sep 17 00:00:00 2001 From: Alexander Haas Date: Tue, 23 Aug 2022 17:05:41 +0200 Subject: [PATCH 91/94] flake fix --- tardis/rest/app/routers/types.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tardis/rest/app/routers/types.py b/tardis/rest/app/routers/types.py index 82ebab7d..c5d14d90 100644 --- a/tardis/rest/app/routers/types.py +++ b/tardis/rest/app/routers/types.py @@ -15,7 +15,7 @@ def sql_to_list(query_result: List[Dict]) -> List[str]: return [list(pair.values())[0] for pair in query_result] except (AttributeError, IndexError, TypeError) as e: raise TardisError( - f"Query result has invalid type/format: {type(query_result)}. Must be List[Dict]" + f"Query result has invalid type/format: {type(query_result)}. Must be List[Dict]" # noqa B950 ) from e From 26477d5954bccadeda371ad4567004923154c3a3 Mon Sep 17 00:00:00 2001 From: Alexander Haas Date: Tue, 23 Aug 2022 17:19:21 +0200 Subject: [PATCH 92/94] fix: Removing db files --- drone_registry.db-shm | Bin 32768 -> 0 bytes drone_registry.db-wal | 0 2 files changed, 0 insertions(+), 0 deletions(-) delete mode 100644 drone_registry.db-shm delete mode 100644 drone_registry.db-wal diff --git a/drone_registry.db-shm b/drone_registry.db-shm deleted file mode 100644 index fe9ac2845eca6fe6da8a63cd096d9cf9e24ece10..0000000000000000000000000000000000000000 GIT binary patch literal 0 HcmV?d00001 literal 32768 zcmeIuAr62r3 Date: Tue, 23 Aug 2022 17:29:00 +0200 Subject: [PATCH 93/94] fix: 'fixing' flake error --- tardis/interfaces/borg.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tardis/interfaces/borg.py b/tardis/interfaces/borg.py index 3738e5c2..9b705d70 100644 --- a/tardis/interfaces/borg.py +++ b/tardis/interfaces/borg.py @@ -1,7 +1,7 @@ from abc import ABCMeta -class Borg(metaclass=ABCMeta): +class Borg(metaclass=ABCMeta): # noqa B024 _shared_state = {} # should be overwritten in all classes inheriting the borg def __init__(self): From d28a0c8159af882c5513fc0cd2e167b35091b237 Mon Sep 17 00:00:00 2001 From: Alexander Haas <104835302+haasal@users.noreply.github.com> Date: Wed, 24 Aug 2022 13:43:19 +0200 Subject: [PATCH 94/94] Deleted unused test_authorization function --- tests/rest_t/app_t/test_security.py | 4 ---- 1 file changed, 4 deletions(-) diff --git a/tests/rest_t/app_t/test_security.py b/tests/rest_t/app_t/test_security.py index cda79089..4ffa92e3 100644 --- a/tests/rest_t/app_t/test_security.py +++ b/tests/rest_t/app_t/test_security.py @@ -69,10 +69,6 @@ def test_check_scope_permissions(self): check_scope_permissions(["resources:get"], ["resources:get"]) - def test_check_authorization(self): - # It is very hard mocking the AuthJWT object. However, authorization is tested implicitly as an integration test in the router tests. # noqa B950 - pass - def test_check_authentication(self): self.clear_lru_cache()