From 74032e17f3b4a4e5d0a1cab9becd9338cd601dc0 Mon Sep 17 00:00:00 2001 From: Daniel Hatton Date: Thu, 29 May 2025 10:54:56 +0100 Subject: [PATCH 01/26] Ability to configure different auth routes for instrument server and frontend routers --- src/murfey/server/api/auth.py | 34 +++++++++++++++++++++++- src/murfey/server/api/file_manip.py | 4 +-- src/murfey/server/api/prometheus.py | 4 +-- src/murfey/server/api/session_control.py | 8 +++--- src/murfey/server/api/workflow.py | 10 +++---- src/murfey/util/config.py | 2 ++ 6 files changed, 48 insertions(+), 14 deletions(-) diff --git a/src/murfey/server/api/auth.py b/src/murfey/server/api/auth.py index edcdc6589..951244fab 100644 --- a/src/murfey/server/api/auth.py +++ b/src/murfey/server/api/auth.py @@ -70,6 +70,10 @@ async def __call__(self, request: Request): oauth2_scheme = OAuth2PasswordBearer(tokenUrl="token") else: oauth2_scheme = CookieScheme(cookie_key=security_config.cookie_key) +if security_config.instrument_auth_type == "token": + instrument_oauth2_scheme = OAuth2PasswordBearer(tokenUrl="token") +else: + instrument_oauth2_scheme = lambda *args, **kwargs: None pwd_context = CryptContext(schemes=["bcrypt"], deprecated="auto") instrument_server_tokens: Dict[float, dict] = {} @@ -170,7 +174,35 @@ async def validate_token(token: Annotated[str, Depends(oauth2_scheme)]): ) async with aiohttp.ClientSession(cookies=cookies) as session: async with session.get( - f"{auth_url}{url_path_for('auth.router', 'simple_token_validation')}", + auth_url, + headers=headers, + ) as response: + success = response.status == 200 + validation_outcome = await response.json() + if not (success and validation_outcome.get("valid")): + raise JWTError + except JWTError: + raise HTTPException( + status_code=status.HTTP_401_UNAUTHORIZED, + detail="Could not validate credentials", + headers={"WWW-Authenticate": "Bearer"}, + ) + return None + + +async def validate_instrument_token( + token: Annotated[str, Depends(instrument_oauth2_scheme)] +): + try: + if security_config.instrument_auth_url: + async with aiohttp.ClientSession() as session: + headers = ( + {} + if not security_config.instrument_auth_type + else {"Authorization": f"Bearer {token}"} + ) + async with session.get( + security_config.instrument_auth_url, headers=headers, ) as response: success = response.status == 200 diff --git a/src/murfey/server/api/file_manip.py b/src/murfey/server/api/file_manip.py index 01724c7eb..9986e4562 100644 --- a/src/murfey/server/api/file_manip.py +++ b/src/murfey/server/api/file_manip.py @@ -8,7 +8,7 @@ from sqlmodel import select from werkzeug.utils import secure_filename -from murfey.server.api.auth import MurfeySessionID, validate_token +from murfey.server.api.auth import MurfeySessionID, validate_instrument_token from murfey.server.gain import Camera, prepare_eer_gain, prepare_gain from murfey.server.murfey_db import murfey_db from murfey.util import sanitise, secure_path @@ -20,7 +20,7 @@ router = APIRouter( prefix="/file_manipulation", - dependencies=[Depends(validate_token)], + dependencies=[Depends(validate_instrument_token)], tags=["File Manipulation"], ) diff --git a/src/murfey/server/api/prometheus.py b/src/murfey/server/api/prometheus.py index 5e8d867ea..6457f25be 100644 --- a/src/murfey/server/api/prometheus.py +++ b/src/murfey/server/api/prometheus.py @@ -8,7 +8,7 @@ from sqlmodel import select import murfey.server.prometheus as prom -from murfey.server.api.auth import validate_token +from murfey.server.api.auth import validate_instrument_token from murfey.server.murfey_db import murfey_db from murfey.util import sanitise from murfey.util.db import RsyncInstance @@ -18,7 +18,7 @@ router = APIRouter( prefix="/prometheus", - dependencies=[Depends(validate_token)], + dependencies=[Depends(validate_instrument_token)], tags=["Prometheus"], ) diff --git a/src/murfey/server/api/session_control.py b/src/murfey/server/api/session_control.py index 418226f87..a884ca1df 100644 --- a/src/murfey/server/api/session_control.py +++ b/src/murfey/server/api/session_control.py @@ -12,7 +12,7 @@ import murfey.server.prometheus as prom from murfey.server import _transport_object -from murfey.server.api.auth import MurfeySessionID, validate_token +from murfey.server.api.auth import MurfeySessionID, validate_instrument_token from murfey.server.api.shared import get_foil_hole as _get_foil_hole from murfey.server.api.shared import ( get_foil_holes_from_grid_square as _get_foil_holes_from_grid_square, @@ -60,7 +60,7 @@ router = APIRouter( prefix="/session_control", - dependencies=[Depends(validate_token)], + dependencies=[Depends(validate_instrument_token)], tags=["Session Control: General"], ) @@ -297,7 +297,7 @@ def delete_rsyncer(session_id: int, source: Path, db=murfey_db): spa_router = APIRouter( prefix="/session_control/spa", - dependencies=[Depends(validate_token)], + dependencies=[Depends(validate_instrument_token)], tags=["Session Control: SPA"], ) @@ -355,7 +355,7 @@ def register_foil_hole( correlative_router = APIRouter( prefix="/session_control/correlative", - dependencies=[Depends(validate_token)], + dependencies=[Depends(validate_instrument_token)], tags=["Session Control: Correlative Imaging"], ) diff --git a/src/murfey/server/api/workflow.py b/src/murfey/server/api/workflow.py index df6da6705..5fc33ce23 100644 --- a/src/murfey/server/api/workflow.py +++ b/src/murfey/server/api/workflow.py @@ -26,7 +26,7 @@ import murfey.server.prometheus as prom from murfey.server import _transport_object -from murfey.server.api.auth import MurfeySessionID, validate_token +from murfey.server.api.auth import MurfeySessionID, validate_instrument_token from murfey.server.api.spa import _cryolo_model_path from murfey.server.feedback import ( _murfey_id, @@ -65,7 +65,7 @@ router = APIRouter( prefix="/workflow", - dependencies=[Depends(validate_token)], + dependencies=[Depends(validate_instrument_token)], tags=["Workflows: General"], ) @@ -285,7 +285,7 @@ def register_proc( spa_router = APIRouter( prefix="/workflow/spa", - dependencies=[Depends(validate_token)], + dependencies=[Depends(validate_instrument_token)], tags=["Workflows: SPA"], ) @@ -514,7 +514,7 @@ async def request_spa_preprocessing( tomo_router = APIRouter( prefix="/workflow/tomo", - dependencies=[Depends(validate_token)], + dependencies=[Depends(validate_instrument_token)], tags=["Workflows: CryoET"], ) @@ -912,7 +912,7 @@ def _add_tilt(): correlative_router = APIRouter( prefix="/workflow/correlative", - dependencies=[Depends(validate_token)], + dependencies=[Depends(validate_instrument_token)], tags=["Workflows: Correlative Imaging"], ) diff --git a/src/murfey/util/config.py b/src/murfey/util/config.py index 31bb380fb..4e8e3eca0 100644 --- a/src/murfey/util/config.py +++ b/src/murfey/util/config.py @@ -127,6 +127,8 @@ class Security(BaseModel): auth_key: str = "" auth_type: Literal["password", "cookie"] = "password" auth_url: str = "" + instrument_auth_type: Literal["token", ""] = "token" + instrument_auth_url: str = "" cookie_key: str = "" session_validation: str = "" session_token_timeout: Optional[int] = None From c281170a6d477b4f9848a6d9fcc6931d43f01b8f Mon Sep 17 00:00:00 2001 From: Daniel Hatton Date: Thu, 29 May 2025 16:59:20 +0100 Subject: [PATCH 02/26] Split token auth mechanisms for two different groups of endpoints --- pyproject.toml | 2 - src/murfey/server/api/auth.py | 86 ++++++++++++++++++----------------- 2 files changed, 44 insertions(+), 44 deletions(-) diff --git a/pyproject.toml b/pyproject.toml index 41b1a2143..e33dd2365 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -96,8 +96,6 @@ GitHub = "https://github.com/DiamondLightSource/python-murfey" "murfey.spa_inject" = "murfey.cli.inject_spa_processing:run" "murfey.spa_ispyb_entries" = "murfey.cli.spa_ispyb_messages:run" "murfey.transfer" = "murfey.cli.transfer:run" -[project.entry-points."murfey.auth.token_validation"] -"password" = "murfey.server.api.auth:password_token_validation" [project.entry-points."murfey.config.extraction"] "murfey_machine" = "murfey.util.config:get_extended_machine_config" [project.entry-points."murfey.workflows"] diff --git a/src/murfey/server/api/auth.py b/src/murfey/server/api/auth.py index 951244fab..abb06f180 100644 --- a/src/murfey/server/api/auth.py +++ b/src/murfey/server/api/auth.py @@ -127,10 +127,6 @@ def check_user(username: str) -> bool: return username in [u.username for u in users] -def validate_instrument_server_token(timestamp: float) -> bool: - return timestamp in instrument_server_tokens.keys() - - def validate_instrument_server_session_token(session_id: int, visit: str): with Session(engine) as murfey_db: session_data = murfey_db.exec( @@ -141,46 +137,28 @@ def validate_instrument_server_session_token(session_id: int, visit: str): return visit == session_data[0].visit -def password_token_validation(token: str): - decoded_data = jwt.decode(token, SECRET_KEY, algorithms=[ALGORITHM]) - # first check if the token has expired - if expiry_time := decoded_data.get("expiry_time"): - if expiry_time < time.time(): - raise JWTError - if decoded_data.get("user"): - if not check_user(decoded_data["user"]): - raise JWTError - elif decoded_data.get("session") is not None: - if not validate_instrument_server_session_token( - decoded_data["session"], decoded_data["visit"] - ): - raise JWTError - else: - raise JWTError - - async def validate_token(token: Annotated[str, Depends(oauth2_scheme)]): try: - if auth_url: - headers = ( - {} - if security_config.auth_type == "cookie" - else {"Authorization": f"Bearer {token}"} - ) - cookies = ( - {security_config.cookie_key: token} - if security_config.auth_type == "cookie" - else {} - ) - async with aiohttp.ClientSession(cookies=cookies) as session: - async with session.get( - auth_url, - headers=headers, - ) as response: - success = response.status == 200 - validation_outcome = await response.json() - if not (success and validation_outcome.get("valid")): + try: + if security_config.auth_type == "password": + await validate_password_token(token) + except JWTError: + await validate_instrument_token(token) + decoded_data = jwt.decode(token, SECRET_KEY, algorithms=[ALGORITHM]) + # first check if the token has expired + if expiry_time := decoded_data.get("expiry_time"): + if expiry_time < time.time(): + raise JWTError + if decoded_data.get("user"): + if not check_user(decoded_data["user"]): + raise JWTError + elif decoded_data.get("session") is not None: + if not validate_instrument_server_session_token( + decoded_data["session"], decoded_data["visit"] + ): raise JWTError + else: + raise JWTError except JWTError: raise HTTPException( status_code=status.HTTP_401_UNAUTHORIZED, @@ -190,6 +168,30 @@ async def validate_token(token: Annotated[str, Depends(oauth2_scheme)]): return None +async def validate_password_token(token: Annotated[str, Depends(oauth2_scheme)]): + if auth_url: + headers = ( + {} + if security_config.auth_type == "cookie" + else {"Authorization": f"Bearer {token}"} + ) + cookies = ( + {security_config.cookie_key: token} + if security_config.auth_type == "cookie" + else {} + ) + async with aiohttp.ClientSession(cookies=cookies) as session: + async with session.get( + f"{auth_url}/validate_token", + headers=headers, + ) as response: + success = response.status == 200 + validation_outcome = await response.json() + if not (success and validation_outcome.get("valid")): + raise JWTError + return None + + async def validate_instrument_token( token: Annotated[str, Depends(instrument_oauth2_scheme)] ): @@ -202,7 +204,7 @@ async def validate_instrument_token( else {"Authorization": f"Bearer {token}"} ) async with session.get( - security_config.instrument_auth_url, + f"{security_config.instrument_auth_url}/validate_token", headers=headers, ) as response: success = response.status == 200 From 27a681ac2562ebd7d302cb3644080ebee356831c Mon Sep 17 00:00:00 2001 From: Eu Pin Tien Date: Tue, 3 Jun 2025 11:33:55 +0100 Subject: [PATCH 03/26] Major refactor of 'murfey.server.api.auth', rearranging functions by purpose and splitting the authentication of instrument and frontend tokens into separate functions; created new annotated ints for type hinting in endpoints receiving requests from frontend and instrument server; updates the other server routers to use the newly created annotated ints --- src/murfey/server/api/auth.py | 292 ++++++++++++------ src/murfey/server/api/file_manip.py | 3 +- src/murfey/server/api/instrument.py | 2 +- .../server/api/processing_parameters.py | 3 +- src/murfey/server/api/session_control.py | 3 +- src/murfey/server/api/session_info.py | 3 +- src/murfey/server/api/workflow.py | 3 +- .../workflows/spa/flush_spa_preprocess.py | 2 +- 8 files changed, 206 insertions(+), 105 deletions(-) diff --git a/src/murfey/server/api/auth.py b/src/murfey/server/api/auth.py index abb06f180..ce81c59e5 100644 --- a/src/murfey/server/api/auth.py +++ b/src/murfey/server/api/auth.py @@ -8,7 +8,6 @@ import aiohttp import requests -from backports.entry_points_selectable import entry_points from fastapi import APIRouter, Depends, HTTPException, Request, status from fastapi.security import HTTPBearer, OAuth2PasswordBearer, OAuth2PasswordRequestForm from jose import JWTError, jwt @@ -78,20 +77,6 @@ async def __call__(self, request: Request): instrument_server_tokens: Dict[float, dict] = {} - -""" -HELPER FUNCTIONS -""" - - -def verify_password(plain_password: str, hashed_password: str) -> bool: - return pwd_context.verify(plain_password, hashed_password) - - -def hash_password(password: str) -> str: - return pwd_context.hash(password) - - # Set up database engine try: _url = url(security_config) @@ -100,22 +85,19 @@ def hash_password(password: str) -> str: engine = None -def validate_user(username: str, password: str) -> bool: - try: - with Session(engine) as murfey_db: - user = murfey_db.exec(select(User).where(User.username == username)).one() - except Exception: - return False - return verify_password(password, user.hashed_password) +def hash_password(password: str) -> str: + return pwd_context.hash(password) -def validate_visit(visit_name: str, token: str) -> bool: - if validators := entry_points().select( - group="murfey.auth.session_validation", - name=security_config.auth_type, - ): - return validators[0].load()(visit_name, token) - return True +""" +======================================================================================= +TOKEN VALIDATION FUNCTIONS +======================================================================================= + +Functions and helpers used to validate incoming requests from both the client and +the frontend. 'validate_token()' and 'validate_instrument_token()' are imported +int the other FastAPI modules and attached as dependencies to the routers. +""" def check_user(username: str) -> bool: @@ -127,75 +109,75 @@ def check_user(username: str) -> bool: return username in [u.username for u in users] -def validate_instrument_server_session_token(session_id: int, visit: str): - with Session(engine) as murfey_db: - session_data = murfey_db.exec( - select(MurfeySession).where(MurfeySession.id == session_id) - ).all() - if len(session_data) != 1: - return False - return visit == session_data[0].visit - - async def validate_token(token: Annotated[str, Depends(oauth2_scheme)]): + """ + Used by the backend routers to validate requests coming in from frontend. + """ try: - try: - if security_config.auth_type == "password": - await validate_password_token(token) - except JWTError: - await validate_instrument_token(token) - decoded_data = jwt.decode(token, SECRET_KEY, algorithms=[ALGORITHM]) - # first check if the token has expired - if expiry_time := decoded_data.get("expiry_time"): - if expiry_time < time.time(): - raise JWTError - if decoded_data.get("user"): - if not check_user(decoded_data["user"]): - raise JWTError - elif decoded_data.get("session") is not None: - if not validate_instrument_server_session_token( - decoded_data["session"], decoded_data["visit"] - ): + # Validate using auth URL if provided; will error if invalid + if auth_url: + headers = ( + {} + if security_config.auth_type == "cookie" + else {"Authorization": f"Bearer {token}"} + ) + cookies = ( + {security_config.cookie_key: token} + if security_config.auth_type == "cookie" + else {} + ) + async with aiohttp.ClientSession(cookies=cookies) as session: + async with session.get( + f"{auth_url}/validate_token", + headers=headers, + ) as response: + success = response.status == 200 + validation_outcome = await response.json() + if not (success and validation_outcome.get("valid")): raise JWTError + # Auth URL MUST be provided if authenticating using cookies else: - raise JWTError + if security_config.auth_type == "cookie": + raise JWTError + + # Validate using password + if security_config.auth_type == "password": + decoded_data = jwt.decode(token, SECRET_KEY, algorithms=[ALGORITHM]) + # Frontend validation + if decoded_data.get("user"): + if not check_user(decoded_data["user"]): + raise JWTError + except JWTError: raise HTTPException( status_code=status.HTTP_401_UNAUTHORIZED, - detail="Could not validate credentials", + detail="Could not validate credentials from frontend", headers={"WWW-Authenticate": "Bearer"}, ) return None -async def validate_password_token(token: Annotated[str, Depends(oauth2_scheme)]): - if auth_url: - headers = ( - {} - if security_config.auth_type == "cookie" - else {"Authorization": f"Bearer {token}"} - ) - cookies = ( - {security_config.cookie_key: token} - if security_config.auth_type == "cookie" - else {} - ) - async with aiohttp.ClientSession(cookies=cookies) as session: - async with session.get( - f"{auth_url}/validate_token", - headers=headers, - ) as response: - success = response.status == 200 - validation_outcome = await response.json() - if not (success and validation_outcome.get("valid")): - raise JWTError - return None +def validate_session_against_visit(session_id: int, visit: str): + """ + Checks that the session ID is associated with the claimed visit. + """ + with Session(engine) as murfey_db: + session_data = murfey_db.exec( + select(MurfeySession).where(MurfeySession.id == session_id) + ).all() + if len(session_data) != 1: + return False + return visit == session_data[0].visit async def validate_instrument_token( token: Annotated[str, Depends(instrument_oauth2_scheme)] ): + """ + Used by the backend routers to check the incoming instrument server token. + """ try: + # Validate using auth URL if provided if security_config.instrument_auth_url: async with aiohttp.ClientSession() as session: headers = ( @@ -212,33 +194,105 @@ async def validate_instrument_token( if not (success and validation_outcome.get("valid")): raise JWTError else: - if validators := entry_points().select( - group="murfey.auth.token_validation", - name=security_config.auth_type, - ): - validators[0].load()(token) + # First, check if the token has expired + decoded_data = jwt.decode(token, SECRET_KEY, algorithms=[ALGORITHM]) + if expiry_time := decoded_data.get("expiry_time"): + if expiry_time < time.time(): + raise JWTError + elif decoded_data.get("session") is not None: + # Check that the decoded session corresponds to the visit + if not validate_session_against_visit( + decoded_data["session"], decoded_data["visit"] + ): + raise JWTError else: raise JWTError except JWTError: raise HTTPException( status_code=status.HTTP_401_UNAUTHORIZED, - detail="Could not validate credentials", + detail="Could not validate credentials from instrument", headers={"WWW-Authenticate": "Bearer"}, ) return None +""" +======================================================================================= +VALIDATING SESSION IDS +======================================================================================= + +Annotated ints are defined here that trigger validation of the session IDs in incoming +requests, verifying that the session is allowed to access the particular visit. + +The 'MurfeySessionID...' types are imported and used in the type hints of the endpoint +functions in the other FastAPI routers, depending on whether requests from the frontend +or the instrument are expected. +""" + + +async def validate_visit(visit_name: str, token: str, instrument_access: bool) -> bool: + """ + Validates the incoming token depending on whether it's from the instrument or the frontend + """ + + # If this token is from the instrument server, validate this way + if instrument_access: + if security_config.instrument_auth_url: + async with aiohttp.ClientSession() as session: + headers = ( + {} + if not security_config.instrument_auth_type + else {"Authorization": f"Bearer {token}"} + ) + async with session.get( + f"{security_config.instrument_auth_url}/validate_visit_access/{visit_name}", + headers=headers, + ) as response: + success = response.status == 200 + validation_outcome = await response.json() + if not (success and validation_outcome.get("valid")): + return False + # Otherwise, use this validation method + else: + if security_config.auth_url: + headers = ( + {} + if security_config.auth_type == "cookie" + else {"Authorization": f"Bearer {token}"} + ) + cookies = ( + {security_config.cookie_key: token} + if security_config.auth_type == "cookie" + else {} + ) + async with aiohttp.ClientSession(cookies=cookies) as session: + async with session.get( + f"{auth_url}/validate_visit_access/{visit_name}", + headers=headers, + ) as response: + success = response.status == 200 + validation_outcome = await response.json() + if not (success and validation_outcome.get("valid")): + return False + return True + + async def validate_session_access( - session_id: int, token: Annotated[str, Depends(oauth2_scheme)] + session_id: int, + token: Annotated[str, Depends(oauth2_scheme)], + instrument_access: bool, ) -> int: - await validate_token(token) + """ + Validates whether the request is authorised to access information about this session + """ with Session(engine) as murfey_db: visit_name = ( murfey_db.exec(select(MurfeySession).where(MurfeySession.id == session_id)) .one() .visit ) - if not validate_visit(visit_name, token): + validated = await validate_visit(visit_name, token, instrument_access) + if not validated: raise HTTPException( status_code=status.HTTP_401_UNAUTHORIZED, detail="You do not have access to this visit", @@ -247,9 +301,53 @@ async def validate_session_access( return session_id -class Token(BaseModel): - access_token: str - token_type: str +def validate_session_access_wrapper(instrument_access: bool): + """ + Factory that returns an async wrapper arond 'validate_session_access' with the + 'instrument_access' field preconfigured. + + This is used to generate FastAPI-compatible dependencies for validating session + access, based on the context of the request. + + Unlike 'functools.partial', this approach preserves introspection compatibility + required by FastAPI for dependency resolution and OpenAPI generation. + """ + + async def _validate(session_id: int, token: Annotated[str, Depends(oauth2_scheme)]): + return await validate_session_access( + session_id, token, instrument_access=instrument_access + ) + + return _validate + + +# Set validation conditions for the session ID based on where the request is from +MurfeySessionIDFrontend = Annotated[ + int, Depends(validate_session_access_wrapper(instrument_access=False)) +] +MurfeySessionIDInstrument = Annotated[ + int, Depends(validate_session_access_wrapper(instrument_access=True)) +] + + +""" +======================================================================================= +API ENDPOINTS AND HELPER FUNCTIONS/CLASSES +======================================================================================= +""" + + +def verify_password(plain_password: str, hashed_password: str) -> bool: + return pwd_context.verify(plain_password, hashed_password) + + +def validate_user(username: str, password: str) -> bool: + try: + with Session(engine) as murfey_db: + user = murfey_db.exec(select(User).where(User.username == username)).one() + except Exception: + return False + return verify_password(password, user.hashed_password) def create_access_token(data: dict, token: str = "") -> str: @@ -274,11 +372,9 @@ def create_access_token(data: dict, token: str = "") -> str: return encoded_jwt -MurfeySessionID = Annotated[int, Depends(validate_session_access)] - -""" -API ENDPOINTS -""" +class Token(BaseModel): + access_token: str + token_type: str @router.post("/token") @@ -313,7 +409,7 @@ async def generate_token( @router.get("/sessions/{session_id}/token") -async def mint_session_token(session_id: MurfeySessionID, db=murfey_db): +async def mint_session_token(session_id: MurfeySessionIDFrontend, db=murfey_db): visit = ( db.exec(select(MurfeySession).where(MurfeySession.id == session_id)).one().visit ) diff --git a/src/murfey/server/api/file_manip.py b/src/murfey/server/api/file_manip.py index 9986e4562..8b4a09e18 100644 --- a/src/murfey/server/api/file_manip.py +++ b/src/murfey/server/api/file_manip.py @@ -8,7 +8,8 @@ from sqlmodel import select from werkzeug.utils import secure_filename -from murfey.server.api.auth import MurfeySessionID, validate_instrument_token +from murfey.server.api.auth import MurfeySessionIDInstrument as MurfeySessionID +from murfey.server.api.auth import validate_instrument_token from murfey.server.gain import Camera, prepare_eer_gain, prepare_gain from murfey.server.murfey_db import murfey_db from murfey.util import sanitise, secure_path diff --git a/src/murfey/server/api/instrument.py b/src/murfey/server/api/instrument.py index 59665399a..34e238208 100644 --- a/src/murfey/server/api/instrument.py +++ b/src/murfey/server/api/instrument.py @@ -12,8 +12,8 @@ from sqlmodel import select from werkzeug.utils import secure_filename +from murfey.server.api.auth import MurfeySessionIDFrontend as MurfeySessionID from murfey.server.api.auth import ( - MurfeySessionID, create_access_token, instrument_server_tokens, oauth2_scheme, diff --git a/src/murfey/server/api/processing_parameters.py b/src/murfey/server/api/processing_parameters.py index 11b9f57de..80061a234 100644 --- a/src/murfey/server/api/processing_parameters.py +++ b/src/murfey/server/api/processing_parameters.py @@ -6,7 +6,8 @@ from pydantic import BaseModel from sqlmodel import Session, select -from murfey.server.api.auth import MurfeySessionID, validate_token +from murfey.server.api.auth import MurfeySessionIDFrontend as MurfeySessionID +from murfey.server.api.auth import validate_token from murfey.server.murfey_db import murfey_db from murfey.util.db import SessionProcessingParameters diff --git a/src/murfey/server/api/session_control.py b/src/murfey/server/api/session_control.py index a884ca1df..e218f93c4 100644 --- a/src/murfey/server/api/session_control.py +++ b/src/murfey/server/api/session_control.py @@ -12,7 +12,8 @@ import murfey.server.prometheus as prom from murfey.server import _transport_object -from murfey.server.api.auth import MurfeySessionID, validate_instrument_token +from murfey.server.api.auth import MurfeySessionIDInstrument as MurfeySessionID +from murfey.server.api.auth import validate_instrument_token from murfey.server.api.shared import get_foil_hole as _get_foil_hole from murfey.server.api.shared import ( get_foil_holes_from_grid_square as _get_foil_holes_from_grid_square, diff --git a/src/murfey/server/api/session_info.py b/src/murfey/server/api/session_info.py index 50e38ba2b..2db5a3a79 100644 --- a/src/murfey/server/api/session_info.py +++ b/src/murfey/server/api/session_info.py @@ -14,7 +14,8 @@ import murfey.server.api.websocket as ws from murfey.server import _transport_object from murfey.server.api import templates -from murfey.server.api.auth import MurfeySessionID, validate_token +from murfey.server.api.auth import MurfeySessionIDFrontend as MurfeySessionID +from murfey.server.api.auth import validate_token from murfey.server.api.shared import get_foil_hole as _get_foil_hole from murfey.server.api.shared import ( get_foil_holes_from_grid_square as _get_foil_holes_from_grid_square, diff --git a/src/murfey/server/api/workflow.py b/src/murfey/server/api/workflow.py index 4a965a7f8..06432d96b 100644 --- a/src/murfey/server/api/workflow.py +++ b/src/murfey/server/api/workflow.py @@ -26,7 +26,8 @@ import murfey.server.prometheus as prom from murfey.server import _transport_object -from murfey.server.api.auth import MurfeySessionID, validate_instrument_token +from murfey.server.api.auth import MurfeySessionIDInstrument as MurfeySessionID +from murfey.server.api.auth import validate_instrument_token from murfey.server.feedback import ( _murfey_id, check_tilt_series_mc, diff --git a/src/murfey/workflows/spa/flush_spa_preprocess.py b/src/murfey/workflows/spa/flush_spa_preprocess.py index bc226e537..b225dfb69 100644 --- a/src/murfey/workflows/spa/flush_spa_preprocess.py +++ b/src/murfey/workflows/spa/flush_spa_preprocess.py @@ -7,7 +7,7 @@ from sqlmodel import Session, select from murfey.server import _transport_object -from murfey.server.api.auth import MurfeySessionID +from murfey.server.api.auth import MurfeySessionIDInstrument as MurfeySessionID from murfey.server.feedback import _murfey_id from murfey.util import sanitise, secure_path from murfey.util.config import get_machine_config, get_microscope From d5efb6f272909895b65c82ba75fdac8ab8419c8c Mon Sep 17 00:00:00 2001 From: Eu Pin Tien Date: Tue, 3 Jun 2025 11:34:33 +0100 Subject: [PATCH 04/26] Added documentation to instrument server-side token validation function --- src/murfey/instrument_server/api.py | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/src/murfey/instrument_server/api.py b/src/murfey/instrument_server/api.py index 93f49099e..95ff5f506 100644 --- a/src/murfey/instrument_server/api.py +++ b/src/murfey/instrument_server/api.py @@ -51,6 +51,9 @@ def validate_session_token( session_id: int, token: Annotated[str, Depends(oauth2_scheme)] ): + """ + Validates the token received from the backend server + """ try: decoded_data = jwt.decode( token, @@ -62,7 +65,7 @@ def validate_session_token( except JWTError: raise HTTPException( status_code=status.HTTP_401_UNAUTHORIZED, - detail="Could not validate credentials", + detail="Could not validate credentials from backend", headers={"WWW-Authenticate": "Bearer"}, ) return session_id From 02e8c194f69bcbfe282f1e27f0d40131bf099025 Mon Sep 17 00:00:00 2001 From: Eu Pin Tien Date: Tue, 3 Jun 2025 12:10:45 +0100 Subject: [PATCH 05/26] Refactored 'murfey.server.api.file_manip' into 'file_io_instrument', 'file_io_frontend', and 'file_io_shared' due to frontened and instrument accessing the same functions; moved 'process_gain()' into 'file_io_shared' so that it can be called from both with proper validation; updated route manifest and client-side URL lookups to reflect this --- src/murfey/client/contexts/spa.py | 2 +- src/murfey/client/contexts/tomo.py | 2 +- src/murfey/client/multigrid_control.py | 4 +- src/murfey/client/tui/app.py | 4 +- src/murfey/client/tui/screens.py | 4 +- src/murfey/server/api/file_io_frontend.py | 26 +++++ .../{file_manip.py => file_io_instrument.py} | 87 ++--------------- src/murfey/server/api/file_io_shared.py | 94 +++++++++++++++++++ src/murfey/server/main.py | 6 +- src/murfey/util/route_manifest.yaml | 16 +++- 10 files changed, 151 insertions(+), 94 deletions(-) create mode 100644 src/murfey/server/api/file_io_frontend.py rename src/murfey/server/api/{file_manip.py => file_io_instrument.py} (69%) create mode 100644 src/murfey/server/api/file_io_shared.py diff --git a/src/murfey/client/contexts/spa.py b/src/murfey/client/contexts/spa.py index 89017f60a..02e335384 100644 --- a/src/murfey/client/contexts/spa.py +++ b/src/murfey/client/contexts/spa.py @@ -581,7 +581,7 @@ def post_transfer( eer_fractionation_file = None if file_transferred_to.suffix == ".eer": response = capture_post( - f"{str(environment.url.geturl())}{url_path_for('file_manip.router', 'write_eer_fractionation_file', visit_name=environment.visit, session_id=environment.murfey_session)}", + f"{str(environment.url.geturl())}{url_path_for('file_io_instrument.router', 'write_eer_fractionation_file', visit_name=environment.visit, session_id=environment.murfey_session)}", json={ "eer_path": str(file_transferred_to), "fractionation": environment.data_collection_parameters[ diff --git a/src/murfey/client/contexts/tomo.py b/src/murfey/client/contexts/tomo.py index ab502febf..9562041f7 100644 --- a/src/murfey/client/contexts/tomo.py +++ b/src/murfey/client/contexts/tomo.py @@ -317,7 +317,7 @@ def _add_tilt( eer_fractionation_file = None if environment.data_collection_parameters.get("num_eer_frames"): response = requests.post( - f"{str(environment.url.geturl())}{url_path_for('file_manip.router', 'write_eer_fractionation_file', visit_name=environment.visit, session_id=environment.murfey_session)}", + f"{str(environment.url.geturl())}{url_path_for('file_io_instrument.router', 'write_eer_fractionation_file', visit_name=environment.visit, session_id=environment.murfey_session)}", json={ "num_frames": environment.data_collection_parameters[ "num_eer_frames" diff --git a/src/murfey/client/multigrid_control.py b/src/murfey/client/multigrid_control.py index 03ab2cfaa..d32aa8747 100644 --- a/src/murfey/client/multigrid_control.py +++ b/src/murfey/client/multigrid_control.py @@ -251,7 +251,7 @@ def _start_rsyncer( log.info(f"starting rsyncer: {source}") if transfer: # Always make sure the destination directory exists - make_directory_url = f"{self.murfey_url}{url_path_for('file_manip.router', 'make_rsyncer_destination', session_id=self.session_id)}" + make_directory_url = f"{self.murfey_url}{url_path_for('file_io_instrument.router', 'make_rsyncer_destination', session_id=self.session_id)}" capture_post(make_directory_url, json={"destination": destination}) if self._environment: self._environment.default_destinations[source] = destination @@ -437,7 +437,7 @@ def _start_dc(self, json, from_form: bool = False): log.info("Registering tomography processing parameters") if self._environment.data_collection_parameters.get("num_eer_frames"): eer_response = requests.post( - f"{str(self._environment.url.geturl())}{url_path_for('file_manip.router', 'write_eer_fractionation_file', visit_name=self._environment.visit, session_id=self._environment.murfey_session)}", + f"{str(self._environment.url.geturl())}{url_path_for('file_io_instrument.router', 'write_eer_fractionation_file', visit_name=self._environment.visit, session_id=self._environment.murfey_session)}", json={ "num_frames": self._environment.data_collection_parameters[ "num_eer_frames" diff --git a/src/murfey/client/tui/app.py b/src/murfey/client/tui/app.py index ff6542de4..b6d73ab5b 100644 --- a/src/murfey/client/tui/app.py +++ b/src/murfey/client/tui/app.py @@ -209,7 +209,7 @@ def _start_rsyncer( log.info(f"starting rsyncer: {source}") if transfer: # Always make sure the destination directory exists - make_directory_url = f"{str(self._url.geturl())}{url_path_for('file_manip.router', 'make_rsyncer_destination', session_id=self._environment.murfey_session)}" + make_directory_url = f"{str(self._url.geturl())}{url_path_for('file_io_instrument.router', 'make_rsyncer_destination', session_id=self._environment.murfey_session)}" capture_post(make_directory_url, json={"destination": destination}) if self._environment: self._environment.default_destinations[source] = destination @@ -488,7 +488,7 @@ def _start_dc(self, json, from_form: bool = False): log.info("Registering tomography processing parameters") if self.app._environment.data_collection_parameters.get("num_eer_frames"): eer_response = requests.post( - f"{str(self.app._environment.url.geturl())}{url_path_for('file_manip.router', 'write_eer_fractionation_file', visit_name=self.app._environment.visit, session_id=self.app._environment.murfey_session)}", + f"{str(self.app._environment.url.geturl())}{url_path_for('file_io_instrument.router', 'write_eer_fractionation_file', visit_name=self.app._environment.visit, session_id=self.app._environment.murfey_session)}", json={ "num_frames": self.app._environment.data_collection_parameters[ "num_eer_frames" diff --git a/src/murfey/client/tui/screens.py b/src/murfey/client/tui/screens.py index 04b52c5be..3ff79d5ae 100644 --- a/src/murfey/client/tui/screens.py +++ b/src/murfey/client/tui/screens.py @@ -110,7 +110,7 @@ def determine_default_destination( _default = environment.destination_registry[source_name] else: suggested_path_response = capture_post( - url=f"{str(environment.url.geturl())}{url_path_for('file_manip.router', 'suggest_path', visit_name=visit, session_id=environment.murfey_session)}", + url=f"{str(environment.url.geturl())}{url_path_for('file_io_instrument.router', 'suggest_path', visit_name=visit, session_id=environment.murfey_session)}", json={ "base_path": f"{destination}/{visit}/{mid_path.parent if include_mid_path else ''}/raw", "touch": touch, @@ -906,7 +906,7 @@ def on_button_pressed(self, event): f"Gain reference file {posix_path(self._dir_tree._gain_reference)!r} was not successfully transferred to {visit_path}/processing" ) process_gain_response = requests.post( - url=f"{str(self.app._environment.url.geturl())}{url_path_for('file_manip.router', 'process_gain', session_id=self.app._environment.murfey_session)}", + url=f"{str(self.app._environment.url.geturl())}{url_path_for('file_io_instrument.router', 'process_gain', session_id=self.app._environment.murfey_session)}", json={ "gain_ref": str(self._dir_tree._gain_reference), "eer": bool( diff --git a/src/murfey/server/api/file_io_frontend.py b/src/murfey/server/api/file_io_frontend.py new file mode 100644 index 000000000..531fa81c8 --- /dev/null +++ b/src/murfey/server/api/file_io_frontend.py @@ -0,0 +1,26 @@ +from logging import getLogger + +from fastapi import APIRouter, Depends + +from murfey.server.api.auth import MurfeySessionIDFrontend as MurfeySessionID +from murfey.server.api.auth import validate_token +from murfey.server.api.file_io_shared import GainReference +from murfey.server.api.file_io_shared import process_gain as _process_gain +from murfey.server.murfey_db import murfey_db + +logger = getLogger("murfey.server.api.file_io_frontend") + + +router = APIRouter( + prefix="/file_io/frontend", + dependencies=[Depends(validate_token)], + tags=["File I/O: Frontend"], +) + + +@router.post("/sessions/{session_id}/process_gain") +async def process_gain( + session_id: MurfeySessionID, gain_reference_params: GainReference, db=murfey_db +): + result = await _process_gain(session_id, gain_reference_params, db) + return result diff --git a/src/murfey/server/api/file_manip.py b/src/murfey/server/api/file_io_instrument.py similarity index 69% rename from src/murfey/server/api/file_manip.py rename to src/murfey/server/api/file_io_instrument.py index 8b4a09e18..ed42ab5ec 100644 --- a/src/murfey/server/api/file_manip.py +++ b/src/murfey/server/api/file_io_instrument.py @@ -10,19 +10,21 @@ from murfey.server.api.auth import MurfeySessionIDInstrument as MurfeySessionID from murfey.server.api.auth import validate_instrument_token -from murfey.server.gain import Camera, prepare_eer_gain, prepare_gain +from murfey.server.api.file_io_shared import GainReference +from murfey.server.api.file_io_shared import process_gain as _process_gain from murfey.server.murfey_db import murfey_db from murfey.util import sanitise, secure_path from murfey.util.config import get_machine_config from murfey.util.db import Session, SessionProcessingParameters from murfey.util.eer import num_frames -logger = getLogger("murfey.server.api.file_manip") +logger = getLogger("murfey.server.api.file_io_instrument") + router = APIRouter( - prefix="/file_manipulation", + prefix="/file_io/instrument", dependencies=[Depends(validate_instrument_token)], - tags=["File Manipulation"], + tags=["File I/O: Instrument"], ) @@ -107,85 +109,12 @@ def make_rsyncer_destination(session_id: int, destination: Dest, db=murfey_db): return destination -class GainReference(BaseModel): - gain_ref: Path - rescale: bool = True - eer: bool = False - tag: str = "" - - @router.post("/sessions/{session_id}/process_gain") async def process_gain( session_id: MurfeySessionID, gain_reference_params: GainReference, db=murfey_db ): - murfey_session = db.exec(select(Session).where(Session.id == session_id)).one() - visit_name = murfey_session.visit - instrument_name = murfey_session.instrument_name - machine_config = get_machine_config(instrument_name=instrument_name)[ - instrument_name - ] - camera = getattr(Camera, machine_config.camera) - if gain_reference_params.eer: - executables = machine_config.external_executables_eer - else: - executables = machine_config.external_executables - env = machine_config.external_environment - safe_path_name = secure_filename(gain_reference_params.gain_ref.name) - filepath = ( - Path(machine_config.rsync_basepath) - / str(datetime.now().year) - / secure_filename(visit_name) - / machine_config.gain_directory_name - ) - - # Check under previous year if the folder doesn't exist - if not filepath.exists(): - filepath_prev = filepath - filepath = ( - Path(machine_config.rsync_basepath) - / str(datetime.now().year - 1) - / secure_filename(visit_name) - / machine_config.gain_directory_name - ) - # If it's not in the previous year, it's a genuine error - if not filepath.exists(): - log_message = ( - "Unable to find gain reference directory under " - f"{str(filepath_prev)!r} or {str(filepath)}" - ) - logger.error(log_message) - raise FileNotFoundError(log_message) - - if gain_reference_params.eer: - new_gain_ref, new_gain_ref_superres = await prepare_eer_gain( - filepath / safe_path_name, - executables, - env, - tag=gain_reference_params.tag, - ) - else: - new_gain_ref, new_gain_ref_superres = await prepare_gain( - camera, - filepath / safe_path_name, - executables, - env, - rescale=gain_reference_params.rescale, - tag=gain_reference_params.tag, - ) - if new_gain_ref and new_gain_ref_superres: - return { - "gain_ref": new_gain_ref.relative_to(Path(machine_config.rsync_basepath)), - "gain_ref_superres": new_gain_ref_superres.relative_to( - Path(machine_config.rsync_basepath) - ), - } - elif new_gain_ref: - return { - "gain_ref": new_gain_ref.relative_to(Path(machine_config.rsync_basepath)), - "gain_ref_superres": None, - } - else: - return {"gain_ref": str(filepath / safe_path_name), "gain_ref_superres": None} + result = await _process_gain(session_id, gain_reference_params, db) + return result class FractionationParameters(BaseModel): diff --git a/src/murfey/server/api/file_io_shared.py b/src/murfey/server/api/file_io_shared.py new file mode 100644 index 000000000..310c8e4b9 --- /dev/null +++ b/src/murfey/server/api/file_io_shared.py @@ -0,0 +1,94 @@ +from datetime import datetime +from logging import getLogger +from pathlib import Path + +from pydantic import BaseModel +from sqlmodel import select +from werkzeug.utils import secure_filename + +from murfey.server.gain import Camera, prepare_eer_gain, prepare_gain +from murfey.server.murfey_db import murfey_db +from murfey.util.config import get_machine_config +from murfey.util.db import Session + +logger = getLogger("murfey.server.api.file_io_shared") + + +class GainReference(BaseModel): + gain_ref: Path + rescale: bool = True + eer: bool = False + tag: str = "" + + +async def process_gain( + session_id: int, gain_reference_params: GainReference, db=murfey_db +): + murfey_session = db.exec(select(Session).where(Session.id == session_id)).one() + visit_name = murfey_session.visit + instrument_name = murfey_session.instrument_name + machine_config = get_machine_config(instrument_name=instrument_name)[ + instrument_name + ] + camera = getattr(Camera, machine_config.camera) + if gain_reference_params.eer: + executables = machine_config.external_executables_eer + else: + executables = machine_config.external_executables + env = machine_config.external_environment + safe_path_name = secure_filename(gain_reference_params.gain_ref.name) + filepath = ( + Path(machine_config.rsync_basepath) + / str(datetime.now().year) + / secure_filename(visit_name) + / machine_config.gain_directory_name + ) + + # Check under previous year if the folder doesn't exist + if not filepath.exists(): + filepath_prev = filepath + filepath = ( + Path(machine_config.rsync_basepath) + / str(datetime.now().year - 1) + / secure_filename(visit_name) + / machine_config.gain_directory_name + ) + # If it's not in the previous year, it's a genuine error + if not filepath.exists(): + log_message = ( + "Unable to find gain reference directory under " + f"{str(filepath_prev)!r} or {str(filepath)}" + ) + logger.error(log_message) + raise FileNotFoundError(log_message) + + if gain_reference_params.eer: + new_gain_ref, new_gain_ref_superres = await prepare_eer_gain( + filepath / safe_path_name, + executables, + env, + tag=gain_reference_params.tag, + ) + else: + new_gain_ref, new_gain_ref_superres = await prepare_gain( + camera, + filepath / safe_path_name, + executables, + env, + rescale=gain_reference_params.rescale, + tag=gain_reference_params.tag, + ) + if new_gain_ref and new_gain_ref_superres: + return { + "gain_ref": new_gain_ref.relative_to(Path(machine_config.rsync_basepath)), + "gain_ref_superres": new_gain_ref_superres.relative_to( + Path(machine_config.rsync_basepath) + ), + } + elif new_gain_ref: + return { + "gain_ref": new_gain_ref.relative_to(Path(machine_config.rsync_basepath)), + "gain_ref_superres": None, + } + else: + return {"gain_ref": str(filepath / safe_path_name), "gain_ref_superres": None} diff --git a/src/murfey/server/main.py b/src/murfey/server/main.py index fb4b595ea..d588476af 100644 --- a/src/murfey/server/main.py +++ b/src/murfey/server/main.py @@ -14,7 +14,8 @@ import murfey.server.api.bootstrap import murfey.server.api.clem import murfey.server.api.display -import murfey.server.api.file_manip +import murfey.server.api.file_io_frontend +import murfey.server.api.file_io_instrument import murfey.server.api.hub import murfey.server.api.instrument import murfey.server.api.mag_table @@ -71,7 +72,8 @@ class Settings(BaseSettings): app.include_router(murfey.server.api.display.router) app.include_router(murfey.server.api.processing_parameters.router) -app.include_router(murfey.server.api.file_manip.router) +app.include_router(murfey.server.api.file_io_frontend.router) +app.include_router(murfey.server.api.file_io_instrument.router) app.include_router(murfey.server.api.instrument.router) diff --git a/src/murfey/util/route_manifest.yaml b/src/murfey/util/route_manifest.yaml index ae1b3dadc..52e55a240 100644 --- a/src/murfey/util/route_manifest.yaml +++ b/src/murfey/util/route_manifest.yaml @@ -410,8 +410,14 @@ murfey.server.api.display.router: type: int methods: - GET -murfey.server.api.file_manip.router: - - path: /file_manipulation/visits/{visit_name}/{session_id}/suggested_path +murfey.server.api.file_io_frontend.router: + - path: /file_io/frontend/sessions/{session_id}/process_gain + function: process_gain + path_params: [] + methods: + - POST +murfey.server.api.file_io_instrument.router: + - path: /file_io/instrument/visits/{visit_name}/{session_id}/suggested_path function: suggest_path path_params: - name: visit_name @@ -420,19 +426,19 @@ murfey.server.api.file_manip.router: type: int methods: - POST - - path: /file_manipulation/sessions/{session_id}/make_rsyncer_destination + - path: /file_io/instrument/sessions/{session_id}/make_rsyncer_destination function: make_rsyncer_destination path_params: - name: session_id type: int methods: - POST - - path: /file_manipulation/sessions/{session_id}/process_gain + - path: /file_io/instrument/sessions/{session_id}/process_gain function: process_gain path_params: [] methods: - POST - - path: /file_manipulation/visits/{visit_name}/{session_id}/eer_fractionation_file + - path: /file_io/instrument/visits/{visit_name}/{session_id}/eer_fractionation_file function: write_eer_fractionation_file path_params: - name: visit_name From f7b400655fde22f436e4ce5fa9427bb0b0d02224 Mon Sep 17 00:00:00 2001 From: Eu Pin Tien Date: Tue, 3 Jun 2025 18:41:43 +0100 Subject: [PATCH 06/26] Added prefixes to the 'auth' and 'clem' routers --- src/murfey/server/api/auth.py | 5 ++++- src/murfey/server/api/clem.py | 5 ++++- 2 files changed, 8 insertions(+), 2 deletions(-) diff --git a/src/murfey/server/api/auth.py b/src/murfey/server/api/auth.py index ce81c59e5..f2c430988 100644 --- a/src/murfey/server/api/auth.py +++ b/src/murfey/server/api/auth.py @@ -25,7 +25,10 @@ logger = getLogger("murfey.server.api.auth") # Set up router -router = APIRouter(tags=["Authentication"]) +router = APIRouter( + prefix="/auth", + tags=["Authentication"], +) class CookieScheme(HTTPBearer): diff --git a/src/murfey/server/api/clem.py b/src/murfey/server/api/clem.py index 1bc043fb8..a13b547b7 100644 --- a/src/murfey/server/api/clem.py +++ b/src/murfey/server/api/clem.py @@ -31,7 +31,10 @@ logger = getLogger("murfey.server.api.clem") # Create APIRouter class object -router = APIRouter(tags=["Workflows: CLEM"]) +router = APIRouter( + prefix="/workflow/clem", + tags=["Workflows: CLEM"], +) # Valid file types valid_file_types = ( From 2a8e75e674e22cbb128c37390a5ba47abb270d58 Mon Sep 17 00:00:00 2001 From: Eu Pin Tien Date: Wed, 4 Jun 2025 09:29:51 +0100 Subject: [PATCH 07/26] Updated route manifest with new URL paths --- src/murfey/util/route_manifest.yaml | 22 +++++++++++----------- 1 file changed, 11 insertions(+), 11 deletions(-) diff --git a/src/murfey/util/route_manifest.yaml b/src/murfey/util/route_manifest.yaml index 52e55a240..52e1df6b5 100644 --- a/src/murfey/util/route_manifest.yaml +++ b/src/murfey/util/route_manifest.yaml @@ -105,17 +105,17 @@ murfey.instrument_server.api.router: methods: - POST murfey.server.api.auth.router: - - path: /token + - path: /auth/token function: generate_token path_params: [] methods: - POST - - path: /sessions/{session_id}/token + - path: /auth/sessions/{session_id}/token function: mint_session_token path_params: [] methods: - GET - - path: /validate_token + - path: /auth/validate_token function: simple_token_validation path_params: [] methods: @@ -315,56 +315,56 @@ murfey.server.api.bootstrap.version: methods: - GET murfey.server.api.clem.router: - - path: /sessions/{session_id}/clem/lif_files + - path: /workflow/clem/sessions/{session_id}/clem/lif_files function: register_lif_file path_params: - name: session_id type: int methods: - POST - - path: /sessions/{session_id}/clem/tiff_files + - path: /workflow/clem/sessions/{session_id}/clem/tiff_files function: register_tiff_file path_params: - name: session_id type: int methods: - POST - - path: /sessions/{session_id}/clem/metadata_files + - path: /workflow/clem/sessions/{session_id}/clem/metadata_files function: register_clem_metadata path_params: - name: session_id type: int methods: - POST - - path: /sessions/{session_id}/clem/image_series + - path: /workflow/clem/sessions/{session_id}/clem/image_series function: register_image_series path_params: - name: session_id type: int methods: - POST - - path: /sessions/{session_id}/clem/image_stacks + - path: /workflow/clem/sessions/{session_id}/clem/image_stacks function: register_image_stack path_params: - name: session_id type: int methods: - POST - - path: /sessions/{session_id}/clem/preprocessing/process_raw_lifs + - path: /workflow/clem/sessions/{session_id}/clem/preprocessing/process_raw_lifs function: process_raw_lifs path_params: - name: session_id type: int methods: - POST - - path: /sessions/{session_id}/clem/preprocessing/process_raw_tiffs + - path: /workflow/clem/sessions/{session_id}/clem/preprocessing/process_raw_tiffs function: process_raw_tiffs path_params: - name: session_id type: int methods: - POST - - path: /sessions/{session_id}/clem/processing/align_and_merge_stacks + - path: /workflow/clem/sessions/{session_id}/clem/processing/align_and_merge_stacks function: align_and_merge_stacks path_params: - name: session_id From c5956e8c412fed72bb9394a11f32d245ea2c949c Mon Sep 17 00:00:00 2001 From: Eu Pin Tien Date: Wed, 4 Jun 2025 13:32:31 +0100 Subject: [PATCH 08/26] Moved 'count_number_of_omves()' from 'session_info' router to 'session_control'; updated route manifest and module where it's called --- src/murfey/client/contexts/spa.py | 2 +- src/murfey/server/api/session_control.py | 10 ++++++++++ src/murfey/server/api/session_info.py | 9 --------- src/murfey/util/route_manifest.yaml | 10 +++++----- 4 files changed, 16 insertions(+), 15 deletions(-) diff --git a/src/murfey/client/contexts/spa.py b/src/murfey/client/contexts/spa.py index 02e335384..6b249c8a3 100644 --- a/src/murfey/client/contexts/spa.py +++ b/src/murfey/client/contexts/spa.py @@ -567,7 +567,7 @@ def post_transfer( ) if not environment.movie_counters.get(str(source)): movie_counts_get = capture_get( - f"{environment.url.geturl()}{url_path_for('session_info.router', 'count_number_of_movies')}", + f"{environment.url.geturl()}{url_path_for('session_control.router', 'count_number_of_movies')}", ) if movie_counts_get is not None: environment.movie_counters[str(source)] = count( diff --git a/src/murfey/server/api/session_control.py b/src/murfey/server/api/session_control.py index e218f93c4..8be560b40 100644 --- a/src/murfey/server/api/session_control.py +++ b/src/murfey/server/api/session_control.py @@ -7,6 +7,7 @@ from fastapi.responses import FileResponse from ispyb.sqlalchemy import AutoProcProgram as ISPyBAutoProcProgram from pydantic import BaseModel +from sqlalchemy import func from sqlmodel import select from werkzeug.utils import secure_filename @@ -39,6 +40,7 @@ DataCollectionGroup, FoilHole, GridSquare, + Movie, ProcessingJob, RsyncInstance, Session, @@ -178,6 +180,14 @@ def register_processing_success_in_ispyb( _transport_object.do_update_processing_status(updated) +@router.get("/num_movies") +def count_number_of_movies(db=murfey_db) -> Dict[str, int]: + res = db.exec( + select(Movie.tag, func.count(Movie.murfey_id)).group_by(Movie.tag) + ).all() + return {r[0]: r[1] for r in res} + + class PostInfo(BaseModel): url: str data: dict diff --git a/src/murfey/server/api/session_info.py b/src/murfey/server/api/session_info.py index 2db5a3a79..d95336a8e 100644 --- a/src/murfey/server/api/session_info.py +++ b/src/murfey/server/api/session_info.py @@ -6,7 +6,6 @@ from fastapi import APIRouter, Depends, Request from fastapi.responses import FileResponse, HTMLResponse from pydantic import BaseModel -from sqlalchemy import func from sqlmodel import select from werkzeug.utils import secure_filename @@ -262,14 +261,6 @@ async def get_clients(db=murfey_db): return clients -@router.get("/num_movies") -def count_number_of_movies(db=murfey_db) -> Dict[str, int]: - res = db.exec( - select(Movie.tag, func.count(Movie.murfey_id)).group_by(Movie.tag) - ).all() - return {r[0]: r[1] for r in res} - - class CurrentGainRef(BaseModel): path: str diff --git a/src/murfey/util/route_manifest.yaml b/src/murfey/util/route_manifest.yaml index 52e1df6b5..ae187f251 100644 --- a/src/murfey/util/route_manifest.yaml +++ b/src/murfey/util/route_manifest.yaml @@ -706,6 +706,11 @@ murfey.server.api.session_control.router: path_params: [] methods: - POST + - path: /session_control/num_movies + function: count_number_of_movies + path_params: [] + methods: + - GET - path: /session_control/instruments/{instrument_name}/failed_client_post function: failed_client_post path_params: @@ -921,11 +926,6 @@ murfey.server.api.session_info.router: path_params: [] methods: - GET - - path: /session_info/num_movies - function: count_number_of_movies - path_params: [] - methods: - - GET - path: /session_info/sessions/{session_id}/current_gain_ref function: update_current_gain_ref path_params: [] From 5f66b8b09c07a2612fb98c04dd2b85b5eca8a2ee Mon Sep 17 00:00:00 2001 From: Eu Pin Tien Date: Thu, 5 Jun 2025 09:16:56 +0100 Subject: [PATCH 09/26] Fixed broken test; used 'url_path_for()' instead of hard-coding URL now --- tests/server/api/test_movies.py | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/tests/server/api/test_movies.py b/tests/server/api/test_movies.py index bc633016b..f24e770c7 100644 --- a/tests/server/api/test_movies.py +++ b/tests/server/api/test_movies.py @@ -6,6 +6,7 @@ from murfey.server.main import app from murfey.server.murfey_db import murfey_db_session +from murfey.util.api import url_path_for @pytest.fixture(scope="module") @@ -53,7 +54,8 @@ def login(test_user): def test_movie_count(mock_check, test_user): token = login(test_user) response = client.get( - "/session_info/num_movies", headers={"Authorization": f"Bearer {token}"} + f"{url_path_for('session_control.router', 'count_number_of_movies')}", + headers={"Authorization": f"Bearer {token}"}, ) assert mock_check.called_once() assert response.status_code == 200 From 7a9d83d1c2b194bf22f9f24a684d2c5aa9fbfcc9 Mon Sep 17 00:00:00 2001 From: Eu Pin Tien Date: Thu, 5 Jun 2025 09:19:43 +0100 Subject: [PATCH 10/26] Attempt to fix broken test --- tests/server/test_main.py | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/tests/server/test_main.py b/tests/server/test_main.py index 24aab534b..c9cb8db8f 100644 --- a/tests/server/test_main.py +++ b/tests/server/test_main.py @@ -30,7 +30,10 @@ def login(test_user): @patch("murfey.server.api.auth.check_user", return_value=True) def test_read_main(mock_check, test_user): token = login(test_user) - response = client.get("/session_info", headers={"Authorization": f"Bearer {token}"}) + response = client.get( + "/session_info/", + headers={"Authorization": f"Bearer {token}"}, + ) assert mock_check.called_once() assert response.status_code == 200 assert "" in response.text.lower() From e8f57e4facb56a1189bf88d36c92fd27aea571b3 Mon Sep 17 00:00:00 2001 From: Eu Pin Tien <eu-pin.tien@diamond.ac.uk> Date: Thu, 5 Jun 2025 09:29:26 +0100 Subject: [PATCH 11/26] Token generating URL was outdated; replaced with URL constructor; replaced 'test_pypi_proxy' URL with URl constructor as well --- tests/server/api/test_movies.py | 5 ++++- tests/server/test_main.py | 10 ++++++++-- 2 files changed, 12 insertions(+), 3 deletions(-) diff --git a/tests/server/api/test_movies.py b/tests/server/api/test_movies.py index f24e770c7..19ebe0fe7 100644 --- a/tests/server/api/test_movies.py +++ b/tests/server/api/test_movies.py @@ -42,7 +42,10 @@ def login(test_user): with patch( "murfey.server.api.auth.validate_user", return_value=True ) as mock_validate: - response = client.post("/token", data=test_user) + response = client.post( + f"{url_path_for('auth.router', 'generate_token')}", + data=test_user, + ) assert mock_validate.called_once() assert response.status_code == 200 token = response.json()["access_token"] diff --git a/tests/server/test_main.py b/tests/server/test_main.py index c9cb8db8f..ed223fef3 100644 --- a/tests/server/test_main.py +++ b/tests/server/test_main.py @@ -6,6 +6,7 @@ from fastapi.testclient import TestClient from murfey.server.main import app +from murfey.util.api import url_path_for client = TestClient(app) @@ -19,7 +20,10 @@ def login(test_user): with patch( "murfey.server.api.auth.validate_user", return_value=True ) as mock_validate: - response = client.post("/token", data=test_user) + response = client.post( + f"{url_path_for('auth.router', 'generate_token')}", + data=test_user, + ) assert mock_validate.called_once() assert response.status_code == 200 token = response.json()["access_token"] @@ -40,7 +44,9 @@ def test_read_main(mock_check, test_user): def test_pypi_proxy(): - response = client.get("/pypi/fastapi") + response = client.get( + f"{url_path_for('boostrap.pypi', 'get_pypi_package_downloads_list', package='fastapi')}" + ) assert response.status_code == 200 assert "<a href" in response.text.lower() assert ".tar.gz" in response.text.lower() From 96fce0c196e762fd573bdb791401790cfbcb828c Mon Sep 17 00:00:00 2001 From: Eu Pin Tien <eu-pin.tien@diamond.ac.uk> Date: Thu, 5 Jun 2025 09:43:34 +0100 Subject: [PATCH 12/26] Used FastAPI's 'APIKeyCookie' object for cookie authentication instead of homebrew method; split session access validation function for instrument server and frontend into separate functions; instrument validation function was incorrectly calling oauth2 scheme for frontend instead of backend; fixed logic for 'create_access_token' and 'generate_token' for handling authentication using either 'password' or 'cookie'; 'simple_token_validation()' should be using instrument server validation function instead --- src/murfey/server/api/auth.py | 287 +++++++++++++++------------------- 1 file changed, 127 insertions(+), 160 deletions(-) diff --git a/src/murfey/server/api/auth.py b/src/murfey/server/api/auth.py index f2c430988..3d806b82e 100644 --- a/src/murfey/server/api/auth.py +++ b/src/murfey/server/api/auth.py @@ -3,17 +3,22 @@ import secrets import time from logging import getLogger -from typing import Annotated, Dict +from typing import Dict from uuid import uuid4 import aiohttp import requests -from fastapi import APIRouter, Depends, HTTPException, Request, status -from fastapi.security import HTTPBearer, OAuth2PasswordBearer, OAuth2PasswordRequestForm +from fastapi import APIRouter, Depends, HTTPException, status +from fastapi.security import ( + APIKeyCookie, + OAuth2PasswordBearer, + OAuth2PasswordRequestForm, +) from jose import JWTError, jwt from passlib.context import CryptContext from pydantic import BaseModel from sqlmodel import Session, create_engine, select +from typing_extensions import Annotated from murfey.server.murfey_db import murfey_db, url from murfey.util.api import url_path_for @@ -31,38 +36,6 @@ ) -class CookieScheme(HTTPBearer): - def __init__( - self, - *, - description: str | None = None, - auto_error: bool = True, - cookie_key: str = "cookie_auth", - ): - """ - Args: - cookie_key: Cookie key to look for in requests - """ - super().__init__( - description=description, - auto_error=auto_error, - ) - - self.cookie_key = cookie_key - - async def __call__(self, request: Request): - token = request.cookies.get(self.cookie_key) - if token is None: - if self.auto_error: - raise HTTPException( - status_code=status.HTTP_401_UNAUTHORIZED, - detail="Not authenticated", - ) - else: - return None - return token - - # Set up variables used for authentication security_config = get_security_config() auth_url = security_config.auth_url @@ -71,7 +44,7 @@ async def __call__(self, request: Request): if security_config.auth_type == "password": oauth2_scheme = OAuth2PasswordBearer(tokenUrl="token") else: - oauth2_scheme = CookieScheme(cookie_key=security_config.cookie_key) + oauth2_scheme = APIKeyCookie(name=security_config.cookie_key) if security_config.instrument_auth_type == "token": instrument_oauth2_scheme = OAuth2PasswordBearer(tokenUrl="token") else: @@ -138,19 +111,19 @@ async def validate_token(token: Annotated[str, Depends(oauth2_scheme)]): validation_outcome = await response.json() if not (success and validation_outcome.get("valid")): raise JWTError - # Auth URL MUST be provided if authenticating using cookies + # If authenticating using cookies; an auth URL MUST be provided else: if security_config.auth_type == "cookie": raise JWTError - # Validate using password if security_config.auth_type == "password": decoded_data = jwt.decode(token, SECRET_KEY, algorithms=[ALGORITHM]) - # Frontend validation + # Check that the user is present and is valid if decoded_data.get("user"): if not check_user(decoded_data["user"]): raise JWTError - + else: + raise JWTError except JWTError: raise HTTPException( status_code=status.HTTP_401_UNAUTHORIZED, @@ -221,7 +194,7 @@ async def validate_instrument_token( """ ======================================================================================= -VALIDATING SESSION IDS +SESSION ID VALIDATION ======================================================================================= Annotated ints are defined here that trigger validation of the session IDs in incoming @@ -233,104 +206,87 @@ async def validate_instrument_token( """ -async def validate_visit(visit_name: str, token: str, instrument_access: bool) -> bool: - """ - Validates the incoming token depending on whether it's from the instrument or the frontend - """ - - # If this token is from the instrument server, validate this way - if instrument_access: - if security_config.instrument_auth_url: - async with aiohttp.ClientSession() as session: - headers = ( - {} - if not security_config.instrument_auth_type - else {"Authorization": f"Bearer {token}"} - ) - async with session.get( - f"{security_config.instrument_auth_url}/validate_visit_access/{visit_name}", - headers=headers, - ) as response: - success = response.status == 200 - validation_outcome = await response.json() - if not (success and validation_outcome.get("valid")): - return False - # Otherwise, use this validation method - else: - if security_config.auth_url: - headers = ( - {} - if security_config.auth_type == "cookie" - else {"Authorization": f"Bearer {token}"} - ) - cookies = ( - {security_config.cookie_key: token} - if security_config.auth_type == "cookie" - else {} - ) - async with aiohttp.ClientSession(cookies=cookies) as session: - async with session.get( - f"{auth_url}/validate_visit_access/{visit_name}", - headers=headers, - ) as response: - success = response.status == 200 - validation_outcome = await response.json() - if not (success and validation_outcome.get("valid")): - return False - return True +def get_visit_name(session_id: int) -> str: + with Session(engine) as murfey_db: + return ( + murfey_db.exec(select(MurfeySession).where(MurfeySession.id == session_id)) + .one() + .visit + ) -async def validate_session_access( +async def validate_frontend_session_access( session_id: int, token: Annotated[str, Depends(oauth2_scheme)], - instrument_access: bool, ) -> int: """ - Validates whether the request is authorised to access information about this session + Validates whether a frontend request can access information about this session """ - with Session(engine) as murfey_db: - visit_name = ( - murfey_db.exec(select(MurfeySession).where(MurfeySession.id == session_id)) - .one() - .visit + visit_name = get_visit_name(session_id) + + if auth_url: + headers = ( + {} + if security_config.auth_type == "cookie" + else {"Authorization": f"Bearer {token}"} ) - validated = await validate_visit(visit_name, token, instrument_access) - if not validated: - raise HTTPException( - status_code=status.HTTP_401_UNAUTHORIZED, - detail="You do not have access to this visit", - headers={"WWW-Authenticate": "Bearer"}, + cookies = ( + {security_config.cookie_key: token} + if security_config.auth_type == "cookie" + else {} ) + async with aiohttp.ClientSession(cookies=cookies) as session: + async with session.get( + f"{auth_url}/validate_visit_access/{visit_name}", + headers=headers, + ) as response: + success = response.status == 200 + validation_outcome: dict = await response.json() + if not (success and validation_outcome.get("valid")): + logger.warning("Unauthorised visit access request from frontend") + raise HTTPException( + status_code=status.HTTP_401_UNAUTHORIZED, + detail="You do not have access to this visit", + headers={"WWW-Authenticate": "Bearer"}, + ) return session_id -def validate_session_access_wrapper(instrument_access: bool): +async def validate_instrument_session_access( + session_id: int, + token: Annotated[str, Depends(instrument_oauth2_scheme)], +) -> int: """ - Factory that returns an async wrapper arond 'validate_session_access' with the - 'instrument_access' field preconfigured. - - This is used to generate FastAPI-compatible dependencies for validating session - access, based on the context of the request. - - Unlike 'functools.partial', this approach preserves introspection compatibility - required by FastAPI for dependency resolution and OpenAPI generation. + Validates whether an instrument request can access information about this session """ + visit_name = get_visit_name(session_id) - async def _validate(session_id: int, token: Annotated[str, Depends(oauth2_scheme)]): - return await validate_session_access( - session_id, token, instrument_access=instrument_access - ) - - return _validate + if security_config.instrument_auth_url: + async with aiohttp.ClientSession() as session: + headers = ( + {} + if not security_config.instrument_auth_type + else {"Authorization": f"Bearer {token}"} + ) + async with session.get( + f"{security_config.instrument_auth_url}/validate_visit_access/{visit_name}", + headers=headers, + ) as response: + success = response.status == 200 + validation_outcome = await response.json() + if not (success and validation_outcome.get("valid")): + logger.warning("Unauthorised visit access request from instrument") + raise HTTPException( + status_code=status.HTTP_401_UNAUTHORIZED, + detail="You do not have access to this visit", + headers={"WWW-Authenticate": "Bearer"}, + ) + return session_id # Set validation conditions for the session ID based on where the request is from -MurfeySessionIDFrontend = Annotated[ - int, Depends(validate_session_access_wrapper(instrument_access=False)) -] -MurfeySessionIDInstrument = Annotated[ - int, Depends(validate_session_access_wrapper(instrument_access=True)) -] +MurfeySessionIDFrontend = Annotated[int, Depends(validate_frontend_session_access)] +MurfeySessionIDInstrument = Annotated[int, Depends(validate_instrument_session_access)] """ @@ -354,23 +310,27 @@ def validate_user(username: str, password: str) -> bool: def create_access_token(data: dict, token: str = "") -> str: - if auth_url and data.get("session"): - session_id = data["session"] - if not isinstance(session_id, int) and session_id > 0: - # check the session ID is alphanumeric for security - raise ValueError("Session ID was invalid (not alphanumeric)") - minted_token_response = requests.get( - f"{auth_url}{url_path_for('auth.router', 'mint_session_token', session_id=session_id)}", - headers={"Authorization": f"Bearer {token}"}, - ) - if minted_token_response.status_code != 200: - raise RuntimeError( - f"Request received status code {minted_token_response.status_code} when trying to create session token" + + # If authenticating with password, auth URL needs a 'mint_session_token' endpoint + if security_config.auth_type == "password": + if auth_url and data.get("session"): + session_id = data["session"] + if not isinstance(session_id, int) and session_id > 0: + # Check the session ID is alphanumeric for security + raise ValueError("Session ID was invalid (not alphanumeric)") + minted_token_response = requests.get( + f"{auth_url}{url_path_for('auth.router', 'mint_session_token', session_id=session_id)}", + headers={"Authorization": f"Bearer {token}"}, ) - return minted_token_response.json()["access_token"] + if minted_token_response.status_code != 200: + raise RuntimeError( + f"Request received status code {minted_token_response.status_code} when trying to create session token" + ) + return minted_token_response.json()["access_token"] to_encode = data.copy() + # Make token for instrument encoded_jwt = jwt.encode(to_encode, SECRET_KEY, algorithm=ALGORITHM) return encoded_jwt @@ -384,31 +344,36 @@ class Token(BaseModel): async def generate_token( form_data: Annotated[OAuth2PasswordRequestForm, Depends()], ) -> Token: - if auth_url: - data = aiohttp.FormData() - data.add_field("username", form_data.username) - data.add_field("password", form_data.password) - async with aiohttp.ClientSession() as session: - async with session.post( - f"{auth_url}{url_path_for('auth.router', 'generate_token')}", - data=data, - ) as response: - validated = response.status == 200 - token = await response.json() - access_token = token.get("access_token") - else: - validated = validate_user(form_data.username, form_data.password) - if not validated: - raise HTTPException( - status_code=status.HTTP_401_UNAUTHORIZED, - detail="Incorrect username or password", - headers={"WWW-Authenticate": "Bearer"}, - ) - if not auth_url: - access_token = create_access_token( - data={"user": form_data.username}, - ) - return Token(access_token=access_token, token_type="bearer") + # Only generate a token if it's a password + if security_config.auth_type == "password": + if auth_url: + data = aiohttp.FormData() + data.add_field("username", form_data.username) + data.add_field("password", form_data.password) + async with aiohttp.ClientSession() as session: + async with session.post( + f"{auth_url}{url_path_for('auth.router', 'generate_token')}", + data=data, + ) as response: + validated = response.status == 200 + token = await response.json() + access_token = token.get("access_token") + else: + validated = validate_user(form_data.username, form_data.password) + if not validated: + raise HTTPException( + status_code=status.HTTP_401_UNAUTHORIZED, + detail="Incorrect username or password", + headers={"WWW-Authenticate": "Bearer"}, + ) + if not auth_url: + access_token = create_access_token( + data={"user": form_data.username}, + ) + return Token(access_token=access_token, token_type="bearer") + + # Return empty token otherwise + return Token(access_token="", token_type="bearer") @router.get("/sessions/{session_id}/token") @@ -431,5 +396,7 @@ async def mint_session_token(session_id: MurfeySessionIDFrontend, db=murfey_db): @router.get("/validate_token") -async def simple_token_validation(token: Annotated[str, Depends(validate_token)]): +async def simple_token_validation( + token: Annotated[str, Depends(validate_instrument_token)] +): return {"valid": True} From fb19d6c331b0ca91fccde573a2896ddf25bf22d9 Mon Sep 17 00:00:00 2001 From: Eu Pin Tien <eu-pin.tien@diamond.ac.uk> Date: Thu, 5 Jun 2025 09:45:55 +0100 Subject: [PATCH 13/26] Typo in router name --- tests/server/test_main.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/server/test_main.py b/tests/server/test_main.py index ed223fef3..2529b6b64 100644 --- a/tests/server/test_main.py +++ b/tests/server/test_main.py @@ -45,7 +45,7 @@ def test_read_main(mock_check, test_user): def test_pypi_proxy(): response = client.get( - f"{url_path_for('boostrap.pypi', 'get_pypi_package_downloads_list', package='fastapi')}" + f"{url_path_for('bootstrap.pypi', 'get_pypi_package_downloads_list', package='fastapi')}" ) assert response.status_code == 200 assert "<a href" in response.text.lower() From ceebff5092a87b739d988d782035bab7bd1aa1ea Mon Sep 17 00:00:00 2001 From: Eu Pin Tien <eu-pin.tien@diamond.ac.uk> Date: Thu, 5 Jun 2025 09:55:48 +0100 Subject: [PATCH 14/26] Adjusted logic in 'password' block of 'generate_token()' to avoid uninitialised local variable --- src/murfey/server/api/auth.py | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/src/murfey/server/api/auth.py b/src/murfey/server/api/auth.py index 3d806b82e..4a61b2868 100644 --- a/src/murfey/server/api/auth.py +++ b/src/murfey/server/api/auth.py @@ -360,16 +360,15 @@ async def generate_token( access_token = token.get("access_token") else: validated = validate_user(form_data.username, form_data.password) + access_token = create_access_token( + data={"user": form_data.username}, + ) if not validated: raise HTTPException( status_code=status.HTTP_401_UNAUTHORIZED, detail="Incorrect username or password", headers={"WWW-Authenticate": "Bearer"}, ) - if not auth_url: - access_token = create_access_token( - data={"user": form_data.username}, - ) return Token(access_token=access_token, token_type="bearer") # Return empty token otherwise From 0bfeafa93ca3f09f811bfdc4a67414cd462b4cd3 Mon Sep 17 00:00:00 2001 From: Eu Pin Tien <eu-pin.tien@diamond.ac.uk> Date: Thu, 5 Jun 2025 10:34:24 +0100 Subject: [PATCH 15/26] Added mock database to 'test_movie_count()', use 'mint_session_token()' instead of 'generate_token()' --- tests/server/api/test_movies.py | 13 +++++++++++-- 1 file changed, 11 insertions(+), 2 deletions(-) diff --git a/tests/server/api/test_movies.py b/tests/server/api/test_movies.py index 19ebe0fe7..23f53b582 100644 --- a/tests/server/api/test_movies.py +++ b/tests/server/api/test_movies.py @@ -43,7 +43,7 @@ def login(test_user): "murfey.server.api.auth.validate_user", return_value=True ) as mock_validate: response = client.post( - f"{url_path_for('auth.router', 'generate_token')}", + f"{url_path_for('auth.router', 'mint_session_token', session_id=1)}", data=test_user, ) assert mock_validate.called_once() @@ -54,12 +54,21 @@ def login(test_user): @patch("murfey.server.api.auth.check_user", return_value=True) -def test_movie_count(mock_check, test_user): +@patch("murfey.server.api.auth.murfey_db") +def test_movie_count( + mock_check, + mock_murfey_db, + test_user, + murfey_db_session, +): + + mock_murfey_db = murfey_db_session token = login(test_user) response = client.get( f"{url_path_for('session_control.router', 'count_number_of_movies')}", headers={"Authorization": f"Bearer {token}"}, ) + assert mock_murfey_db assert mock_check.called_once() assert response.status_code == 200 assert len(mock_session.method_calls) == 2 From 3076af28bdbc667921c2dc51c9c7142cef79a97e Mon Sep 17 00:00:00 2001 From: Eu Pin Tien <eu-pin.tien@diamond.ac.uk> Date: Thu, 5 Jun 2025 11:31:06 +0100 Subject: [PATCH 16/26] Rewrote 'test_movie_count' function to make use database set up in pytest --- tests/server/api/test_movies.py | 91 +++++++++++++++------------------ 1 file changed, 42 insertions(+), 49 deletions(-) diff --git a/tests/server/api/test_movies.py b/tests/server/api/test_movies.py index 23f53b582..c4f5032e7 100644 --- a/tests/server/api/test_movies.py +++ b/tests/server/api/test_movies.py @@ -1,75 +1,68 @@ -from unittest.mock import Mock, create_autospec, patch +from unittest.mock import ANY -import pytest from fastapi.testclient import TestClient from sqlmodel import Session +from murfey.server.api.auth import validate_instrument_token from murfey.server.main import app -from murfey.server.murfey_db import murfey_db_session +from murfey.server.murfey_db import murfey_db from murfey.util.api import url_path_for +from murfey.util.db import Movie +# @pytest.fixture(scope="module") +# def test_user(): +# return {"username": "testuser", "password": "testpass"} -@pytest.fixture(scope="module") -def test_user(): - return {"username": "testuser", "password": "testpass"} +# def movies_return(): +# return [("Supervisor_1", 2)] -def movies_return(): - return [("Supervisor_1", 2)] +# expression = Mock() +# expression.all = movies_return -expression = Mock() -expression.all = movies_return +# mock_session = create_autospec(Session, instance=True) +# mock_session.exec.return_value = expression -mock_session = create_autospec(Session, instance=True) -mock_session.exec.return_value = expression +# def override_murfey_db(): +# try: +# db = mock_session +# yield db +# finally: +# db.close() -def override_murfey_db(): - try: - db = mock_session - yield db - finally: - db.close() - -app.dependency_overrides[murfey_db_session] = override_murfey_db +# app.dependency_overrides[murfey_db] = override_murfey_db client = TestClient(app) -def login(test_user): - with patch( - "murfey.server.api.auth.validate_user", return_value=True - ) as mock_validate: - response = client.post( - f"{url_path_for('auth.router', 'mint_session_token', session_id=1)}", - data=test_user, - ) - assert mock_validate.called_once() - assert response.status_code == 200 - token = response.json()["access_token"] - assert token is not None - return token - - -@patch("murfey.server.api.auth.check_user", return_value=True) -@patch("murfey.server.api.auth.murfey_db") def test_movie_count( - mock_check, - mock_murfey_db, - test_user, - murfey_db_session, + murfey_db_session: Session, ): - mock_murfey_db = murfey_db_session - token = login(test_user) + # Insert test movies into Murfey DB + tag = "test_movie" + num_movies = 5 + murfey_db_session + for i in range(num_movies): + movie_db_entry = Movie( + murfey_id=i, + path="/some/path", + image_number=i, + tag=tag, + ) + murfey_db_session.add(movie_db_entry) + murfey_db_session.commit() + + # Replace the murfey_db instance in endpoint with properly initialised pytest one + app.dependency_overrides[murfey_db] = murfey_db_session + # Disable instrument token validation + app.dependency_overrides[validate_instrument_token] = lambda: None + response = client.get( f"{url_path_for('session_control.router', 'count_number_of_movies')}", - headers={"Authorization": f"Bearer {token}"}, + headers={"Authorization": f"Bearer {ANY}"}, ) - assert mock_murfey_db - assert mock_check.called_once() - assert response.status_code == 200 - assert len(mock_session.method_calls) == 2 - assert response.json() == {"Supervisor_1": 2} + assert response.json() == {tag: num_movies} From 27748f024201b516c3f3c9f33f88e76e44da63fa Mon Sep 17 00:00:00 2001 From: Eu Pin Tien <eu-pin.tien@diamond.ac.uk> Date: Thu, 5 Jun 2025 12:17:47 +0100 Subject: [PATCH 17/26] Use Murfey database fixture and add entries to it --- tests/server/api/test_movies.py | 75 +++++++++++++++++++++++++++++---- 1 file changed, 66 insertions(+), 9 deletions(-) diff --git a/tests/server/api/test_movies.py b/tests/server/api/test_movies.py index c4f5032e7..8eb4178f4 100644 --- a/tests/server/api/test_movies.py +++ b/tests/server/api/test_movies.py @@ -7,7 +7,16 @@ from murfey.server.main import app from murfey.server.murfey_db import murfey_db from murfey.util.api import url_path_for -from murfey.util.db import Movie +from murfey.util.db import ( + AutoProcProgram, + DataCollection, + DataCollectionGroup, + Movie, + MurfeyLedger, + ProcessingJob, +) +from murfey.util.db import Session as MurfeySession +from tests.conftest import ExampleVisit, get_or_create_db_entry # @pytest.fixture(scope="module") # def test_user(): @@ -42,19 +51,67 @@ def test_movie_count( murfey_db_session: Session, ): - # Insert test movies into Murfey DB + # Insert table dependencies + session_entry: MurfeySession = get_or_create_db_entry( + murfey_db_session, + MurfeySession, + lookup_kwargs={"id": ExampleVisit.murfey_session_id}, + ) + dcg_entry: DataCollectionGroup = get_or_create_db_entry( + murfey_db_session, + DataCollectionGroup, + lookup_kwargs={ + "id": 0, + "session_id": session_entry.id, + "tag": "test_dcg", + }, + ) + dc_entry: DataCollection = get_or_create_db_entry( + murfey_db_session, + DataCollection, + lookup_kwargs={"id": 0, "tag": "test_dc", "dcg_id": dcg_entry.id}, + ) + processing_job_entry: ProcessingJob = get_or_create_db_entry( + murfey_db_session, + ProcessingJob, + lookup_kwargs={ + "id": 0, + "recipe": "test_recipe", + "dc_id": dc_entry.id, + }, + ) + autoproc_entry: AutoProcProgram = get_or_create_db_entry( + murfey_db_session, + AutoProcProgram, + lookup_kwargs={ + "id": 0, + "pj_id": processing_job_entry.id, + }, + ) + + # Insert test movies and one-to-one dependencies into Murfey DB tag = "test_movie" num_movies = 5 murfey_db_session for i in range(num_movies): - movie_db_entry = Movie( - murfey_id=i, - path="/some/path", - image_number=i, - tag=tag, + murfey_ledger_entry: MurfeyLedger = get_or_create_db_entry( + murfey_db_session, + MurfeyLedger, + lookup_kwargs={ + "id": i, + "app_id": autoproc_entry.id, + }, + ) + _: Movie = get_or_create_db_entry( + murfey_db_session, + Movie, + lookup_kwargs={ + "murfey_id": murfey_ledger_entry.id, + "path": "/some/path", + "image_number": i, + "tag": tag, + }, ) - murfey_db_session.add(movie_db_entry) - murfey_db_session.commit() # Replace the murfey_db instance in endpoint with properly initialised pytest one app.dependency_overrides[murfey_db] = murfey_db_session From 89a06077775f2db32d4ea459f5de615aef391f9a Mon Sep 17 00:00:00 2001 From: Eu Pin Tien <eu-pin.tien@diamond.ac.uk> Date: Thu, 5 Jun 2025 12:26:53 +0100 Subject: [PATCH 18/26] No need to re-do Session table entry --- tests/server/api/test_movies.py | 14 ++++++-------- 1 file changed, 6 insertions(+), 8 deletions(-) diff --git a/tests/server/api/test_movies.py b/tests/server/api/test_movies.py index 8eb4178f4..1ccc1ba3e 100644 --- a/tests/server/api/test_movies.py +++ b/tests/server/api/test_movies.py @@ -15,7 +15,6 @@ MurfeyLedger, ProcessingJob, ) -from murfey.util.db import Session as MurfeySession from tests.conftest import ExampleVisit, get_or_create_db_entry # @pytest.fixture(scope="module") @@ -52,24 +51,23 @@ def test_movie_count( ): # Insert table dependencies - session_entry: MurfeySession = get_or_create_db_entry( - murfey_db_session, - MurfeySession, - lookup_kwargs={"id": ExampleVisit.murfey_session_id}, - ) dcg_entry: DataCollectionGroup = get_or_create_db_entry( murfey_db_session, DataCollectionGroup, lookup_kwargs={ "id": 0, - "session_id": session_entry.id, + "session_id": ExampleVisit.murfey_session_id, "tag": "test_dcg", }, ) dc_entry: DataCollection = get_or_create_db_entry( murfey_db_session, DataCollection, - lookup_kwargs={"id": 0, "tag": "test_dc", "dcg_id": dcg_entry.id}, + lookup_kwargs={ + "id": 0, + "tag": "test_dc", + "dcg_id": dcg_entry.id, + }, ) processing_job_entry: ProcessingJob = get_or_create_db_entry( murfey_db_session, From 00fe67f55376388c582e3b8de79b277c6cb85cb8 Mon Sep 17 00:00:00 2001 From: Eu Pin Tien <eu-pin.tien@diamond.ac.uk> Date: Thu, 5 Jun 2025 12:45:43 +0100 Subject: [PATCH 19/26] Put the database and validation overrides in a test FastAPI client fixture --- tests/server/api/test_movies.py | 22 ++++++++++++++-------- 1 file changed, 14 insertions(+), 8 deletions(-) diff --git a/tests/server/api/test_movies.py b/tests/server/api/test_movies.py index 1ccc1ba3e..d644cb6ce 100644 --- a/tests/server/api/test_movies.py +++ b/tests/server/api/test_movies.py @@ -1,6 +1,7 @@ from unittest.mock import ANY from fastapi.testclient import TestClient +from pytest import fixture from sqlmodel import Session from murfey.server.api.auth import validate_instrument_token @@ -43,11 +44,21 @@ # app.dependency_overrides[murfey_db] = override_murfey_db -client = TestClient(app) + +@fixture(scope="module") +def fastapi_client(murfey_db_session): + # Replace the murfey_db instance in endpoint with properly initialised pytest one + app.dependency_overrides[murfey_db] = murfey_db_session + # Disable instrument token validation + app.dependency_overrides[validate_instrument_token] = lambda: None + + with TestClient(app) as client: + yield client def test_movie_count( - murfey_db_session: Session, + fastapi_client: TestClient, + murfey_db_session: Session, # From conftest.py ): # Insert table dependencies @@ -111,12 +122,7 @@ def test_movie_count( }, ) - # Replace the murfey_db instance in endpoint with properly initialised pytest one - app.dependency_overrides[murfey_db] = murfey_db_session - # Disable instrument token validation - app.dependency_overrides[validate_instrument_token] = lambda: None - - response = client.get( + response = fastapi_client.get( f"{url_path_for('session_control.router', 'count_number_of_movies')}", headers={"Authorization": f"Bearer {ANY}"}, ) From 7fae5d83b805c455cff304a17cf2fd1b19fbcc2d Mon Sep 17 00:00:00 2001 From: Eu Pin Tien <eu-pin.tien@diamond.ac.uk> Date: Thu, 5 Jun 2025 13:32:26 +0100 Subject: [PATCH 20/26] Database session gets reset every test, so the FastAPI test client that uses the session as a dependency has to be regenerated every test too --- tests/server/api/test_movies.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/tests/server/api/test_movies.py b/tests/server/api/test_movies.py index d644cb6ce..258740396 100644 --- a/tests/server/api/test_movies.py +++ b/tests/server/api/test_movies.py @@ -1,7 +1,7 @@ from unittest.mock import ANY +import pytest from fastapi.testclient import TestClient -from pytest import fixture from sqlmodel import Session from murfey.server.api.auth import validate_instrument_token @@ -45,7 +45,7 @@ # app.dependency_overrides[murfey_db] = override_murfey_db -@fixture(scope="module") +@pytest.fixture def fastapi_client(murfey_db_session): # Replace the murfey_db instance in endpoint with properly initialised pytest one app.dependency_overrides[murfey_db] = murfey_db_session From 226926989cd649544e48125b29568bba03517ac1 Mon Sep 17 00:00:00 2001 From: Eu Pin Tien <eu-pin.tien@diamond.ac.uk> Date: Thu, 5 Jun 2025 14:00:18 +0100 Subject: [PATCH 21/26] Moved Murfey DB URL out of fixture and into a module-level variable --- tests/conftest.py | 11 ++++++----- 1 file changed, 6 insertions(+), 5 deletions(-) diff --git a/tests/conftest.py b/tests/conftest.py index bb1d86f37..ad55f10d8 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -285,14 +285,15 @@ def ispyb_db_session( ======================================================================================= """ +murfey_db_url = ( + f"postgresql+psycopg2://{os.environ['POSTGRES_USER']}:{os.environ['POSTGRES_PASSWORD']}" + f"@{os.environ['POSTGRES_HOST']}:{os.environ['POSTGRES_PORT']}/{os.environ['POSTGRES_DB']}" +) + @pytest.fixture(scope="session") def murfey_db_engine(): - url = ( - f"postgresql+psycopg2://{os.environ['POSTGRES_USER']}:{os.environ['POSTGRES_PASSWORD']}" - f"@{os.environ['POSTGRES_HOST']}:{os.environ['POSTGRES_PORT']}/{os.environ['POSTGRES_DB']}" - ) - engine = create_engine(url) + engine = create_engine(murfey_db_url) SQLModel.metadata.create_all(engine) yield engine engine.dispose() From e6c0f3ceb8441fe38a0040325aa48f7fe220d44d Mon Sep 17 00:00:00 2001 From: Eu Pin Tien <eu-pin.tien@diamond.ac.uk> Date: Thu, 5 Jun 2025 14:00:44 +0100 Subject: [PATCH 22/26] Import 'murfey_db' only after mocks have been set --- tests/server/api/test_movies.py | 24 ++++++++++++++++++++---- 1 file changed, 20 insertions(+), 4 deletions(-) diff --git a/tests/server/api/test_movies.py b/tests/server/api/test_movies.py index 258740396..df47021e1 100644 --- a/tests/server/api/test_movies.py +++ b/tests/server/api/test_movies.py @@ -1,4 +1,4 @@ -from unittest.mock import ANY +from unittest.mock import ANY, patch import pytest from fastapi.testclient import TestClient @@ -6,8 +6,8 @@ from murfey.server.api.auth import validate_instrument_token from murfey.server.main import app -from murfey.server.murfey_db import murfey_db from murfey.util.api import url_path_for +from murfey.util.config import security_from_file from murfey.util.db import ( AutoProcProgram, DataCollection, @@ -16,7 +16,7 @@ MurfeyLedger, ProcessingJob, ) -from tests.conftest import ExampleVisit, get_or_create_db_entry +from tests.conftest import ExampleVisit, get_or_create_db_entry, murfey_db_url # @pytest.fixture(scope="module") # def test_user(): @@ -45,8 +45,24 @@ # app.dependency_overrides[murfey_db] = override_murfey_db +@patch("murfey.server.murfey_db.get_security_config") +@patch("murfey.server.murfey_db.url") @pytest.fixture -def fastapi_client(murfey_db_session): +def fastapi_client( + mock_get_url, + mock_get_security_config, + mock_security_configuration, + murfey_db_session, +): + # Set up mock security configs for + mock_get_url.return_value = murfey_db_url + mock_get_security_config.return_value = security_from_file( + mock_security_configuration + ) + + # Defer import of 'murfey_db' until after mocks are configured + from murfey.server.murfey_db import murfey_db + # Replace the murfey_db instance in endpoint with properly initialised pytest one app.dependency_overrides[murfey_db] = murfey_db_session # Disable instrument token validation From c35cc7427f95e81a8b45730b9d81e83f97c02535 Mon Sep 17 00:00:00 2001 From: Eu Pin Tien <eu-pin.tien@diamond.ac.uk> Date: Thu, 5 Jun 2025 14:18:46 +0100 Subject: [PATCH 23/26] Removed patch decorators from fixtures due to decorator conflict --- tests/server/api/test_movies.py | 42 +++++++++++++++++---------------- 1 file changed, 22 insertions(+), 20 deletions(-) diff --git a/tests/server/api/test_movies.py b/tests/server/api/test_movies.py index df47021e1..25588be44 100644 --- a/tests/server/api/test_movies.py +++ b/tests/server/api/test_movies.py @@ -45,35 +45,37 @@ # app.dependency_overrides[murfey_db] = override_murfey_db -@patch("murfey.server.murfey_db.get_security_config") -@patch("murfey.server.murfey_db.url") @pytest.fixture -def fastapi_client( - mock_get_url, - mock_get_security_config, +def fastapi_test_client( mock_security_configuration, murfey_db_session, ): # Set up mock security configs for - mock_get_url.return_value = murfey_db_url - mock_get_security_config.return_value = security_from_file( - mock_security_configuration - ) - - # Defer import of 'murfey_db' until after mocks are configured - from murfey.server.murfey_db import murfey_db + with ( + patch( + "murfey.server.murfey_db.get_security_config" + ) as mock_get_security_config, + patch("murfey.server.murfey_db.url") as mock_get_url, + ): + + # Defer import of 'murfey_db' until after mocks are configured + mock_get_url.return_value = murfey_db_url + mock_get_security_config.return_value = security_from_file( + mock_security_configuration + ) + from murfey.server.murfey_db import murfey_db - # Replace the murfey_db instance in endpoint with properly initialised pytest one - app.dependency_overrides[murfey_db] = murfey_db_session - # Disable instrument token validation - app.dependency_overrides[validate_instrument_token] = lambda: None + # Replace the murfey_db instance in endpoint with properly initialised pytest one + app.dependency_overrides[murfey_db] = murfey_db_session + # Disable instrument token validation + app.dependency_overrides[validate_instrument_token] = lambda: None - with TestClient(app) as client: - yield client + with TestClient(app) as client: + yield client def test_movie_count( - fastapi_client: TestClient, + fastapi_test_client: TestClient, murfey_db_session: Session, # From conftest.py ): @@ -138,7 +140,7 @@ def test_movie_count( }, ) - response = fastapi_client.get( + response = fastapi_test_client.get( f"{url_path_for('session_control.router', 'count_number_of_movies')}", headers={"Authorization": f"Bearer {ANY}"}, ) From 8cb679a354b734deb896a8f487fbfb1e54389b0e Mon Sep 17 00:00:00 2001 From: Eu Pin Tien <eu-pin.tien@diamond.ac.uk> Date: Thu, 5 Jun 2025 14:26:43 +0100 Subject: [PATCH 24/26] Move test_client startup into test function, as database state doesn't carry over from test function into endpoint --- tests/server/api/test_movies.py | 148 +++++++++++++++----------------- 1 file changed, 70 insertions(+), 78 deletions(-) diff --git a/tests/server/api/test_movies.py b/tests/server/api/test_movies.py index 25588be44..89ce5c203 100644 --- a/tests/server/api/test_movies.py +++ b/tests/server/api/test_movies.py @@ -1,6 +1,5 @@ from unittest.mock import ANY, patch -import pytest from fastapi.testclient import TestClient from sqlmodel import Session @@ -45,11 +44,11 @@ # app.dependency_overrides[murfey_db] = override_murfey_db -@pytest.fixture -def fastapi_test_client( - mock_security_configuration, - murfey_db_session, +def test_movie_count( + mock_security_configuration, # From conftest.py + murfey_db_session: Session, # From conftest.py ): + # Set up mock security configs for with ( patch( @@ -71,77 +70,70 @@ def fastapi_test_client( app.dependency_overrides[validate_instrument_token] = lambda: None with TestClient(app) as client: - yield client - - -def test_movie_count( - fastapi_test_client: TestClient, - murfey_db_session: Session, # From conftest.py -): - - # Insert table dependencies - dcg_entry: DataCollectionGroup = get_or_create_db_entry( - murfey_db_session, - DataCollectionGroup, - lookup_kwargs={ - "id": 0, - "session_id": ExampleVisit.murfey_session_id, - "tag": "test_dcg", - }, - ) - dc_entry: DataCollection = get_or_create_db_entry( - murfey_db_session, - DataCollection, - lookup_kwargs={ - "id": 0, - "tag": "test_dc", - "dcg_id": dcg_entry.id, - }, - ) - processing_job_entry: ProcessingJob = get_or_create_db_entry( - murfey_db_session, - ProcessingJob, - lookup_kwargs={ - "id": 0, - "recipe": "test_recipe", - "dc_id": dc_entry.id, - }, - ) - autoproc_entry: AutoProcProgram = get_or_create_db_entry( - murfey_db_session, - AutoProcProgram, - lookup_kwargs={ - "id": 0, - "pj_id": processing_job_entry.id, - }, - ) - - # Insert test movies and one-to-one dependencies into Murfey DB - tag = "test_movie" - num_movies = 5 - murfey_db_session - for i in range(num_movies): - murfey_ledger_entry: MurfeyLedger = get_or_create_db_entry( - murfey_db_session, - MurfeyLedger, - lookup_kwargs={ - "id": i, - "app_id": autoproc_entry.id, - }, - ) - _: Movie = get_or_create_db_entry( - murfey_db_session, - Movie, - lookup_kwargs={ - "murfey_id": murfey_ledger_entry.id, - "path": "/some/path", - "image_number": i, - "tag": tag, - }, - ) - response = fastapi_test_client.get( - f"{url_path_for('session_control.router', 'count_number_of_movies')}", - headers={"Authorization": f"Bearer {ANY}"}, - ) - assert response.json() == {tag: num_movies} + # Insert table dependencies + dcg_entry: DataCollectionGroup = get_or_create_db_entry( + murfey_db_session, + DataCollectionGroup, + lookup_kwargs={ + "id": 0, + "session_id": ExampleVisit.murfey_session_id, + "tag": "test_dcg", + }, + ) + dc_entry: DataCollection = get_or_create_db_entry( + murfey_db_session, + DataCollection, + lookup_kwargs={ + "id": 0, + "tag": "test_dc", + "dcg_id": dcg_entry.id, + }, + ) + processing_job_entry: ProcessingJob = get_or_create_db_entry( + murfey_db_session, + ProcessingJob, + lookup_kwargs={ + "id": 0, + "recipe": "test_recipe", + "dc_id": dc_entry.id, + }, + ) + autoproc_entry: AutoProcProgram = get_or_create_db_entry( + murfey_db_session, + AutoProcProgram, + lookup_kwargs={ + "id": 0, + "pj_id": processing_job_entry.id, + }, + ) + + # Insert test movies and one-to-one dependencies into Murfey DB + tag = "test_movie" + num_movies = 5 + murfey_db_session + for i in range(num_movies): + murfey_ledger_entry: MurfeyLedger = get_or_create_db_entry( + murfey_db_session, + MurfeyLedger, + lookup_kwargs={ + "id": i, + "app_id": autoproc_entry.id, + }, + ) + _: Movie = get_or_create_db_entry( + murfey_db_session, + Movie, + lookup_kwargs={ + "murfey_id": murfey_ledger_entry.id, + "path": "/some/path", + "image_number": i, + "tag": tag, + }, + ) + + response = client.get( + f"{url_path_for('session_control.router', 'count_number_of_movies')}", + headers={"Authorization": f"Bearer {ANY}"}, + ) + assert response.json() == {tag: num_movies} From de4a73a9359e6990edbecd7436a988dffd96fd15 Mon Sep 17 00:00:00 2001 From: Eu Pin Tien <eu-pin.tien@diamond.ac.uk> Date: Thu, 5 Jun 2025 14:33:38 +0100 Subject: [PATCH 25/26] =?UTF-8?q?Test=20funciton=20directly=20(=E3=83=8E?= =?UTF-8?q?=E0=B2=A0=E7=9B=8A=E0=B2=A0)=E3=83=8E=E5=BD=A1=E2=94=BB?= =?UTF-8?q?=E2=94=81=E2=94=BB?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- tests/server/api/test_movies.py | 185 +++++++++++--------------------- 1 file changed, 64 insertions(+), 121 deletions(-) diff --git a/tests/server/api/test_movies.py b/tests/server/api/test_movies.py index 89ce5c203..4cf3e2e77 100644 --- a/tests/server/api/test_movies.py +++ b/tests/server/api/test_movies.py @@ -1,12 +1,6 @@ -from unittest.mock import ANY, patch - -from fastapi.testclient import TestClient from sqlmodel import Session -from murfey.server.api.auth import validate_instrument_token -from murfey.server.main import app -from murfey.util.api import url_path_for -from murfey.util.config import security_from_file +from murfey.server.api.session_control import count_number_of_movies from murfey.util.db import ( AutoProcProgram, DataCollection, @@ -15,125 +9,74 @@ MurfeyLedger, ProcessingJob, ) -from tests.conftest import ExampleVisit, get_or_create_db_entry, murfey_db_url - -# @pytest.fixture(scope="module") -# def test_user(): -# return {"username": "testuser", "password": "testpass"} - - -# def movies_return(): -# return [("Supervisor_1", 2)] - - -# expression = Mock() -# expression.all = movies_return - -# mock_session = create_autospec(Session, instance=True) -# mock_session.exec.return_value = expression - - -# def override_murfey_db(): -# try: -# db = mock_session -# yield db -# finally: -# db.close() - - -# app.dependency_overrides[murfey_db] = override_murfey_db +from tests.conftest import ExampleVisit, get_or_create_db_entry def test_movie_count( - mock_security_configuration, # From conftest.py murfey_db_session: Session, # From conftest.py ): - # Set up mock security configs for - with ( - patch( - "murfey.server.murfey_db.get_security_config" - ) as mock_get_security_config, - patch("murfey.server.murfey_db.url") as mock_get_url, - ): - - # Defer import of 'murfey_db' until after mocks are configured - mock_get_url.return_value = murfey_db_url - mock_get_security_config.return_value = security_from_file( - mock_security_configuration + # Insert table dependencies + dcg_entry: DataCollectionGroup = get_or_create_db_entry( + murfey_db_session, + DataCollectionGroup, + lookup_kwargs={ + "id": 0, + "session_id": ExampleVisit.murfey_session_id, + "tag": "test_dcg", + }, + ) + dc_entry: DataCollection = get_or_create_db_entry( + murfey_db_session, + DataCollection, + lookup_kwargs={ + "id": 0, + "tag": "test_dc", + "dcg_id": dcg_entry.id, + }, + ) + processing_job_entry: ProcessingJob = get_or_create_db_entry( + murfey_db_session, + ProcessingJob, + lookup_kwargs={ + "id": 0, + "recipe": "test_recipe", + "dc_id": dc_entry.id, + }, + ) + autoproc_entry: AutoProcProgram = get_or_create_db_entry( + murfey_db_session, + AutoProcProgram, + lookup_kwargs={ + "id": 0, + "pj_id": processing_job_entry.id, + }, + ) + + # Insert test movies and one-to-one dependencies into Murfey DB + tag = "test_movie" + num_movies = 5 + murfey_db_session + for i in range(num_movies): + murfey_ledger_entry: MurfeyLedger = get_or_create_db_entry( + murfey_db_session, + MurfeyLedger, + lookup_kwargs={ + "id": i, + "app_id": autoproc_entry.id, + }, + ) + _: Movie = get_or_create_db_entry( + murfey_db_session, + Movie, + lookup_kwargs={ + "murfey_id": murfey_ledger_entry.id, + "path": "/some/path", + "image_number": i, + "tag": tag, + }, ) - from murfey.server.murfey_db import murfey_db - - # Replace the murfey_db instance in endpoint with properly initialised pytest one - app.dependency_overrides[murfey_db] = murfey_db_session - # Disable instrument token validation - app.dependency_overrides[validate_instrument_token] = lambda: None - - with TestClient(app) as client: - - # Insert table dependencies - dcg_entry: DataCollectionGroup = get_or_create_db_entry( - murfey_db_session, - DataCollectionGroup, - lookup_kwargs={ - "id": 0, - "session_id": ExampleVisit.murfey_session_id, - "tag": "test_dcg", - }, - ) - dc_entry: DataCollection = get_or_create_db_entry( - murfey_db_session, - DataCollection, - lookup_kwargs={ - "id": 0, - "tag": "test_dc", - "dcg_id": dcg_entry.id, - }, - ) - processing_job_entry: ProcessingJob = get_or_create_db_entry( - murfey_db_session, - ProcessingJob, - lookup_kwargs={ - "id": 0, - "recipe": "test_recipe", - "dc_id": dc_entry.id, - }, - ) - autoproc_entry: AutoProcProgram = get_or_create_db_entry( - murfey_db_session, - AutoProcProgram, - lookup_kwargs={ - "id": 0, - "pj_id": processing_job_entry.id, - }, - ) - - # Insert test movies and one-to-one dependencies into Murfey DB - tag = "test_movie" - num_movies = 5 - murfey_db_session - for i in range(num_movies): - murfey_ledger_entry: MurfeyLedger = get_or_create_db_entry( - murfey_db_session, - MurfeyLedger, - lookup_kwargs={ - "id": i, - "app_id": autoproc_entry.id, - }, - ) - _: Movie = get_or_create_db_entry( - murfey_db_session, - Movie, - lookup_kwargs={ - "murfey_id": murfey_ledger_entry.id, - "path": "/some/path", - "image_number": i, - "tag": tag, - }, - ) - response = client.get( - f"{url_path_for('session_control.router', 'count_number_of_movies')}", - headers={"Authorization": f"Bearer {ANY}"}, - ) - assert response.json() == {tag: num_movies} + # Run function and evaluate result + result = count_number_of_movies(murfey_db_session) + assert result == {tag: num_movies} From 1cdae65bf82bf825e3515cb2b033b5037df471e7 Mon Sep 17 00:00:00 2001 From: Eu Pin Tien <eu-pin.tien@diamond.ac.uk> Date: Thu, 5 Jun 2025 15:47:55 +0100 Subject: [PATCH 26/26] Minor fixes to comments and redundant variables --- tests/server/api/test_movies.py | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/tests/server/api/test_movies.py b/tests/server/api/test_movies.py index 4cf3e2e77..00cc4f464 100644 --- a/tests/server/api/test_movies.py +++ b/tests/server/api/test_movies.py @@ -53,10 +53,9 @@ def test_movie_count( }, ) - # Insert test movies and one-to-one dependencies into Murfey DB + # Insert test movies and one-to-one dependencies tag = "test_movie" num_movies = 5 - murfey_db_session for i in range(num_movies): murfey_ledger_entry: MurfeyLedger = get_or_create_db_entry( murfey_db_session,