From 4e22b4217b0706f5c8758ad32395bdc655d36e5b Mon Sep 17 00:00:00 2001 From: Jacob Williamson Date: Tue, 11 Aug 2026 14:53:46 +0100 Subject: [PATCH 01/22] Add OIDC to config and simplify blueapi config --- src/daq_queuing_service/app/_config.py | 12 +++++++++--- .../blueapi_interaction/get_client.py | 5 ----- 2 files changed, 9 insertions(+), 8 deletions(-) diff --git a/src/daq_queuing_service/app/_config.py b/src/daq_queuing_service/app/_config.py index c3706ae..d09a105 100644 --- a/src/daq_queuing_service/app/_config.py +++ b/src/daq_queuing_service/app/_config.py @@ -2,8 +2,8 @@ from pathlib import Path import yaml -from blueapi.config import ApplicationConfig -from pydantic import BaseModel +from blueapi.config import OIDCConfig, RestConfig, StompConfig +from pydantic import BaseModel, Field CONFIG_PATH = "/etc/config/config.yaml" TEST_CONFIG_PATH = "tests/test_data/test_config.yaml" @@ -14,9 +14,15 @@ class ConverterConfig(BaseModel): name: str +class BlueapiConfig(BaseModel): + stomp: StompConfig = Field(default_factory=StompConfig) + api: RestConfig = Field(default_factory=RestConfig) + + class AppConfig(BaseModel): - blueapi: ApplicationConfig + blueapi: BlueapiConfig converter: ConverterConfig + oidc: OIDCConfig | None = None def get_default_config() -> str: diff --git a/src/daq_queuing_service/blueapi_interaction/get_client.py b/src/daq_queuing_service/blueapi_interaction/get_client.py index 79647c6..c9fbdfc 100644 --- a/src/daq_queuing_service/blueapi_interaction/get_client.py +++ b/src/daq_queuing_service/blueapi_interaction/get_client.py @@ -1,5 +1,3 @@ -from unittest.mock import MagicMock - from blueapi.client import BlueapiClient from blueapi.client.event_bus import EventBusClient from blueapi.client.rest import BlueapiRestClient @@ -10,9 +8,6 @@ def get_blueapi_client(blueapi_config: ApplicationConfig) -> BlueapiClient: - if not blueapi_config.oidc: - blueapi_config.oidc = MagicMock() - blueapi_rest_client = BlueapiRestClient( config=blueapi_config.api, # Waiting on https://github.com/DiamondLightSource/blueapi/pull/1553 From a40035c385c381e6ec14863a88b04bdeaa63878e Mon Sep 17 00:00:00 2001 From: Jacob Williamson Date: Tue, 11 Aug 2026 16:49:12 +0100 Subject: [PATCH 02/22] Add auth WIP --- src/daq_queuing_service/app/app.py | 20 ++++- src/daq_queuing_service/app/authentication.py | 86 +++++++++++++++++++ .../blueapi_interaction/get_client.py | 4 +- tests/test_data/i15_1/test_authn_config.yaml | 17 ++++ 4 files changed, 124 insertions(+), 3 deletions(-) create mode 100644 src/daq_queuing_service/app/authentication.py create mode 100644 tests/test_data/i15_1/test_authn_config.yaml diff --git a/src/daq_queuing_service/app/app.py b/src/daq_queuing_service/app/app.py index bd6d5bb..106e0a7 100644 --- a/src/daq_queuing_service/app/app.py +++ b/src/daq_queuing_service/app/app.py @@ -6,9 +6,15 @@ from fastapi import FastAPI from fastapi.middleware.cors import CORSMiddleware +from fastapi.param_functions import Depends +from fastapi.params import Depends as DependsType from daq_queuing_service.api.api import create_api_router from daq_queuing_service.api.errors import register_exception_handlers +from daq_queuing_service.app.authentication import ( + build_access_token_check, + build_current_user, +) from daq_queuing_service.blueapi_interaction.blueapi_adapter import BlueapiClientAdapter from daq_queuing_service.blueapi_interaction.get_client import get_blueapi_client from daq_queuing_service.broadcaster import Broadcaster @@ -54,6 +60,17 @@ def log_task_exception(task: asyncio.Task[NoReturn]): app = FastAPI(lifespan=lifespan) + dependencies: list[DependsType] = [] + if config.oidc: + validate_token = build_access_token_check(config.oidc) + current_user = build_current_user(validate_token) + + app.swagger_ui_init_oauth = { + "clientId": "NOT_SUPPORTED", + } + + dependencies.append(Depends(current_user)) + if dev: # Allows local client/UI through CORS app.add_middleware( CORSMiddleware, @@ -75,7 +92,8 @@ def log_task_exception(task: asyncio.Task[NoReturn]): register_exception_handlers(app) app.include_router( - create_api_router(app.state.queue, broadcaster, config, converter) + create_api_router(app.state.queue, broadcaster, config, converter), + dependencies=dependencies, ) return app diff --git a/src/daq_queuing_service/app/authentication.py b/src/daq_queuing_service/app/authentication.py new file mode 100644 index 0000000..772fdb4 --- /dev/null +++ b/src/daq_queuing_service/app/authentication.py @@ -0,0 +1,86 @@ +from collections.abc import Callable +from typing import Annotated, Any + +import jwt +from blueapi.config import OIDCConfig +from fastapi import Depends, HTTPException, Request +from fastapi.security import HTTPAuthorizationCredentials, HTTPBearer +from pydantic import BaseModel, ValidationError +from starlette.status import HTTP_401_UNAUTHORIZED + + +class User(BaseModel): + fedid: str + email: str | None = None + username: str | None = None + + +# Some of the following contents of this file were copied from blueapi +# See https://github.com/DiamondLightSource/blueapi/blob/2108ee0c89b4399d961106f7f23082a58d48a564/src/blueapi/service/authentication.py#L281-L340 + +bearer_scheme = HTTPBearer(auto_error=False) + + +def unchecked_bearer_token( + credentials: Annotated[HTTPAuthorizationCredentials | None, Depends(bearer_scheme)], +) -> str | None: + if credentials is None: + return None + return credentials.credentials + + +UncheckedBearerToken = Annotated[str | None, Depends(unchecked_bearer_token)] + + +def build_access_token_check( + config: OIDCConfig, +) -> Callable[[UncheckedBearerToken], dict[str, Any]]: + """ + Create a function to validate the bearer token of requests + + The returned function should be used via fastAPI's 'Depends' mechanism to + ensure users are authenticated + """ + jwkclient = jwt.PyJWKClient(config.jwks_uri) + + def validate_bearer_token(token: UncheckedBearerToken): + """Check that a bearer token is valid and inject into request state""" + if not token: + raise HTTPException( + status_code=HTTP_401_UNAUTHORIZED, + detail="Not authenticated", + headers={"WWW-Authenticate": "Bearer"}, + ) + + signing_key = jwkclient.get_signing_key_from_jwt(token) + decoded: dict[str, Any] = jwt.decode( + token, + signing_key.key, + algorithms=config.id_token_signing_alg_values_supported, + verify=True, + audience=config.client_audience, + issuer=config.issuer, + ) + return decoded + + return validate_bearer_token + + +def build_current_user( + validate_token: Callable[..., dict[str, Any]], +) -> Callable[[Request, dict[str, Any]], User]: + def current_user( + request: Request, + decoded: Annotated[dict[str, Any], Depends(validate_token)], + ) -> User: + try: + user = User.model_validate(decoded) + except ValidationError as e: + raise HTTPException( + status_code=HTTP_401_UNAUTHORIZED, + detail="Invalid token claims", + ) from e + request.state.user = user + return user + + return current_user diff --git a/src/daq_queuing_service/blueapi_interaction/get_client.py b/src/daq_queuing_service/blueapi_interaction/get_client.py index c9fbdfc..75f7191 100644 --- a/src/daq_queuing_service/blueapi_interaction/get_client.py +++ b/src/daq_queuing_service/blueapi_interaction/get_client.py @@ -1,13 +1,13 @@ from blueapi.client import BlueapiClient from blueapi.client.event_bus import EventBusClient from blueapi.client.rest import BlueapiRestClient -from blueapi.config import ApplicationConfig from bluesky_stomp.messaging import Broker, StompClient +from daq_queuing_service.app._config import BlueapiConfig from daq_queuing_service.blueapi_interaction.token_retriever import UDCTokenRetriever -def get_blueapi_client(blueapi_config: ApplicationConfig) -> BlueapiClient: +def get_blueapi_client(blueapi_config: BlueapiConfig) -> BlueapiClient: blueapi_rest_client = BlueapiRestClient( config=blueapi_config.api, # Waiting on https://github.com/DiamondLightSource/blueapi/pull/1553 diff --git a/tests/test_data/i15_1/test_authn_config.yaml b/tests/test_data/i15_1/test_authn_config.yaml new file mode 100644 index 0000000..ef37764 --- /dev/null +++ b/tests/test_data/i15_1/test_authn_config.yaml @@ -0,0 +1,17 @@ +converter: + path: "daq_queuing_service.plugins.converter" + name: "Converter" +blueapi: + api: + url: "http://localhost:8000" + stomp: + enabled: true # All other stomp settings will be ignored if this is false + url: tcp://localhost:61613 + auth: + username: guest + password: guest +oidc: + issuer: "https://identity.diamond.ac.uk/realms/dls" + client_id: "daq-queuing-service" + client_audience: "account" + logout_redirect_endpoint: "oauth2/sign_out" From 23d2056db46c8321167e521e48a641d4e663c4ee Mon Sep 17 00:00:00 2001 From: Jacob Williamson Date: Wed, 12 Aug 2026 13:52:03 +0100 Subject: [PATCH 03/22] Add whitelist of authorised fedIDs --- src/daq_queuing_service/api/api.py | 35 ++++++++++--------- src/daq_queuing_service/app/_config.py | 1 + src/daq_queuing_service/app/app.py | 18 +++++++--- src/daq_queuing_service/app/authentication.py | 17 ++++++--- src/daq_queuing_service/app/authorisation.py | 20 +++++++++++ .../{i15_1 => }/test_authn_config.yaml | 0 6 files changed, 66 insertions(+), 25 deletions(-) create mode 100644 src/daq_queuing_service/app/authorisation.py rename tests/test_data/{i15_1 => }/test_authn_config.yaml (100%) diff --git a/src/daq_queuing_service/api/api.py b/src/daq_queuing_service/api/api.py index 4320e7c..2bef8b3 100644 --- a/src/daq_queuing_service/api/api.py +++ b/src/daq_queuing_service/api/api.py @@ -1,13 +1,14 @@ import asyncio import json -from collections.abc import AsyncGenerator +from collections.abc import AsyncGenerator, Callable from blueapi.service.model import TaskRequest -from fastapi import APIRouter, Request, Response +from fastapi import APIRouter, Depends, Request, Response from fastapi.responses import EventSourceResponse from pydantic import BaseModel from daq_queuing_service.app._config import AppConfig +from daq_queuing_service.app.authentication import User from daq_queuing_service.blueapi_interaction.blueapi_call import BlueapiCallResponse from daq_queuing_service.broadcaster import Broadcaster from daq_queuing_service.plugins.converter import Converter, ValidateError @@ -44,7 +45,9 @@ def create_api_router( broadcaster: Broadcaster[QUEUE_EVENTS], config: AppConfig, converter: Converter, + whitelist_check: Callable[[User], User] | None = None, ) -> APIRouter: + authorised = [Depends(whitelist_check)] or None router = APIRouter() @router.get("/healthz") @@ -62,7 +65,7 @@ def read_root(request: Request): def get_config() -> AppConfig: return config - @router.patch("/queue/state") + @router.patch("/queue/state", dependencies=authorised) async def update_queue_state(payload: QueueStateUpdate) -> QueueState: if payload.paused: return await queue.pause_queue(PauseReason.USER_REQUESTED) @@ -73,11 +76,11 @@ async def update_queue_state(payload: QueueStateUpdate) -> QueueState: def get_queue_state() -> QueueState: return queue.state - @router.get("/queue") + @router.get("/queue", dependencies=authorised) async def get_queued_tasks(status: Status | None = None) -> list[TaskWithPosition]: return _filter_by_status(await queue.get_queue(), status) - @router.post("/queue") + @router.post("/queue", dependencies=authorised) async def add_tasks_to_queue( experiments: list[TaskRequest | Experiment], position: int | None = None, @@ -92,49 +95,49 @@ async def add_tasks_to_queue( await queue.add_tasks(tasks, position) return task_ids - @router.delete("/queue") + @router.delete("/queue", dependencies=authorised) async def cancel_all_tasks() -> list[TaskWithPosition]: return await queue.cancel_all_tasks() - @router.post("/queue/move") + @router.post("/queue/move", dependencies=authorised) async def move_task(task_id: str, new_position: int) -> int: return await queue.move_task(task_id, new_position) - @router.delete("/queue/tasks") + @router.delete("/queue/tasks", dependencies=authorised) async def cancel_tasks(payload: TaskCancelRequest) -> list[TaskWithPosition]: return await queue.cancel_tasks(payload.task_ids) - @router.get("/queue/{position}") + @router.get("/queue/{position}", dependencies=authorised) async def get_task_by_position(position: int) -> TaskWithPosition | None: return await queue.get_task_by_position(position) - @router.get("/tasks") + @router.get("/tasks", dependencies=authorised) async def get_all_tasks(status: Status | None = None) -> list[TaskWithPosition]: return _filter_by_status(await queue.get_tasks(), status) - @router.get("/tasks/{task_id}") + @router.get("/tasks/{task_id}", dependencies=authorised) async def get_task_by_id(task_id: str) -> TaskWithPosition: return await queue.get_task_by_id(task_id) - @router.get("/history") + @router.get("/history", dependencies=authorised) async def get_completed_tasks( status: Status | None = None, ) -> list[TaskWithPosition]: return _filter_by_status(await queue.get_history(), status) - @router.delete("/history") + @router.delete("/history", dependencies=authorised) async def clear_history(): return await queue.clear_history() - @router.get("/call_queue") + @router.get("/call_queue", dependencies=authorised) async def get_call_queue() -> list[BlueapiCallResponse]: return await queue.get_call_queue() - @router.get("/call_history") + @router.get("/call_history", dependencies=authorised) async def get_call_history() -> list[BlueapiCallResponse]: return await queue.get_call_history() - @router.get("/events") + @router.get("/events", dependencies=authorised) async def stream_events() -> EventSourceResponse: subscriber = broadcaster.subscribe() diff --git a/src/daq_queuing_service/app/_config.py b/src/daq_queuing_service/app/_config.py index d09a105..6fb7342 100644 --- a/src/daq_queuing_service/app/_config.py +++ b/src/daq_queuing_service/app/_config.py @@ -23,6 +23,7 @@ class AppConfig(BaseModel): blueapi: BlueapiConfig converter: ConverterConfig oidc: OIDCConfig | None = None + authorisation_whitelist: list[str] | None = None def get_default_config() -> str: diff --git a/src/daq_queuing_service/app/app.py b/src/daq_queuing_service/app/app.py index 106e0a7..53a92a9 100644 --- a/src/daq_queuing_service/app/app.py +++ b/src/daq_queuing_service/app/app.py @@ -13,7 +13,10 @@ from daq_queuing_service.api.errors import register_exception_handlers from daq_queuing_service.app.authentication import ( build_access_token_check, - build_current_user, + build_get_current_user, +) +from daq_queuing_service.app.authorisation import ( + build_ensure_current_user_is_in_whitelist, ) from daq_queuing_service.blueapi_interaction.blueapi_adapter import BlueapiClientAdapter from daq_queuing_service.blueapi_interaction.get_client import get_blueapi_client @@ -61,15 +64,20 @@ def log_task_exception(task: asyncio.Task[NoReturn]): app = FastAPI(lifespan=lifespan) dependencies: list[DependsType] = [] + whitelist_check = None if config.oidc: validate_token = build_access_token_check(config.oidc) - current_user = build_current_user(validate_token) + get_current_user = build_get_current_user(validate_token) app.swagger_ui_init_oauth = { "clientId": "NOT_SUPPORTED", } - dependencies.append(Depends(current_user)) + dependencies.append(Depends(get_current_user)) + + whitelist_check = build_ensure_current_user_is_in_whitelist( + config.authorisation_whitelist, get_current_user + ) if dev: # Allows local client/UI through CORS app.add_middleware( @@ -92,7 +100,9 @@ def log_task_exception(task: asyncio.Task[NoReturn]): register_exception_handlers(app) app.include_router( - create_api_router(app.state.queue, broadcaster, config, converter), + create_api_router( + app.state.queue, broadcaster, config, converter, whitelist_check + ), dependencies=dependencies, ) diff --git a/src/daq_queuing_service/app/authentication.py b/src/daq_queuing_service/app/authentication.py index 772fdb4..1adc1d6 100644 --- a/src/daq_queuing_service/app/authentication.py +++ b/src/daq_queuing_service/app/authentication.py @@ -5,6 +5,7 @@ from blueapi.config import OIDCConfig from fastapi import Depends, HTTPException, Request from fastapi.security import HTTPAuthorizationCredentials, HTTPBearer +from jwt.exceptions import DecodeError from pydantic import BaseModel, ValidationError from starlette.status import HTTP_401_UNAUTHORIZED @@ -15,7 +16,7 @@ class User(BaseModel): username: str | None = None -# Some of the following contents of this file were copied from blueapi +# Some of the following code was copied from blueapi # See https://github.com/DiamondLightSource/blueapi/blob/2108ee0c89b4399d961106f7f23082a58d48a564/src/blueapi/service/authentication.py#L281-L340 bearer_scheme = HTTPBearer(auto_error=False) @@ -52,7 +53,13 @@ def validate_bearer_token(token: UncheckedBearerToken): headers={"WWW-Authenticate": "Bearer"}, ) - signing_key = jwkclient.get_signing_key_from_jwt(token) + try: + signing_key = jwkclient.get_signing_key_from_jwt(token) + except DecodeError as e: + raise HTTPException( + status_code=HTTP_401_UNAUTHORIZED, + detail="Cannot decode token", + ) from e decoded: dict[str, Any] = jwt.decode( token, signing_key.key, @@ -66,10 +73,10 @@ def validate_bearer_token(token: UncheckedBearerToken): return validate_bearer_token -def build_current_user( +def build_get_current_user( validate_token: Callable[..., dict[str, Any]], ) -> Callable[[Request, dict[str, Any]], User]: - def current_user( + def get_current_user( request: Request, decoded: Annotated[dict[str, Any], Depends(validate_token)], ) -> User: @@ -83,4 +90,4 @@ def current_user( request.state.user = user return user - return current_user + return get_current_user diff --git a/src/daq_queuing_service/app/authorisation.py b/src/daq_queuing_service/app/authorisation.py new file mode 100644 index 0000000..9a8a643 --- /dev/null +++ b/src/daq_queuing_service/app/authorisation.py @@ -0,0 +1,20 @@ +from collections.abc import Callable +from typing import Annotated + +from fastapi import Depends, HTTPException +from starlette.status import HTTP_403_FORBIDDEN + +from daq_queuing_service.app.authentication import User + + +def build_ensure_current_user_is_in_whitelist( + whitelist: list[str] | None, get_current_user: Callable[..., User] +) -> Callable[[User], User]: + def ensure_current_user_is_in_whitelist( + current_user: Annotated[User, Depends(get_current_user)], + ) -> User: + if whitelist is None or current_user.fedid in whitelist: + return current_user + raise HTTPException(status_code=HTTP_403_FORBIDDEN, detail="Not authorised") + + return ensure_current_user_is_in_whitelist diff --git a/tests/test_data/i15_1/test_authn_config.yaml b/tests/test_data/test_authn_config.yaml similarity index 100% rename from tests/test_data/i15_1/test_authn_config.yaml rename to tests/test_data/test_authn_config.yaml From c936949e991e33fa05e20d2d21c384cfe7238e7c Mon Sep 17 00:00:00 2001 From: Jacob Williamson Date: Wed, 12 Aug 2026 14:31:41 +0100 Subject: [PATCH 04/22] Fix bug --- src/daq_queuing_service/api/api.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/daq_queuing_service/api/api.py b/src/daq_queuing_service/api/api.py index 2bef8b3..cd12b4a 100644 --- a/src/daq_queuing_service/api/api.py +++ b/src/daq_queuing_service/api/api.py @@ -47,7 +47,7 @@ def create_api_router( converter: Converter, whitelist_check: Callable[[User], User] | None = None, ) -> APIRouter: - authorised = [Depends(whitelist_check)] or None + authorised = [Depends(whitelist_check)] if whitelist_check else None router = APIRouter() @router.get("/healthz") From 74b2ab0747945d74fc1da5d85a2a5bb6b9842916 Mon Sep 17 00:00:00 2001 From: Jacob Williamson Date: Wed, 12 Aug 2026 14:34:46 +0100 Subject: [PATCH 05/22] Fix lint --- tests/unit_tests/test_get_blueapi_client.py | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/tests/unit_tests/test_get_blueapi_client.py b/tests/unit_tests/test_get_blueapi_client.py index 0220a62..9605cff 100644 --- a/tests/unit_tests/test_get_blueapi_client.py +++ b/tests/unit_tests/test_get_blueapi_client.py @@ -1,8 +1,9 @@ from unittest.mock import MagicMock, patch -from blueapi.config import ApplicationConfig, RestConfig, StompConfig +from blueapi.config import RestConfig, StompConfig from pydantic import HttpUrl +from daq_queuing_service.app._config import BlueapiConfig from daq_queuing_service.blueapi_interaction.get_client import get_blueapi_client @@ -15,7 +16,7 @@ def test_get_blueapi_clients_constructs_clients_with_expected_args_and_returns_c mock_token_retriever: MagicMock, ): rest_config = RestConfig(url=HttpUrl("http://test_url.com")) - blueapi_client = get_blueapi_client(ApplicationConfig(api=rest_config)) + blueapi_client = get_blueapi_client(BlueapiConfig(api=rest_config)) mock_rest_client.assert_called_once_with( config=rest_config, session_manager=mock_token_retriever.return_value @@ -35,7 +36,7 @@ def test_get_blueapi_clients_constructs_blueapi_client_with_stomp_if_enabled_in_ ): rest_config = RestConfig(url=HttpUrl("http://test_url.com")) _ = get_blueapi_client( - ApplicationConfig(api=rest_config, stomp=StompConfig(enabled=True)) + BlueapiConfig(api=rest_config, stomp=StompConfig(enabled=True)) ) mock_blueapi_client.assert_called_once_with( From cb1919fd4d138034fc607eb167235c171bad4cd6 Mon Sep 17 00:00:00 2001 From: Jacob Williamson Date: Wed, 12 Aug 2026 15:05:37 +0100 Subject: [PATCH 06/22] Don't require authentication for healthz endpoint --- src/daq_queuing_service/api/api.py | 16 ++++++++++----- src/daq_queuing_service/app/app.py | 5 +++-- tests/unit_tests/test_api.py | 6 ++++-- tests/unit_tests/test_app.py | 29 +++++++++++++++------------- utility_scripts/generate_api_docs.py | 7 +++---- 5 files changed, 37 insertions(+), 26 deletions(-) diff --git a/src/daq_queuing_service/api/api.py b/src/daq_queuing_service/api/api.py index cd12b4a..ec46375 100644 --- a/src/daq_queuing_service/api/api.py +++ b/src/daq_queuing_service/api/api.py @@ -40,7 +40,17 @@ def _filter_by_status( return [task for task in tasks if task.status == status] -def create_api_router( +def public_routes() -> APIRouter: + router = APIRouter() + + @router.get("/healthz") + async def healthz(): + return Response() + + return router + + +def protected_routes( queue: TaskQueue, broadcaster: Broadcaster[QUEUE_EVENTS], config: AppConfig, @@ -50,10 +60,6 @@ def create_api_router( authorised = [Depends(whitelist_check)] if whitelist_check else None router = APIRouter() - @router.get("/healthz") - async def healthz(): - return Response() - @router.get("/") def read_root(request: Request): base_url = str(request.base_url) diff --git a/src/daq_queuing_service/app/app.py b/src/daq_queuing_service/app/app.py index 53a92a9..f94a022 100644 --- a/src/daq_queuing_service/app/app.py +++ b/src/daq_queuing_service/app/app.py @@ -9,7 +9,7 @@ from fastapi.param_functions import Depends from fastapi.params import Depends as DependsType -from daq_queuing_service.api.api import create_api_router +from daq_queuing_service.api.api import protected_routes, public_routes from daq_queuing_service.api.errors import register_exception_handlers from daq_queuing_service.app.authentication import ( build_access_token_check, @@ -99,8 +99,9 @@ def log_task_exception(task: asyncio.Task[NoReturn]): ) register_exception_handlers(app) + app.include_router(public_routes()) app.include_router( - create_api_router( + protected_routes( app.state.queue, broadcaster, config, converter, whitelist_check ), dependencies=dependencies, diff --git a/tests/unit_tests/test_api.py b/tests/unit_tests/test_api.py index 5882c7a..d0b931b 100644 --- a/tests/unit_tests/test_api.py +++ b/tests/unit_tests/test_api.py @@ -19,7 +19,8 @@ from daq_queuing_service.api.api import ( TaskCancelRequest, - create_api_router, + protected_routes, + public_routes, ) from daq_queuing_service.api.errors import register_exception_handlers from daq_queuing_service.app._config import TEST_CONFIG_PATH, load_config @@ -67,8 +68,9 @@ def app( ) -> FastAPI: app = FastAPI() register_exception_handlers(app) + app.include_router(public_routes()) app.include_router( - create_api_router( + protected_routes( task_queue_with_history, broadcaster, load_config(Path(TEST_CONFIG_PATH)), diff --git a/tests/unit_tests/test_app.py b/tests/unit_tests/test_app.py index 28c96a5..d950c27 100644 --- a/tests/unit_tests/test_app.py +++ b/tests/unit_tests/test_app.py @@ -2,7 +2,7 @@ import logging from pathlib import Path from typing import NoReturn -from unittest.mock import AsyncMock, patch +from unittest.mock import AsyncMock, MagicMock, patch import pytest from fastapi import FastAPI @@ -39,13 +39,15 @@ def test_create_app_registers_exception_handlers(): mock_register_exception_handlers.assert_called_once() -def test_create_app_adds_router(): - with patch( - "daq_queuing_service.app.app.create_api_router" - ) as mock_create_api_router: - create_app(Path(TEST_CONFIG_PATH)) +@patch("daq_queuing_service.app.app.public_routes") +@patch("daq_queuing_service.app.app.protected_routes") +def test_create_app_adds_router( + mock_public_routes: MagicMock, mock_protected_routes: MagicMock +): + create_app(Path(TEST_CONFIG_PATH)) - mock_create_api_router.assert_called_once() + mock_public_routes.assert_called_once() + mock_protected_routes.assert_called_once() def test_lifespan_runs_without_error(): @@ -68,11 +70,12 @@ def test_worker_task_cancelled_on_shutdown(): assert worker_task.cancelled() -def test_queue_and_worker_added_to_app_state_and_queue_object_shared_across_app(): - with patch( - "daq_queuing_service.app.app.create_api_router" - ) as mock_create_api_router: - app = create_app(Path(TEST_CONFIG_PATH)) +@patch("daq_queuing_service.app.app.protected_routes") +def test_queue_and_worker_added_to_app_state_and_queue_object_shared_across_app( + mock_protected_routes: MagicMock, +): + + app = create_app(Path(TEST_CONFIG_PATH)) app_queue = app.state.queue app_worker = app.state.worker @@ -80,7 +83,7 @@ def test_queue_and_worker_added_to_app_state_and_queue_object_shared_across_app( assert isinstance(app_queue, TaskQueue) assert isinstance(app_worker, QueueWorker) assert app_worker._queue is app_queue - assert mock_create_api_router.call_args_list[0].args[0] is app_queue + assert mock_protected_routes.call_args_list[0].args[0] is app_queue @patch( diff --git a/utility_scripts/generate_api_docs.py b/utility_scripts/generate_api_docs.py index 7f06017..6934f6f 100644 --- a/utility_scripts/generate_api_docs.py +++ b/utility_scripts/generate_api_docs.py @@ -4,12 +4,11 @@ from fastapi import FastAPI from fastapi.openapi.utils import get_openapi -from daq_queuing_service.api.api import create_api_router +from daq_queuing_service.api.api import protected_routes, public_routes app = FastAPI() -app.include_router( - create_api_router(MagicMock(), MagicMock(), MagicMock(), MagicMock()) -) +app.include_router(public_routes()) +app.include_router(protected_routes(MagicMock(), MagicMock(), MagicMock(), MagicMock())) openapi = get_openapi( title=app.title, From b46a33f5b4762a33e634339fd43f36dfba0b16b0 Mon Sep 17 00:00:00 2001 From: Jacob Williamson Date: Wed, 12 Aug 2026 15:25:57 +0100 Subject: [PATCH 07/22] Add some logging --- src/daq_queuing_service/app/authentication.py | 3 +++ src/daq_queuing_service/app/authorisation.py | 10 +++++++++- tests/unit_tests/test_app.py | 2 +- 3 files changed, 13 insertions(+), 2 deletions(-) diff --git a/src/daq_queuing_service/app/authentication.py b/src/daq_queuing_service/app/authentication.py index 1adc1d6..fcd2ed8 100644 --- a/src/daq_queuing_service/app/authentication.py +++ b/src/daq_queuing_service/app/authentication.py @@ -9,6 +9,8 @@ from pydantic import BaseModel, ValidationError from starlette.status import HTTP_401_UNAUTHORIZED +from daq_queuing_service.worker.worker import LOGGER + class User(BaseModel): fedid: str @@ -68,6 +70,7 @@ def validate_bearer_token(token: UncheckedBearerToken): audience=config.client_audience, issuer=config.issuer, ) + LOGGER.debug(f"Decoded valid token: {decoded}") return decoded return validate_bearer_token diff --git a/src/daq_queuing_service/app/authorisation.py b/src/daq_queuing_service/app/authorisation.py index 9a8a643..1841e56 100644 --- a/src/daq_queuing_service/app/authorisation.py +++ b/src/daq_queuing_service/app/authorisation.py @@ -5,6 +5,7 @@ from starlette.status import HTTP_403_FORBIDDEN from daq_queuing_service.app.authentication import User +from daq_queuing_service.worker.worker import LOGGER def build_ensure_current_user_is_in_whitelist( @@ -13,7 +14,14 @@ def build_ensure_current_user_is_in_whitelist( def ensure_current_user_is_in_whitelist( current_user: Annotated[User, Depends(get_current_user)], ) -> User: - if whitelist is None or current_user.fedid in whitelist: + LOGGER.debug(f"Got user: {current_user}") + if whitelist is None: + LOGGER.debug("No user whitelist. All authenticated users are authorised.") + return current_user + elif current_user.fedid in whitelist: + LOGGER.debug( + f"FedID {current_user.fedid} found in whitelist, user authorised." + ) return current_user raise HTTPException(status_code=HTTP_403_FORBIDDEN, detail="Not authorised") diff --git a/tests/unit_tests/test_app.py b/tests/unit_tests/test_app.py index d950c27..7a21b6d 100644 --- a/tests/unit_tests/test_app.py +++ b/tests/unit_tests/test_app.py @@ -41,7 +41,7 @@ def test_create_app_registers_exception_handlers(): @patch("daq_queuing_service.app.app.public_routes") @patch("daq_queuing_service.app.app.protected_routes") -def test_create_app_adds_router( +def test_create_app_adds_routers( mock_public_routes: MagicMock, mock_protected_routes: MagicMock ): create_app(Path(TEST_CONFIG_PATH)) From 3df5b91b6cf1660842d12269ec4055dd483e81de Mon Sep 17 00:00:00 2001 From: Jacob Williamson Date: Wed, 12 Aug 2026 16:00:02 +0100 Subject: [PATCH 08/22] More logging --- src/daq_queuing_service/app/authentication.py | 1 + 1 file changed, 1 insertion(+) diff --git a/src/daq_queuing_service/app/authentication.py b/src/daq_queuing_service/app/authentication.py index fcd2ed8..3ba8eed 100644 --- a/src/daq_queuing_service/app/authentication.py +++ b/src/daq_queuing_service/app/authentication.py @@ -44,6 +44,7 @@ def build_access_token_check( The returned function should be used via fastAPI's 'Depends' mechanism to ensure users are authenticated """ + LOGGER.info(f"JWKS URI: {config.jwks_uri}") jwkclient = jwt.PyJWKClient(config.jwks_uri) def validate_bearer_token(token: UncheckedBearerToken): From 3b721ab848a23c1cfdc52fa9506ceea879c84c9b Mon Sep 17 00:00:00 2001 From: Jacob Williamson Date: Thu, 13 Aug 2026 11:32:02 +0100 Subject: [PATCH 09/22] Use resolute as base imahe --- Dockerfile | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Dockerfile b/Dockerfile index a51162f..b63d633 100644 --- a/Dockerfile +++ b/Dockerfile @@ -1,6 +1,6 @@ # The devcontainer should use the developer target and run as root with podman # or docker with user namespaces. -FROM ghcr.io/diamondlightsource/ubuntu-devcontainer:noble AS developer +FROM ghcr.io/diamondlightsource/ubuntu-devcontainer:resolute AS developer # Add any system dependencies for the developer/build environment here RUN apt-get update -y && apt-get install -y --no-install-recommends \ From 3586b8c1f21b3288cf24571a976ba7c4ef65bec8 Mon Sep 17 00:00:00 2001 From: Jacob Williamson Date: Thu, 13 Aug 2026 13:34:04 +0100 Subject: [PATCH 10/22] Install ca-certificates in container --- Dockerfile | 5 +++++ src/daq_queuing_service/app/authentication.py | 8 +++++++- 2 files changed, 12 insertions(+), 1 deletion(-) diff --git a/Dockerfile b/Dockerfile index b63d633..1d64146 100644 --- a/Dockerfile +++ b/Dockerfile @@ -54,6 +54,11 @@ FROM ubuntu:resolute AS runtime # some-library \ # && apt-get dist-clean +RUN apt-get update && \ + apt-get install -y --no-install-recommends ca-certificates && \ + update-ca-certificates && \ + apt-get dist-clean + # Copy the python installation from the build stage COPY --from=build /python /python diff --git a/src/daq_queuing_service/app/authentication.py b/src/daq_queuing_service/app/authentication.py index 3ba8eed..9539de8 100644 --- a/src/daq_queuing_service/app/authentication.py +++ b/src/daq_queuing_service/app/authentication.py @@ -5,7 +5,7 @@ from blueapi.config import OIDCConfig from fastapi import Depends, HTTPException, Request from fastapi.security import HTTPAuthorizationCredentials, HTTPBearer -from jwt.exceptions import DecodeError +from jwt.exceptions import DecodeError, ExpiredSignatureError from pydantic import BaseModel, ValidationError from starlette.status import HTTP_401_UNAUTHORIZED @@ -63,6 +63,12 @@ def validate_bearer_token(token: UncheckedBearerToken): status_code=HTTP_401_UNAUTHORIZED, detail="Cannot decode token", ) from e + except ExpiredSignatureError as e: + raise HTTPException( + status_code=HTTP_401_UNAUTHORIZED, + detail="Token expired", + ) from e + decoded: dict[str, Any] = jwt.decode( token, signing_key.key, From 9c89ab787ea076848f8527926d819cedc27bc9aa Mon Sep 17 00:00:00 2001 From: Jacob Williamson Date: Thu, 13 Aug 2026 13:46:42 +0100 Subject: [PATCH 11/22] Improve error detail --- src/daq_queuing_service/app/authorisation.py | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/src/daq_queuing_service/app/authorisation.py b/src/daq_queuing_service/app/authorisation.py index 1841e56..7bd5da0 100644 --- a/src/daq_queuing_service/app/authorisation.py +++ b/src/daq_queuing_service/app/authorisation.py @@ -23,6 +23,9 @@ def ensure_current_user_is_in_whitelist( f"FedID {current_user.fedid} found in whitelist, user authorised." ) return current_user - raise HTTPException(status_code=HTTP_403_FORBIDDEN, detail="Not authorised") + raise HTTPException( + status_code=HTTP_403_FORBIDDEN, + detail="Not authorised. You are not in the whitelist of authorised FedIDs", + ) return ensure_current_user_is_in_whitelist From ad4a9433bb31b7f8b92f0ee8d8bfe21927db2205 Mon Sep 17 00:00:00 2001 From: Jacob Williamson Date: Fri, 14 Aug 2026 09:58:31 +0100 Subject: [PATCH 12/22] Fixes and tests --- src/daq_queuing_service/__main__.py | 4 +-- src/daq_queuing_service/api/api.py | 24 ++++++------- src/daq_queuing_service/app/_config.py | 2 +- src/daq_queuing_service/app/app.py | 2 +- tests/constants.py | 2 ++ ...onfig.yaml => test_config_with_authn.yaml} | 0 tests/unit_tests/conftest.py | 8 +++++ tests/unit_tests/test_api.py | 3 +- tests/unit_tests/test_app.py | 32 +++++++++++------ tests/unit_tests/test_authentication.py | 0 tests/unit_tests/test_authorisation.py | 35 +++++++++++++++++++ 11 files changed, 83 insertions(+), 29 deletions(-) create mode 100644 tests/constants.py rename tests/test_data/{test_authn_config.yaml => test_config_with_authn.yaml} (100%) create mode 100644 tests/unit_tests/test_authentication.py create mode 100644 tests/unit_tests/test_authorisation.py diff --git a/src/daq_queuing_service/__main__.py b/src/daq_queuing_service/__main__.py index 7a09e3a..035b213 100644 --- a/src/daq_queuing_service/__main__.py +++ b/src/daq_queuing_service/__main__.py @@ -6,7 +6,7 @@ import uvicorn -from daq_queuing_service.app._config import get_default_config +from daq_queuing_service.app._config import get_default_config_path from . import __version__ @@ -19,7 +19,7 @@ def main(args: Sequence[str] | None = None) -> None: parser.add_argument("-v", "--version", action="version", version=__version__) parser.add_argument("-p", "--port", type=int, default=8000) parser.add_argument("--dev", action="store_true", default=False) - parser.add_argument("--config", type=Path, default=get_default_config()) + parser.add_argument("--config", type=Path, default=get_default_config_path()) parsed_args = parser.parse_args(args) diff --git a/src/daq_queuing_service/api/api.py b/src/daq_queuing_service/api/api.py index ec46375..c8108ca 100644 --- a/src/daq_queuing_service/api/api.py +++ b/src/daq_queuing_service/api/api.py @@ -40,13 +40,24 @@ def _filter_by_status( return [task for task in tasks if task.status == status] -def public_routes() -> APIRouter: +def public_routes(queue: TaskQueue) -> APIRouter: router = APIRouter() + @router.get("/") + def read_root(request: Request): + base_url = str(request.base_url) + return ( + f"Welcome to the daq queuing service. Visit {base_url}docs for Uvicorn API." + ) + @router.get("/healthz") async def healthz(): return Response() + @router.get("/queue/state") + def get_queue_state() -> QueueState: + return queue.state + return router @@ -60,13 +71,6 @@ def protected_routes( authorised = [Depends(whitelist_check)] if whitelist_check else None router = APIRouter() - @router.get("/") - def read_root(request: Request): - base_url = str(request.base_url) - return ( - f"Welcome to the daq queuing service. Visit {base_url}docs for Uvicorn API." - ) - @router.get("/config") def get_config() -> AppConfig: return config @@ -78,10 +82,6 @@ async def update_queue_state(payload: QueueStateUpdate) -> QueueState: else: return await queue.resume_queue() - @router.get("/queue/state") - def get_queue_state() -> QueueState: - return queue.state - @router.get("/queue", dependencies=authorised) async def get_queued_tasks(status: Status | None = None) -> list[TaskWithPosition]: return _filter_by_status(await queue.get_queue(), status) diff --git a/src/daq_queuing_service/app/_config.py b/src/daq_queuing_service/app/_config.py index 6fb7342..79167d6 100644 --- a/src/daq_queuing_service/app/_config.py +++ b/src/daq_queuing_service/app/_config.py @@ -26,7 +26,7 @@ class AppConfig(BaseModel): authorisation_whitelist: list[str] | None = None -def get_default_config() -> str: +def get_default_config_path() -> str: return CONFIG_PATH if os.path.isfile(CONFIG_PATH) else TEST_CONFIG_PATH diff --git a/src/daq_queuing_service/app/app.py b/src/daq_queuing_service/app/app.py index f94a022..1f6250f 100644 --- a/src/daq_queuing_service/app/app.py +++ b/src/daq_queuing_service/app/app.py @@ -99,7 +99,7 @@ def log_task_exception(task: asyncio.Task[NoReturn]): ) register_exception_handlers(app) - app.include_router(public_routes()) + app.include_router(public_routes(app.state.queue)) app.include_router( protected_routes( app.state.queue, broadcaster, config, converter, whitelist_check diff --git a/tests/constants.py b/tests/constants.py new file mode 100644 index 0000000..b8e4585 --- /dev/null +++ b/tests/constants.py @@ -0,0 +1,2 @@ +TEST_CONFIG_PATH = "tests/test_data/test_config.yaml" +TEST_CONFIG_WITH_AUTHN_PATH = "tests/test_data/test_config_with_authn.yaml" diff --git a/tests/test_data/test_authn_config.yaml b/tests/test_data/test_config_with_authn.yaml similarity index 100% rename from tests/test_data/test_authn_config.yaml rename to tests/test_data/test_config_with_authn.yaml diff --git a/tests/unit_tests/conftest.py b/tests/unit_tests/conftest.py index 77f2989..20ae5f7 100644 --- a/tests/unit_tests/conftest.py +++ b/tests/unit_tests/conftest.py @@ -1,6 +1,7 @@ import pytest from blueapi.service.model import TaskRequest from blueapi.worker.event import TaskError, TaskResult +from fastapi.dependencies.models import Dependant from pytest import MonkeyPatch from daq_queuing_service.blueapi_interaction.blueapi_call import BlueapiCall @@ -154,3 +155,10 @@ def _construct_blueapi_task_request( ) return DNConverter() + + +def has_dependency_name(dep: Dependant, name: str): + if getattr(dep.call, "__name__", None) == name: + return True + + return any(has_dependency_name(child, name) for child in dep.dependencies) diff --git a/tests/unit_tests/test_api.py b/tests/unit_tests/test_api.py index d0b931b..4ddf08d 100644 --- a/tests/unit_tests/test_api.py +++ b/tests/unit_tests/test_api.py @@ -62,13 +62,12 @@ def broadcaster() -> Broadcaster[QUEUE_EVENTS]: @pytest.fixture def app( task_queue_with_history: TaskQueue, - blueapi_client: BlueapiRestClient, broadcaster: Broadcaster[QUEUE_EVENTS], converter: Converter, ) -> FastAPI: app = FastAPI() register_exception_handlers(app) - app.include_router(public_routes()) + app.include_router(public_routes(task_queue_with_history)) app.include_router( protected_routes( task_queue_with_history, diff --git a/tests/unit_tests/test_app.py b/tests/unit_tests/test_app.py index 7a21b6d..d51012e 100644 --- a/tests/unit_tests/test_app.py +++ b/tests/unit_tests/test_app.py @@ -4,25 +4,17 @@ from typing import NoReturn from unittest.mock import AsyncMock, MagicMock, patch -import pytest from fastapi import FastAPI from fastapi.middleware.cors import CORSMiddleware +from fastapi.routing import APIRoute from fastapi.testclient import TestClient from pytest import LogCaptureFixture -from daq_queuing_service.app._config import TEST_CONFIG_PATH +from constants import TEST_CONFIG_PATH, TEST_CONFIG_WITH_AUTHN_PATH from daq_queuing_service.app.app import create_app from daq_queuing_service.task_queue.queue import TaskQueue from daq_queuing_service.worker.worker import QueueWorker - - -@pytest.fixture(autouse=True) -def patch_config_path(): - with patch( - "daq_queuing_service.app._config.CONFIG_PATH", - "tests/test_data/test_config.yaml", - ): - yield +from unit_tests.conftest import has_dependency_name def test_create_app_returns_fast_api_object(): @@ -113,3 +105,21 @@ def test_if_dev_mode_cors_middlewhere_added_to_app(): allow_methods=["*"], allow_headers=["*"], ) + + +def test_create_app_adds_auth_dependencies_to_correct_routes(): + no_auth_required = ["read_root", "healthz", "get_queue_state"] + app = create_app(Path(TEST_CONFIG_WITH_AUTHN_PATH)) + + for route in app.routes: + if isinstance(route, APIRoute): + if route.name not in no_auth_required: + assert has_dependency_name(route.dependant, "validate_bearer_token"), ( + f"No access token check dependency for route {str(route)}" + ) + assert has_dependency_name(route.dependant, "get_current_user"), ( + f"No get user dependency for route {str(route)}" + ) + else: + assert not has_dependency_name(route.dependant, "validate_bearer_token") + assert not has_dependency_name(route.dependant, "get_current_user") diff --git a/tests/unit_tests/test_authentication.py b/tests/unit_tests/test_authentication.py new file mode 100644 index 0000000..e69de29 diff --git a/tests/unit_tests/test_authorisation.py b/tests/unit_tests/test_authorisation.py new file mode 100644 index 0000000..ab6b3d7 --- /dev/null +++ b/tests/unit_tests/test_authorisation.py @@ -0,0 +1,35 @@ +from unittest.mock import MagicMock + +import pytest +from fastapi import HTTPException + +from daq_queuing_service.app.authentication import User +from daq_queuing_service.app.authorisation import ( + build_ensure_current_user_is_in_whitelist, +) + + +def test_ensure_current_user_is_in_whitelist_returns_user_if_user_in_whitelist(): + user = User(fedid="abc12345") + + whitelist_check = build_ensure_current_user_is_in_whitelist( + ["abc12345", "def67890"], MagicMock() + ) + assert whitelist_check(user) == user + + +def test_ensure_current_user_is_in_whitelist_raises_error_if_user_not_in_whitelist(): + user = User(fedid="abc12345") + + whitelist_check = build_ensure_current_user_is_in_whitelist( + ["def67890"], MagicMock() + ) + with pytest.raises(HTTPException): + whitelist_check(user) + + +def test_ensure_current_user_is_in_whitelist_returns_user_no_whitelist_provided(): + user = User(fedid="abc12345") + + whitelist_check = build_ensure_current_user_is_in_whitelist(None, MagicMock()) + assert whitelist_check(user) == user From 70797d598b233663193323a2c3d6d4b9c6540b3b Mon Sep 17 00:00:00 2001 From: Jacob Williamson Date: Fri, 14 Aug 2026 10:05:10 +0100 Subject: [PATCH 13/22] Pin blueapi --- pyproject.toml | 3 ++- uv.lock | 10 +++------- 2 files changed, 5 insertions(+), 8 deletions(-) diff --git a/pyproject.toml b/pyproject.toml index a6a9659..5414a2f 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -13,7 +13,8 @@ classifiers = [ ] description = "A service to queue tasks and chain BlueAPI calls" dependencies = [ - "blueapi>=1.18.0", + # Pin needed until https://github.com/DiamondLightSource/blueapi/pull/1625 in a release + "blueapi @ git+https://github.com/DiamondLightSource/blueapi.git@308f55f43f0623d36101d124ab9e85b9e7532568", "fastapi>=0.136.0", "pydantic>=2.13.2", ] diff --git a/uv.lock b/uv.lock index 04d7067..1ec2ac1 100644 --- a/uv.lock +++ b/uv.lock @@ -354,8 +354,8 @@ wheels = [ [[package]] name = "blueapi" -version = "1.18.0" -source = { registry = "https://pypi.org/simple" } +version = "1.18.1.dev3+g308f55f43" +source = { git = "https://github.com/DiamondLightSource/blueapi.git?rev=308f55f43f0623d36101d124ab9e85b9e7532568#308f55f43f0623d36101d124ab9e85b9e7532568" } dependencies = [ { name = "aioca" }, { name = "aiohttp" }, @@ -381,10 +381,6 @@ dependencies = [ { name = "tomlkit" }, { name = "uvicorn" }, ] -sdist = { url = "https://files.pythonhosted.org/packages/fb/6c/826f1f3818cf6c28b2ef0cc50769245ff32959e21f9b2323613f0c2d8f5f/blueapi-1.18.0.tar.gz", hash = "sha256:1ff7933b9ee2a2620c7817c6435223493a41317fe203277d1f3368063cca01fa", size = 1881191, upload-time = "2026-08-07T09:48:18.142Z" } -wheels = [ - { url = "https://files.pythonhosted.org/packages/19/77/20f7d841722cf820dd863f2a8ec4837bece0e4fcc07804be2f1ee978f308/blueapi-1.18.0-py3-none-any.whl", hash = "sha256:5037f6f27427945577e7a2ba64e10e844b244e1712313d87985b54bc9dba8b43", size = 87982, upload-time = "2026-08-07T09:48:16.716Z" }, -] [[package]] name = "bluesky" @@ -980,7 +976,7 @@ dev = [ [package.metadata] requires-dist = [ - { name = "blueapi", specifier = ">=1.18.0" }, + { name = "blueapi", git = "https://github.com/DiamondLightSource/blueapi.git?rev=308f55f43f0623d36101d124ab9e85b9e7532568" }, { name = "fastapi", specifier = ">=0.136.0" }, { name = "pydantic", specifier = ">=2.13.2" }, ] From dfc0729288011c9c4ddb2eb26ab28232453ee883 Mon Sep 17 00:00:00 2001 From: Jacob Williamson Date: Fri, 14 Aug 2026 12:03:50 +0100 Subject: [PATCH 14/22] More tests and prevent tests calling real identity provider --- src/daq_queuing_service/api/api.py | 5 + tests/constants.py | 2 +- .../config_with_auth.yaml} | 1 - tests/test_data/test_config_with_auth.yaml | 18 ++++ tests/unit_tests/conftest.py | 49 +++++++++- tests/unit_tests/test_api.py | 91 ++++++++++++++++++- tests/unit_tests/test_app.py | 7 +- 7 files changed, 165 insertions(+), 8 deletions(-) rename tests/{test_data/test_config_with_authn.yaml => system_tests/config_with_auth.yaml} (90%) create mode 100644 tests/test_data/test_config_with_auth.yaml diff --git a/src/daq_queuing_service/api/api.py b/src/daq_queuing_service/api/api.py index c8108ca..5d9549e 100644 --- a/src/daq_queuing_service/api/api.py +++ b/src/daq_queuing_service/api/api.py @@ -41,6 +41,7 @@ def _filter_by_status( def public_routes(queue: TaskQueue) -> APIRouter: + """No authentication is required to access these endpoints.""" router = APIRouter() @router.get("/") @@ -68,6 +69,10 @@ def protected_routes( converter: Converter, whitelist_check: Callable[[User], User] | None = None, ) -> APIRouter: + """Authentication is required to access these endpoints (if turned on in config). + Additionally, for endpoints that depend on whitelist_check, you must be in the + whitelist of authorised fedIDs to access them. + """ authorised = [Depends(whitelist_check)] if whitelist_check else None router = APIRouter() diff --git a/tests/constants.py b/tests/constants.py index b8e4585..7a8b2c1 100644 --- a/tests/constants.py +++ b/tests/constants.py @@ -1,2 +1,2 @@ TEST_CONFIG_PATH = "tests/test_data/test_config.yaml" -TEST_CONFIG_WITH_AUTHN_PATH = "tests/test_data/test_config_with_authn.yaml" +TEST_CONFIG_WITH_AUTH_PATH = "tests/test_data/test_config_with_auth.yaml" diff --git a/tests/test_data/test_config_with_authn.yaml b/tests/system_tests/config_with_auth.yaml similarity index 90% rename from tests/test_data/test_config_with_authn.yaml rename to tests/system_tests/config_with_auth.yaml index ef37764..f8221c7 100644 --- a/tests/test_data/test_config_with_authn.yaml +++ b/tests/system_tests/config_with_auth.yaml @@ -14,4 +14,3 @@ oidc: issuer: "https://identity.diamond.ac.uk/realms/dls" client_id: "daq-queuing-service" client_audience: "account" - logout_redirect_endpoint: "oauth2/sign_out" diff --git a/tests/test_data/test_config_with_auth.yaml b/tests/test_data/test_config_with_auth.yaml new file mode 100644 index 0000000..bd48528 --- /dev/null +++ b/tests/test_data/test_config_with_auth.yaml @@ -0,0 +1,18 @@ +converter: + path: "daq_queuing_service.plugins.converter" + name: "Converter" +blueapi: + api: + url: "http://localhost:8000" + stomp: + enabled: true # All other stomp settings will be ignored if this is false + url: tcp://localhost:61613 + auth: + username: guest + password: guest +oidc: + issuer: "https://example.com" + client_id: "daq-queuing-service" + client_audience: "account" +authorisation_whitelist: + - "abc12345" diff --git a/tests/unit_tests/conftest.py b/tests/unit_tests/conftest.py index 20ae5f7..d7b9e88 100644 --- a/tests/unit_tests/conftest.py +++ b/tests/unit_tests/conftest.py @@ -1,9 +1,20 @@ +from functools import cached_property +from pathlib import Path +from typing import Any +from unittest.mock import MagicMock, patch + import pytest +from blueapi.config import OIDCConfig from blueapi.service.model import TaskRequest from blueapi.worker.event import TaskError, TaskResult +from fastapi import FastAPI from fastapi.dependencies.models import Dependant from pytest import MonkeyPatch +from constants import TEST_CONFIG_WITH_AUTH_PATH +from daq_queuing_service.app._config import AppConfig, load_config +from daq_queuing_service.app.app import create_app +from daq_queuing_service.app.authentication import User from daq_queuing_service.blueapi_interaction.blueapi_call import BlueapiCall from daq_queuing_service.broadcaster import Broadcaster from daq_queuing_service.log import LOGGER @@ -157,7 +168,43 @@ def _construct_blueapi_task_request( return DNConverter() -def has_dependency_name(dep: Dependant, name: str): +@pytest.fixture +def use_config_with_auth(): + class MockOIDCConfig(OIDCConfig): + @cached_property + def _config_from_oidc_url(self) -> dict[str, Any]: + # This would usually make a real request, we don't want this in tests + return {} + + config = load_config(Path(TEST_CONFIG_WITH_AUTH_PATH)) + assert config.oidc is not None + config.oidc = MockOIDCConfig.model_validate(config.oidc.model_dump()) + with patch("daq_queuing_service.app.app.load_config", return_value=config): + yield config + + +@pytest.fixture +def app_with_auth(use_config_with_auth: AppConfig) -> FastAPI: + return create_app(Path("")) + + +@pytest.fixture +def app_with_authz(use_config_with_auth: AppConfig): + """Authentication always passes. Only user abc12345 is authorised""" + + def fake_get_current_user(): + return User(fedid="xyz54321") + + with patch( + "daq_queuing_service.app.app.build_get_current_user", + MagicMock(return_value=fake_get_current_user), + ): + app = create_app(Path("")) + yield app + return app + + +def has_dependency_name(dep: Dependant, name: str) -> bool: if getattr(dep.call, "__name__", None) == name: return True diff --git a/tests/unit_tests/test_api.py b/tests/unit_tests/test_api.py index 4ddf08d..bd1e0a3 100644 --- a/tests/unit_tests/test_api.py +++ b/tests/unit_tests/test_api.py @@ -5,6 +5,7 @@ from typing import Any from unittest.mock import MagicMock, patch +import httpx import pytest from blueapi.client.rest import ( BlueapiRestClient, @@ -17,13 +18,14 @@ from fastapi.encoders import jsonable_encoder from fastapi.testclient import TestClient +from constants import TEST_CONFIG_PATH from daq_queuing_service.api.api import ( TaskCancelRequest, protected_routes, public_routes, ) from daq_queuing_service.api.errors import register_exception_handlers -from daq_queuing_service.app._config import TEST_CONFIG_PATH, load_config +from daq_queuing_service.app._config import load_config from daq_queuing_service.blueapi_interaction.blueapi_call import ( BlueapiCall, BlueapiCallResponse, @@ -84,6 +86,17 @@ def test_client(app: FastAPI) -> TestClient: return TestClient(app) +@pytest.fixture +def test_client_with_auth(app_with_auth: FastAPI) -> TestClient: + return TestClient(app_with_auth) + + +@pytest.fixture +def test_client_with_authz(app_with_authz: FastAPI) -> TestClient: + """Authentication always passes. Only user abc12345 is authorised""" + return TestClient(app_with_authz) + + def test_read_root_returns_expected_string(test_client: TestClient): response = test_client.get("/") assert response.status_code == 200 @@ -927,3 +940,79 @@ def read_stream(): time.sleep(0.2) assert len(received) == 5 assert received != [] + + +@pytest.mark.parametrize( + "endpoint, method", + [ + ("/config", "get"), + ("/queue/state", "patch"), + ("/queue", "get"), + ("/queue", "post"), + ("/queue", "delete"), + ("/queue/move", "post"), + ("/queue/tasks", "delete"), + ("/queue/5", "get"), + ("/tasks", "get"), + ("/tasks/task_id", "get"), + ("/history", "get"), + ("/history", "delete"), + ("/call_queue", "get"), + ("/call_history", "get"), + ("/events", "get"), + ], +) +def test_endpoints_blocked_by_authentication_check_if_no_token_provided( + endpoint: str, method: str, test_client_with_auth: TestClient +): + response: httpx.Response = getattr(test_client_with_auth, method)(endpoint) + assert response.status_code == 401 + assert response.json() == {"detail": "Not authenticated"} + + +@pytest.mark.parametrize( + "endpoint, method", + [("/", "get"), ("/healthz", "get"), ("/queue/state", "get")], +) +def test_public_endpoints_not_blocked_by_auth( + endpoint: str, method: str, test_client_with_auth: TestClient +): + response: httpx.Response = getattr(test_client_with_auth, method)(endpoint) + assert response.status_code == 200 + + +@pytest.mark.parametrize( + "endpoint, method", + [ + ("/queue/state", "patch"), + ("/queue", "get"), + ("/queue", "post"), + ("/queue", "delete"), + ("/queue/move", "post"), + ("/queue/tasks", "delete"), + ("/queue/5", "get"), + ("/tasks", "get"), + ("/tasks/task_id", "get"), + ("/history", "get"), + ("/history", "delete"), + ("/call_queue", "get"), + ("/call_history", "get"), + ("/events", "get"), + ], +) +def test_endpoints_blocked_by_authorisation_check_if_user_not_in_whitelist( + endpoint: str, method: str, test_client_with_authz: TestClient +): + response: httpx.Response = getattr(test_client_with_authz, method)(endpoint) + assert response.status_code == 403 + assert response.json() == { + "detail": "Not authorised. You are not in the whitelist of authorised FedIDs" + } + + +@pytest.mark.parametrize("endpoint, method", [("/config", "get")]) +def test_endpoints_that_require_authn_but_not_authz_any_user_allowed( + endpoint: str, method: str, test_client_with_authz: TestClient +): + response: httpx.Response = getattr(test_client_with_authz, method)(endpoint) + assert response.status_code == 200 diff --git a/tests/unit_tests/test_app.py b/tests/unit_tests/test_app.py index d51012e..a49bc3b 100644 --- a/tests/unit_tests/test_app.py +++ b/tests/unit_tests/test_app.py @@ -10,7 +10,7 @@ from fastapi.testclient import TestClient from pytest import LogCaptureFixture -from constants import TEST_CONFIG_PATH, TEST_CONFIG_WITH_AUTHN_PATH +from constants import TEST_CONFIG_PATH from daq_queuing_service.app.app import create_app from daq_queuing_service.task_queue.queue import TaskQueue from daq_queuing_service.worker.worker import QueueWorker @@ -107,11 +107,10 @@ def test_if_dev_mode_cors_middlewhere_added_to_app(): ) -def test_create_app_adds_auth_dependencies_to_correct_routes(): +def test_create_app_adds_auth_dependencies_to_correct_routes(app_with_auth: FastAPI): no_auth_required = ["read_root", "healthz", "get_queue_state"] - app = create_app(Path(TEST_CONFIG_WITH_AUTHN_PATH)) - for route in app.routes: + for route in app_with_auth.routes: if isinstance(route, APIRoute): if route.name not in no_auth_required: assert has_dependency_name(route.dependant, "validate_bearer_token"), ( From 34b749dcf418e71cc2921f6de1e0aa03ba946098 Mon Sep 17 00:00:00 2001 From: Jacob Williamson Date: Fri, 14 Aug 2026 12:07:31 +0100 Subject: [PATCH 15/22] Fix docs script and generate api docs --- docs/reference/rest_api.json | 463 +++------------------------ utility_scripts/generate_api_docs.py | 2 +- 2 files changed, 47 insertions(+), 418 deletions(-) diff --git a/docs/reference/rest_api.json b/docs/reference/rest_api.json index 48a1ce6..c9d39c1 100644 --- a/docs/reference/rest_api.json +++ b/docs/reference/rest_api.json @@ -5,22 +5,6 @@ "version": "0.1.0" }, "paths": { - "/healthz": { - "get": { - "summary": "Healthz", - "operationId": "healthz_healthz_get", - "responses": { - "200": { - "description": "Successful Response", - "content": { - "application/json": { - "schema": {} - } - } - } - } - } - }, "/": { "get": { "summary": "Read Root", @@ -37,18 +21,16 @@ } } }, - "/config": { + "/healthz": { "get": { - "summary": "Get Config", - "operationId": "get_config_config_get", + "summary": "Healthz", + "operationId": "healthz_healthz_get", "responses": { "200": { "description": "Successful Response", "content": { "application/json": { - "schema": { - "$ref": "#/components/schemas/AppConfig" - } + "schema": {} } } } @@ -109,6 +91,24 @@ } } }, + "/config": { + "get": { + "summary": "Get Config", + "operationId": "get_config_config_get", + "responses": { + "200": { + "description": "Successful Response", + "content": { + "application/json": { + "schema": { + "$ref": "#/components/schemas/AppConfig" + } + } + } + } + } + } + }, "/queue": { "get": { "summary": "Get Queued Tasks", @@ -605,45 +605,10 @@ "AppConfig": { "properties": { "blueapi": { - "$ref": "#/components/schemas/ApplicationConfig" + "$ref": "#/components/schemas/BlueapiConfig" }, "converter": { "$ref": "#/components/schemas/ConverterConfig" - } - }, - "type": "object", - "required": [ - "blueapi", - "converter" - ], - "title": "AppConfig" - }, - "ApplicationConfig": { - "properties": { - "stomp": { - "$ref": "#/components/schemas/StompConfig" - }, - "tiled": { - "$ref": "#/components/schemas/TiledConfig" - }, - "env": { - "$ref": "#/components/schemas/EnvironmentConfig" - }, - "logging": { - "$ref": "#/components/schemas/LoggingConfig" - }, - "api": { - "$ref": "#/components/schemas/RestConfig" - }, - "scratch": { - "anyOf": [ - { - "$ref": "#/components/schemas/ScratchConfig" - }, - { - "type": "null" - } - ] }, "oidc": { "anyOf": [ @@ -655,43 +620,27 @@ } ] }, - "auth_token_path": { + "authorisation_whitelist": { "anyOf": [ { - "type": "string", - "format": "path" + "items": { + "type": "string" + }, + "type": "array" }, { "type": "null" } ], - "title": "Auth Token Path" - }, - "numtracker": { - "anyOf": [ - { - "$ref": "#/components/schemas/NumtrackerConfig" - }, - { - "type": "null" - } - ] - }, - "opa": { - "anyOf": [ - { - "$ref": "#/components/schemas/OpaConfig" - }, - { - "type": "null" - } - ] + "title": "Authorisation Whitelist" } }, - "additionalProperties": false, "type": "object", - "title": "ApplicationConfig", - "description": "Config for the worker application as a whole. Root of\nconfig tree." + "required": [ + "blueapi", + "converter" + ], + "title": "AppConfig" }, "BasicAuthentication": { "properties": { @@ -805,6 +754,18 @@ ], "title": "BlueapiCallResponse" }, + "BlueapiConfig": { + "properties": { + "stomp": { + "$ref": "#/components/schemas/StompConfig" + }, + "api": { + "$ref": "#/components/schemas/RestConfig" + } + }, + "type": "object", + "title": "BlueapiConfig" + }, "CORSConfig": { "properties": { "origins": { @@ -877,82 +838,6 @@ ], "title": "ConverterConfig" }, - "DeviceManagerSource": { - "properties": { - "module": { - "type": "string", - "title": "Module", - "description": "Module to be imported" - }, - "kind": { - "type": "string", - "const": "deviceManager", - "title": "Kind", - "default": "deviceManager" - }, - "mock": { - "type": "boolean", - "title": "Mock", - "description": "If true, ophyd_async device connections are mocked", - "default": false - }, - "name": { - "type": "string", - "title": "Name", - "description": "Name of the device manager in the module", - "default": "devices" - } - }, - "additionalProperties": false, - "type": "object", - "required": [ - "module" - ], - "title": "DeviceManagerSource" - }, - "EnvironmentConfig": { - "properties": { - "sources": { - "items": { - "oneOf": [ - { - "$ref": "#/components/schemas/PlanSource" - }, - { - "$ref": "#/components/schemas/DeviceManagerSource" - } - ], - "discriminator": { - "propertyName": "kind", - "mapping": { - "deviceManager": "#/components/schemas/DeviceManagerSource", - "planFunctions": "#/components/schemas/PlanSource" - } - } - }, - "type": "array", - "title": "Sources", - "default": [] - }, - "events": { - "$ref": "#/components/schemas/WorkerEventConfig" - }, - "metadata": { - "anyOf": [ - { - "$ref": "#/components/schemas/MetadataConfig" - }, - { - "type": "null" - } - ] - } - }, - "additionalProperties": false, - "type": "object", - "title": "EnvironmentConfig", - "description": "Config for the RunEngine environment" - }, "Experiment": { "properties": { "name": { @@ -1003,25 +888,6 @@ ], "title": "ExperimentDefinition" }, - "GraylogConfig": { - "properties": { - "enabled": { - "type": "boolean", - "title": "Enabled", - "default": false - }, - "url": { - "type": "string", - "minLength": 1, - "format": "uri", - "title": "Url", - "default": "tcp://localhost:5555" - } - }, - "additionalProperties": false, - "type": "object", - "title": "GraylogConfig" - }, "HTTPValidationError": { "properties": { "detail": { @@ -1035,67 +901,6 @@ "type": "object", "title": "HTTPValidationError" }, - "LoggingConfig": { - "properties": { - "level": { - "type": "string", - "enum": [ - "NOTSET", - "DEBUG", - "INFO", - "WARNING", - "ERROR", - "CRITICAL" - ], - "title": "Level", - "default": "INFO" - }, - "graylog": { - "$ref": "#/components/schemas/GraylogConfig", - "default": { - "enabled": false, - "url": "tcp://localhost:5555" - } - } - }, - "additionalProperties": false, - "type": "object", - "title": "LoggingConfig" - }, - "MetadataConfig": { - "properties": { - "instrument": { - "type": "string", - "title": "Instrument" - } - }, - "additionalProperties": false, - "type": "object", - "required": [ - "instrument" - ], - "title": "MetadataConfig" - }, - "NumtrackerConfig": { - "properties": { - "url": { - "type": "string", - "maxLength": 2083, - "minLength": 1, - "format": "uri", - "title": "Url", - "default": "http://localhost:8406/graphql" - }, - "detector_file_template": { - "type": "string", - "title": "Detector File Template", - "default": "{instrument}-{scan_id}-{device_name}" - } - }, - "additionalProperties": false, - "type": "object", - "title": "NumtrackerConfig" - }, "OIDCConfig": { "properties": { "well_known_url": { @@ -1148,33 +953,6 @@ ], "title": "OIDCConfig" }, - "OpaConfig": { - "properties": { - "root": { - "type": "string", - "maxLength": 2083, - "minLength": 1, - "format": "uri", - "title": "Root", - "default": "http://localhost:8181/" - }, - "audience": { - "type": "string", - "title": "Audience", - "default": "account" - }, - "tiled_service_account_check": { - "type": "string", - "title": "Tiled Service Account Check" - } - }, - "additionalProperties": false, - "type": "object", - "required": [ - "tiled_service_account_check" - ], - "title": "OpaConfig" - }, "PauseReason": { "type": "string", "enum": [ @@ -1184,27 +962,6 @@ ], "title": "PauseReason" }, - "PlanSource": { - "properties": { - "module": { - "type": "string", - "title": "Module", - "description": "Module to be imported" - }, - "kind": { - "type": "string", - "const": "planFunctions", - "title": "Kind", - "default": "planFunctions" - } - }, - "additionalProperties": false, - "type": "object", - "required": [ - "module" - ], - "title": "PlanSource" - }, "QueueState": { "properties": { "paused": { @@ -1284,85 +1041,6 @@ ], "title": "Sample" }, - "ScratchConfig": { - "properties": { - "root": { - "type": "string", - "format": "path", - "title": "Root", - "description": "The root directory of the scratch area, all repositories will be cloned under this directory.", - "default": "/tmp/scratch/blueapi" - }, - "required_gid": { - "anyOf": [ - { - "type": "integer" - }, - { - "type": "null" - } - ], - "title": "Required Gid", - "description": "\nRequired owner GID for the scratch directory. If supplied, the setup-scratch\ncommand will check the scratch area ownership and raise an error if it is\nnot owned by , or if it does not have SGID permission bit set.\n" - }, - "repositories": { - "items": { - "$ref": "#/components/schemas/ScratchRepository" - }, - "type": "array", - "title": "Repositories", - "description": "Details of repositories to be cloned and imported into blueapi" - } - }, - "additionalProperties": false, - "type": "object", - "title": "ScratchConfig" - }, - "ScratchRepository": { - "properties": { - "name": { - "type": "string", - "title": "Name", - "description": "Unique name for this repository in the scratch directory", - "default": "example" - }, - "remote_url": { - "type": "string", - "title": "Remote Url", - "description": "URL to clone from", - "default": "https://github.com/example/example.git" - }, - "target_revision": { - "type": "string", - "title": "Target Revision", - "description": "Revision (branch or tag) to check out when cloning - defaults to remote's HEAD. If a tag is used, the repo will be left in a 'detached head' state." - } - }, - "additionalProperties": false, - "type": "object", - "title": "ScratchRepository" - }, - "ServiceAccount": { - "properties": { - "client_id": { - "type": "string", - "title": "Client Id", - "description": "Service account client ID", - "default": "" - }, - "client_secret": { - "type": "string", - "format": "password", - "title": "Client Secret", - "description": "Service account client secret", - "default": "", - "writeOnly": true - } - }, - "additionalProperties": false, - "type": "object", - "title": "ServiceAccount" - }, "Status": { "type": "string", "enum": [ @@ -1561,42 +1239,6 @@ ], "title": "TaskWithPosition" }, - "TiledConfig": { - "properties": { - "enabled": { - "type": "boolean", - "title": "Enabled", - "description": "True if blueapi should forward data to a Tiled instance", - "default": false - }, - "url": { - "type": "string", - "maxLength": 2083, - "minLength": 1, - "format": "uri", - "title": "Url", - "default": "http://localhost:8407/" - }, - "authentication": { - "anyOf": [ - { - "type": "string" - }, - { - "$ref": "#/components/schemas/ServiceAccount" - }, - { - "type": "null" - } - ], - "title": "Authentication", - "description": "Tiled Authentication can be API_KEY or OIDC Service account" - } - }, - "additionalProperties": false, - "type": "object", - "title": "TiledConfig" - }, "ValidationError": { "properties": { "loc": { @@ -1636,19 +1278,6 @@ "type" ], "title": "ValidationError" - }, - "WorkerEventConfig": { - "properties": { - "broadcast_status_events": { - "type": "boolean", - "title": "Broadcast Status Events", - "default": true - } - }, - "additionalProperties": false, - "type": "object", - "title": "WorkerEventConfig", - "description": "Config for event broadcasting via the message bus" } } } diff --git a/utility_scripts/generate_api_docs.py b/utility_scripts/generate_api_docs.py index 6934f6f..cc25505 100644 --- a/utility_scripts/generate_api_docs.py +++ b/utility_scripts/generate_api_docs.py @@ -7,7 +7,7 @@ from daq_queuing_service.api.api import protected_routes, public_routes app = FastAPI() -app.include_router(public_routes()) +app.include_router(public_routes(MagicMock())) app.include_router(protected_routes(MagicMock(), MagicMock(), MagicMock(), MagicMock())) openapi = get_openapi( From fbdac19c877d98e7e0aa92f6ee79b2e570f88970 Mon Sep 17 00:00:00 2001 From: Jacob Williamson Date: Fri, 14 Aug 2026 12:48:45 +0100 Subject: [PATCH 16/22] More tests --- src/daq_queuing_service/app/authentication.py | 2 +- tests/unit_tests/conftest.py | 11 +- tests/unit_tests/test_authentication.py | 118 ++++++++++++++++++ 3 files changed, 128 insertions(+), 3 deletions(-) diff --git a/src/daq_queuing_service/app/authentication.py b/src/daq_queuing_service/app/authentication.py index 9539de8..1969b02 100644 --- a/src/daq_queuing_service/app/authentication.py +++ b/src/daq_queuing_service/app/authentication.py @@ -15,7 +15,7 @@ class User(BaseModel): fedid: str email: str | None = None - username: str | None = None + name: str | None = None # Some of the following code was copied from blueapi diff --git a/tests/unit_tests/conftest.py b/tests/unit_tests/conftest.py index d7b9e88..fc50946 100644 --- a/tests/unit_tests/conftest.py +++ b/tests/unit_tests/conftest.py @@ -169,7 +169,7 @@ def _construct_blueapi_task_request( @pytest.fixture -def use_config_with_auth(): +def oidc_config(): class MockOIDCConfig(OIDCConfig): @cached_property def _config_from_oidc_url(self) -> dict[str, Any]: @@ -178,7 +178,14 @@ def _config_from_oidc_url(self) -> dict[str, Any]: config = load_config(Path(TEST_CONFIG_WITH_AUTH_PATH)) assert config.oidc is not None - config.oidc = MockOIDCConfig.model_validate(config.oidc.model_dump()) + return MockOIDCConfig.model_validate(config.oidc.model_dump()) + + +@pytest.fixture +def use_config_with_auth(oidc_config: OIDCConfig): + + config = load_config(Path(TEST_CONFIG_WITH_AUTH_PATH)) + config.oidc = oidc_config with patch("daq_queuing_service.app.app.load_config", return_value=config): yield config diff --git a/tests/unit_tests/test_authentication.py b/tests/unit_tests/test_authentication.py index e69de29..9091032 100644 --- a/tests/unit_tests/test_authentication.py +++ b/tests/unit_tests/test_authentication.py @@ -0,0 +1,118 @@ +from unittest.mock import MagicMock, patch + +import pytest +from blueapi.config import OIDCConfig +from fastapi import HTTPException +from fastapi.security import HTTPAuthorizationCredentials +from jwt import DecodeError, ExpiredSignatureError + +from daq_queuing_service.app.authentication import ( + User, + build_access_token_check, + build_get_current_user, + unchecked_bearer_token, +) + + +@pytest.fixture(autouse=True) +def jwk_client(): + with patch( + "daq_queuing_service.app.authentication.jwt.PyJWKClient" + ) as mock_client_class: + yield mock_client_class.return_value + + +def test_unchecked_bearer_token_returns_credentials(): + expected = "fake credentials" + result = unchecked_bearer_token( + HTTPAuthorizationCredentials(scheme="", credentials=expected) + ) + assert result == expected + + +def test_validate_bearer_token_gets_signing_key_from_jwt_and_decodes_token_with_it( + oidc_config: OIDCConfig, jwk_client: MagicMock +): + token = "fake_token" + validate_bearer_token = build_access_token_check(oidc_config) + with patch("daq_queuing_service.app.authentication.jwt.decode") as mock_decode: + validate_bearer_token(token) + + jwk_client.get_signing_key_from_jwt.assert_called_once_with(token) + signing_key = jwk_client.get_signing_key_from_jwt.return_value.key + mock_decode.assert_called_once_with( + token, + signing_key, + algorithms=oidc_config.id_token_signing_alg_values_supported, + verify=True, + audience=oidc_config.client_audience, + issuer=oidc_config.issuer, + ) + + +def test_validate_bearer_token_raises_appropriate_http_exception_if_no_token_provided( + oidc_config: OIDCConfig, +): + validate_bearer_token = build_access_token_check(oidc_config) + with pytest.raises(HTTPException) as exc: + validate_bearer_token(None) + + assert exc.value.status_code == 401 + assert exc.value.detail == "Not authenticated" + + +def test_validate_bearer_token_raises_appropriate_http_exception_if_decode_error( + oidc_config: OIDCConfig, jwk_client: MagicMock +): + jwk_client.get_signing_key_from_jwt.side_effect = DecodeError + validate_bearer_token = build_access_token_check(oidc_config) + with pytest.raises(HTTPException) as exc: + validate_bearer_token("token") + + assert exc.value.status_code == 401 + assert exc.value.detail == "Cannot decode token" + + +def test_validate_bearer_token_raises_appropriate_http_exception_if_expired_error( + oidc_config: OIDCConfig, jwk_client: MagicMock +): + jwk_client.get_signing_key_from_jwt.side_effect = ExpiredSignatureError + validate_bearer_token = build_access_token_check(oidc_config) + with pytest.raises(HTTPException) as exc: + validate_bearer_token("token") + + assert exc.value.status_code == 401 + assert exc.value.detail == "Token expired" + + +def test_get_current_user_builds_user_from_validated_token_and_adds_to_request_state(): + def validate_user(): + return { + "fedid": "abc12355", + "email": "joe.blogs@diamond.ac.uk", + "name": "Joe Blogs", + } + + expected_user = User( + fedid="abc12355", email="joe.blogs@diamond.ac.uk", name="Joe Blogs" + ) + + get_current_user = build_get_current_user(MagicMock()) + request = MagicMock() + user = get_current_user(request, validate_user()) + + assert user == expected_user + assert request.state.user == expected_user + + +def test_get_current_user_raises_error_if_token_cannot_validate_to_user_class(): + def validate_user(): + return {"no fedid": "abc12355"} + + get_current_user = build_get_current_user(MagicMock()) + request = MagicMock() + with pytest.raises(HTTPException) as exc: + get_current_user(request, validate_user()) + + assert exc.value.status_code == 401 + assert exc.value.detail == "Invalid token claims" From d6203934836a865646f4701503c832cc41c1af16 Mon Sep 17 00:00:00 2001 From: Jacob Williamson Date: Fri, 14 Aug 2026 13:33:35 +0100 Subject: [PATCH 17/22] typo --- tests/unit_tests/test_authorisation.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/unit_tests/test_authorisation.py b/tests/unit_tests/test_authorisation.py index ab6b3d7..a26f349 100644 --- a/tests/unit_tests/test_authorisation.py +++ b/tests/unit_tests/test_authorisation.py @@ -28,7 +28,7 @@ def test_ensure_current_user_is_in_whitelist_raises_error_if_user_not_in_whiteli whitelist_check(user) -def test_ensure_current_user_is_in_whitelist_returns_user_no_whitelist_provided(): +def test_ensure_current_user_is_in_whitelist_returns_user_if_no_whitelist_provided(): user = User(fedid="abc12345") whitelist_check = build_ensure_current_user_is_in_whitelist(None, MagicMock()) From 11f38ab6caa06e0487c110e08eff2dfdc2af8170 Mon Sep 17 00:00:00 2001 From: Jacob Williamson Date: Fri, 14 Aug 2026 14:13:58 +0100 Subject: [PATCH 18/22] Add user to task model --- src/daq_queuing_service/api/api.py | 12 +++++++++++- src/daq_queuing_service/app/authentication.py | 2 +- src/daq_queuing_service/task_queue/task.py | 3 +++ .../unit_tests/plugins/i15-1/test_i15_1_converter.py | 5 +++++ tests/unit_tests/test_api.py | 12 ++++++++++++ tests/unit_tests/test_queue.py | 10 ++++++++++ 6 files changed, 42 insertions(+), 2 deletions(-) diff --git a/src/daq_queuing_service/api/api.py b/src/daq_queuing_service/api/api.py index 5d9549e..e9799db 100644 --- a/src/daq_queuing_service/api/api.py +++ b/src/daq_queuing_service/api/api.py @@ -1,6 +1,7 @@ import asyncio import json from collections.abc import AsyncGenerator, Callable +from typing import Annotated from blueapi.service.model import TaskRequest from fastapi import APIRouter, Depends, Request, Response @@ -24,6 +25,14 @@ # pyright: reportUnusedFunction=false +def get_current_user(request: Request) -> User | None: + if hasattr(request.state, "user"): + return request.state.user + + +CurrentUser = Annotated[User | None, Depends(get_current_user)] + + class QueueStateUpdate(BaseModel): paused: bool @@ -94,6 +103,7 @@ async def get_queued_tasks(status: Status | None = None) -> list[TaskWithPositio @router.post("/queue", dependencies=authorised) async def add_tasks_to_queue( experiments: list[TaskRequest | Experiment], + user: CurrentUser, position: int | None = None, ) -> list[str]: try: @@ -101,7 +111,7 @@ async def add_tasks_to_queue( except Exception as e: raise ValidateError(*e.args) from e - tasks = [Task(experiment=experiment) for experiment in experiments] + tasks = [Task(experiment=experiment, user=user) for experiment in experiments] task_ids = [task.id for task in tasks] await queue.add_tasks(tasks, position) return task_ids diff --git a/src/daq_queuing_service/app/authentication.py b/src/daq_queuing_service/app/authentication.py index 1969b02..a287afa 100644 --- a/src/daq_queuing_service/app/authentication.py +++ b/src/daq_queuing_service/app/authentication.py @@ -9,7 +9,7 @@ from pydantic import BaseModel, ValidationError from starlette.status import HTTP_401_UNAUTHORIZED -from daq_queuing_service.worker.worker import LOGGER +from daq_queuing_service.log import LOGGER class User(BaseModel): diff --git a/src/daq_queuing_service/task_queue/task.py b/src/daq_queuing_service/task_queue/task.py index 016a187..205b3c1 100644 --- a/src/daq_queuing_service/task_queue/task.py +++ b/src/daq_queuing_service/task_queue/task.py @@ -5,6 +5,7 @@ from blueapi.service.model import TaskRequest from pydantic import BaseModel, Field, computed_field +from daq_queuing_service.app.authentication import User from daq_queuing_service.blueapi_interaction.blueapi_call import ( BlueapiCall, BlueapiCallResponse, @@ -52,6 +53,7 @@ class Task(BaseModel): experiment: Experiment | TaskRequest id: str = Field(default_factory=create_uuid_str) blueapi_calls: list[BlueapiCall] = Field(default_factory=lambda: []) + user: User | None = None _cancelled: bool = False def cancel(self): @@ -97,6 +99,7 @@ class TaskWithPosition(BaseModel): blueapi_calls: list[BlueapiCallResponse] position: int | None kind: TaskKind + user: User | None @classmethod def from_task(cls, task: Task, position: int | None = None) -> Self: diff --git a/tests/unit_tests/plugins/i15-1/test_i15_1_converter.py b/tests/unit_tests/plugins/i15-1/test_i15_1_converter.py index 282c368..9d10227 100644 --- a/tests/unit_tests/plugins/i15-1/test_i15_1_converter.py +++ b/tests/unit_tests/plugins/i15-1/test_i15_1_converter.py @@ -132,6 +132,7 @@ def test_experiment_with_correct_experiment_type_are_converted(): blueapi_calls=[], position=None, kind=TaskKind.EXPERIMENT, + user=None, ) call_list = I151Converter().construct_blueapi_calls([task], [], []) assert len(call_list) == 3 @@ -153,6 +154,7 @@ def test_mix_of_experiments_with_correct_experiment_type_are_converted(): blueapi_calls=[], position=None, kind=TaskKind.EXPERIMENT, + user=None, ) class BadExperiment: @@ -201,6 +203,7 @@ def test_if_no_background_found_in_tiled_then_background_scan_added_to_tasks( "blueapi_calls": [], "status": Status.QUEUED, "kind": TaskKind.EXPERIMENT, + "user": None, } @@ -281,6 +284,7 @@ def test_same_experiment_in_different_instrument_sessions_will_add_background_in "blueapi_calls": [], "status": Status.QUEUED, "kind": TaskKind.EXPERIMENT, + "user": None, } new_tasks[2].id = "" assert new_tasks[2].model_dump() == { @@ -298,6 +302,7 @@ def test_same_experiment_in_different_instrument_sessions_will_add_background_in "blueapi_calls": [], "status": Status.QUEUED, "kind": TaskKind.EXPERIMENT, + "user": None, } diff --git a/tests/unit_tests/test_api.py b/tests/unit_tests/test_api.py index bd1e0a3..29e7ad4 100644 --- a/tests/unit_tests/test_api.py +++ b/tests/unit_tests/test_api.py @@ -174,6 +174,7 @@ def test_get_queued_tasks_returns_queued_task(test_client: TestClient): ], "position": 0, "kind": "Experiment", + "user": None, }, { "experiment": { @@ -202,6 +203,7 @@ def test_get_queued_tasks_returns_queued_task(test_client: TestClient): ], "position": 1, "kind": "Experiment", + "user": None, }, { "experiment": { @@ -230,6 +232,7 @@ def test_get_queued_tasks_returns_queued_task(test_client: TestClient): ], "position": 2, "kind": "Experiment", + "user": None, }, ] @@ -265,6 +268,7 @@ def test_get_queued_tasks_can_filter_by_task_status(test_client: TestClient): ], "position": 0, "kind": "Experiment", + "user": None, } ] @@ -314,6 +318,7 @@ async def test_get_all_tasks_can_filter_by_task_status(test_client: TestClient): ], "position": None, "kind": "Experiment", + "user": None, } ] @@ -370,6 +375,7 @@ async def test_add_tasks_to_queue_adds_to_queue_and_and_returns_task_ids( position=3, status=Status.QUEUED, kind=TaskKind.PLAN, + user=None, ) @@ -620,6 +626,7 @@ async def test_cancel_tasks_removes_task_from_queue_and_returns_tasks( ], "position": None, "kind": "Experiment", + "user": None, }, { "experiment": { @@ -648,6 +655,7 @@ async def test_cancel_tasks_removes_task_from_queue_and_returns_tasks( ], "position": None, "kind": "Experiment", + "user": None, }, ] @@ -748,6 +756,7 @@ async def test_cancel_all_tasks_removes_all_queued_tasks_from_queue_and_returns_ ], "position": None, "kind": "Experiment", + "user": None, }, { "experiment": { @@ -776,6 +785,7 @@ async def test_cancel_all_tasks_removes_all_queued_tasks_from_queue_and_returns_ ], "position": None, "kind": "Experiment", + "user": None, }, ] @@ -810,6 +820,7 @@ def test_get_task_by_position_returns_expected_task(test_client: TestClient): ], "position": 1, "kind": "Experiment", + "user": None, } @@ -843,6 +854,7 @@ def test_get_task_by_id_returns_expected_task(test_client: TestClient): ], "position": 1, "kind": "Experiment", + "user": None, } diff --git a/tests/unit_tests/test_queue.py b/tests/unit_tests/test_queue.py index c78da08..bfb9ef2 100644 --- a/tests/unit_tests/test_queue.py +++ b/tests/unit_tests/test_queue.py @@ -304,6 +304,7 @@ async def test_get_queue_only_returns_tasks_in_queue( ], position=0, kind=TaskKind.EXPERIMENT, + user=None, ), TaskWithPosition( experiment=Experiment( @@ -330,6 +331,7 @@ async def test_get_queue_only_returns_tasks_in_queue( ], position=1, kind=TaskKind.EXPERIMENT, + user=None, ), TaskWithPosition( experiment=Experiment( @@ -356,6 +358,7 @@ async def test_get_queue_only_returns_tasks_in_queue( ], position=2, kind=TaskKind.EXPERIMENT, + user=None, ), ] @@ -397,6 +400,7 @@ async def test_get_history_only_returns_tasks_in_history( ], position=None, kind=TaskKind.EXPERIMENT, + user=None, ), TaskWithPosition( experiment=Experiment( @@ -423,6 +427,7 @@ async def test_get_history_only_returns_tasks_in_history( ], position=None, kind=TaskKind.EXPERIMENT, + user=None, ), ] @@ -465,6 +470,7 @@ async def test_get_tasks_returns_tasks_in_queue_and_history( ], position=None, kind=TaskKind.EXPERIMENT, + user=None, ), TaskWithPosition( experiment=Experiment( @@ -491,6 +497,7 @@ async def test_get_tasks_returns_tasks_in_queue_and_history( ], position=None, kind=TaskKind.EXPERIMENT, + user=None, ), TaskWithPosition( experiment=Experiment( @@ -517,6 +524,7 @@ async def test_get_tasks_returns_tasks_in_queue_and_history( ], position=0, kind=TaskKind.EXPERIMENT, + user=None, ), TaskWithPosition( experiment=Experiment( @@ -543,6 +551,7 @@ async def test_get_tasks_returns_tasks_in_queue_and_history( ], position=1, kind=TaskKind.EXPERIMENT, + user=None, ), TaskWithPosition( experiment=Experiment( @@ -569,6 +578,7 @@ async def test_get_tasks_returns_tasks_in_queue_and_history( ], position=2, kind=TaskKind.EXPERIMENT, + user=None, ), ] From 4b79dbfeb4e31d84bd89468ebe0e6afef2580737 Mon Sep 17 00:00:00 2001 From: Jacob Williamson Date: Fri, 14 Aug 2026 14:43:59 +0100 Subject: [PATCH 19/22] Add test --- tests/unit_tests/test_api.py | 24 ++++++++++++++++++++++++ 1 file changed, 24 insertions(+) diff --git a/tests/unit_tests/test_api.py b/tests/unit_tests/test_api.py index 29e7ad4..b297770 100644 --- a/tests/unit_tests/test_api.py +++ b/tests/unit_tests/test_api.py @@ -21,11 +21,13 @@ from constants import TEST_CONFIG_PATH from daq_queuing_service.api.api import ( TaskCancelRequest, + get_current_user, protected_routes, public_routes, ) from daq_queuing_service.api.errors import register_exception_handlers from daq_queuing_service.app._config import load_config +from daq_queuing_service.app.authentication import User from daq_queuing_service.blueapi_interaction.blueapi_call import ( BlueapiCall, BlueapiCallResponse, @@ -379,6 +381,28 @@ async def test_add_tasks_to_queue_adds_to_queue_and_and_returns_task_ids( ) +async def test_add_tasks_to_queue_adds_user_to_task_object( + app: FastAPI, task_queue_with_history: TaskQueue +): + user = User(fedid="abc12345", email="joe.blogs@diamond.ac.uk", name="Joe Blogs") + app.dependency_overrides[get_current_user] = lambda: user + test_client = TestClient(app) + + task_id = test_client.post( + "/queue", + json=[ + { + "name": "add_tasks", + "params": {"time": 10}, + "instrument_session": "abc", + } + ], + ).json()[0] + task = await task_queue_with_history.get_task_by_id(task_id) + assert task + assert task.user == user + + async def test_add_tasks_to_queue_validates_new_tasks_and_gives_expected_error_if_fails( test_client: TestClient, task_queue_with_history: TaskQueue, converter: Converter ): From 86effdf1eea281dfe693c48683af9ac442c12720 Mon Sep 17 00:00:00 2001 From: Jacob Williamson Date: Fri, 14 Aug 2026 14:46:08 +0100 Subject: [PATCH 20/22] Test for coverage --- tests/unit_tests/test_api.py | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/tests/unit_tests/test_api.py b/tests/unit_tests/test_api.py index b297770..a312edd 100644 --- a/tests/unit_tests/test_api.py +++ b/tests/unit_tests/test_api.py @@ -1052,3 +1052,10 @@ def test_endpoints_that_require_authn_but_not_authz_any_user_allowed( ): response: httpx.Response = getattr(test_client_with_authz, method)(endpoint) assert response.status_code == 200 + + +def test_get_current_user_returns_user_from_request_state(): + request = MagicMock() + user = User(fedid="abc12345") + request.state.user = user + assert get_current_user(request) == user From 02a6eaed4547e6798c3aa539495a86bea6645207 Mon Sep 17 00:00:00 2001 From: Jacob Williamson Date: Tue, 18 Aug 2026 11:30:24 +0100 Subject: [PATCH 21/22] Update versions --- pyproject.toml | 5 ++--- uv.lock | 14 +++++++++----- 2 files changed, 11 insertions(+), 8 deletions(-) diff --git a/pyproject.toml b/pyproject.toml index cd9efe0..99ebdf2 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -13,11 +13,10 @@ classifiers = [ ] description = "A service to queue tasks and chain BlueAPI calls" dependencies = [ - # Pin needed until https://github.com/DiamondLightSource/blueapi/pull/1625 in a release - "blueapi @ git+https://github.com/DiamondLightSource/blueapi.git@308f55f43f0623d36101d124ab9e85b9e7532568", + "blueapi>=1.18.1", "fastapi>=0.136.0", "pydantic>=2.13.2", - "tiled>=0.2.9", + "tiled[client]>=0.2.9", ] dynamic = ["version"] license.file = "LICENSE" diff --git a/uv.lock b/uv.lock index 68ae462..1371684 100644 --- a/uv.lock +++ b/uv.lock @@ -354,8 +354,8 @@ wheels = [ [[package]] name = "blueapi" -version = "1.18.1.dev3+g308f55f43" -source = { git = "https://github.com/DiamondLightSource/blueapi.git?rev=308f55f43f0623d36101d124ab9e85b9e7532568#308f55f43f0623d36101d124ab9e85b9e7532568" } +version = "1.18.1" +source = { registry = "https://pypi.org/simple" } dependencies = [ { name = "aioca" }, { name = "aiohttp" }, @@ -381,6 +381,10 @@ dependencies = [ { name = "tomlkit" }, { name = "uvicorn" }, ] +sdist = { url = "https://files.pythonhosted.org/packages/ac/a6/26003d820f710e7b68e4b2f49f7d5a7700f5f00266f76fb17d2c8baebab4/blueapi-1.18.1.tar.gz", hash = "sha256:99bebac382f3e8a088d9fe22bc3972547e60c6648c36535fd025c83df8e97f37", size = 1881821, upload-time = "2026-08-18T09:36:05.463Z" } +wheels = [ + { url = "https://files.pythonhosted.org/packages/dd/f6/fa26155e2373a9ef3d76b6a678bab9aad88e49524a10e3b1a2c0409c1758/blueapi-1.18.1-py3-none-any.whl", hash = "sha256:f8c362eda55fcc3eea5c306425456b805c63b14efe680e9790c23ca6080d9e4b", size = 88065, upload-time = "2026-08-18T09:36:04.292Z" }, +] [[package]] name = "bluesky" @@ -951,7 +955,7 @@ dependencies = [ { name = "blueapi" }, { name = "fastapi" }, { name = "pydantic" }, - { name = "tiled" }, + { name = "tiled", extra = ["client"] }, ] [package.dev-dependencies] @@ -977,10 +981,10 @@ dev = [ [package.metadata] requires-dist = [ - { name = "blueapi", git = "https://github.com/DiamondLightSource/blueapi.git?rev=308f55f43f0623d36101d124ab9e85b9e7532568" }, + { name = "blueapi", specifier = ">=1.18.1" }, { name = "fastapi", specifier = ">=0.136.0" }, { name = "pydantic", specifier = ">=2.13.2" }, - { name = "tiled", specifier = ">=0.2.9" }, + { name = "tiled", extras = ["client"], specifier = ">=0.2.9" }, ] [package.metadata.requires-dev] From ca344b8c2f55c2500d9807c5bfafbaac3d9e0ca4 Mon Sep 17 00:00:00 2001 From: Jacob Williamson Date: Tue, 18 Aug 2026 17:15:39 +0100 Subject: [PATCH 22/22] Lock down get_config endpoint --- src/daq_queuing_service/api/api.py | 32 ++++++++++++++---------------- src/daq_queuing_service/app/app.py | 9 ++++----- tests/unit_tests/test_api.py | 9 +-------- 3 files changed, 20 insertions(+), 30 deletions(-) diff --git a/src/daq_queuing_service/api/api.py b/src/daq_queuing_service/api/api.py index e9799db..cd8573e 100644 --- a/src/daq_queuing_service/api/api.py +++ b/src/daq_queuing_service/api/api.py @@ -1,6 +1,6 @@ import asyncio import json -from collections.abc import AsyncGenerator, Callable +from collections.abc import AsyncGenerator from typing import Annotated from blueapi.service.model import TaskRequest @@ -76,31 +76,29 @@ def protected_routes( broadcaster: Broadcaster[QUEUE_EVENTS], config: AppConfig, converter: Converter, - whitelist_check: Callable[[User], User] | None = None, ) -> APIRouter: """Authentication is required to access these endpoints (if turned on in config). Additionally, for endpoints that depend on whitelist_check, you must be in the whitelist of authorised fedIDs to access them. """ - authorised = [Depends(whitelist_check)] if whitelist_check else None router = APIRouter() @router.get("/config") def get_config() -> AppConfig: return config - @router.patch("/queue/state", dependencies=authorised) + @router.patch("/queue/state") async def update_queue_state(payload: QueueStateUpdate) -> QueueState: if payload.paused: return await queue.pause_queue(PauseReason.USER_REQUESTED) else: return await queue.resume_queue() - @router.get("/queue", dependencies=authorised) + @router.get("/queue") async def get_queued_tasks(status: Status | None = None) -> list[TaskWithPosition]: return _filter_by_status(await queue.get_queue(), status) - @router.post("/queue", dependencies=authorised) + @router.post("/queue") async def add_tasks_to_queue( experiments: list[TaskRequest | Experiment], user: CurrentUser, @@ -116,49 +114,49 @@ async def add_tasks_to_queue( await queue.add_tasks(tasks, position) return task_ids - @router.delete("/queue", dependencies=authorised) + @router.delete("/queue") async def cancel_all_tasks() -> list[TaskWithPosition]: return await queue.cancel_all_tasks() - @router.post("/queue/move", dependencies=authorised) + @router.post("/queue/move") async def move_task(task_id: str, new_position: int) -> int: return await queue.move_task(task_id, new_position) - @router.delete("/queue/tasks", dependencies=authorised) + @router.delete("/queue/tasks") async def cancel_tasks(payload: TaskCancelRequest) -> list[TaskWithPosition]: return await queue.cancel_tasks(payload.task_ids) - @router.get("/queue/{position}", dependencies=authorised) + @router.get("/queue/{position}") async def get_task_by_position(position: int) -> TaskWithPosition | None: return await queue.get_task_by_position(position) - @router.get("/tasks", dependencies=authorised) + @router.get("/tasks") async def get_all_tasks(status: Status | None = None) -> list[TaskWithPosition]: return _filter_by_status(await queue.get_tasks(), status) - @router.get("/tasks/{task_id}", dependencies=authorised) + @router.get("/tasks/{task_id}") async def get_task_by_id(task_id: str) -> TaskWithPosition: return await queue.get_task_by_id(task_id) - @router.get("/history", dependencies=authorised) + @router.get("/history") async def get_completed_tasks( status: Status | None = None, ) -> list[TaskWithPosition]: return _filter_by_status(await queue.get_history(), status) - @router.delete("/history", dependencies=authorised) + @router.delete("/history") async def clear_history(): return await queue.clear_history() - @router.get("/call_queue", dependencies=authorised) + @router.get("/call_queue") async def get_call_queue() -> list[BlueapiCallResponse]: return await queue.get_call_queue() - @router.get("/call_history", dependencies=authorised) + @router.get("/call_history") async def get_call_history() -> list[BlueapiCallResponse]: return await queue.get_call_history() - @router.get("/events", dependencies=authorised) + @router.get("/events") async def stream_events() -> EventSourceResponse: subscriber = broadcaster.subscribe() diff --git a/src/daq_queuing_service/app/app.py b/src/daq_queuing_service/app/app.py index 1f6250f..86b0d79 100644 --- a/src/daq_queuing_service/app/app.py +++ b/src/daq_queuing_service/app/app.py @@ -73,12 +73,13 @@ def log_task_exception(task: asyncio.Task[NoReturn]): "clientId": "NOT_SUPPORTED", } - dependencies.append(Depends(get_current_user)) - whitelist_check = build_ensure_current_user_is_in_whitelist( config.authorisation_whitelist, get_current_user ) + dependencies.append(Depends(get_current_user)) + dependencies.append(Depends(whitelist_check)) + if dev: # Allows local client/UI through CORS app.add_middleware( CORSMiddleware, @@ -101,9 +102,7 @@ def log_task_exception(task: asyncio.Task[NoReturn]): register_exception_handlers(app) app.include_router(public_routes(app.state.queue)) app.include_router( - protected_routes( - app.state.queue, broadcaster, config, converter, whitelist_check - ), + protected_routes(app.state.queue, broadcaster, config, converter), dependencies=dependencies, ) diff --git a/tests/unit_tests/test_api.py b/tests/unit_tests/test_api.py index a312edd..b3296c3 100644 --- a/tests/unit_tests/test_api.py +++ b/tests/unit_tests/test_api.py @@ -1020,6 +1020,7 @@ def test_public_endpoints_not_blocked_by_auth( @pytest.mark.parametrize( "endpoint, method", [ + ("/config", "get"), ("/queue/state", "patch"), ("/queue", "get"), ("/queue", "post"), @@ -1046,14 +1047,6 @@ def test_endpoints_blocked_by_authorisation_check_if_user_not_in_whitelist( } -@pytest.mark.parametrize("endpoint, method", [("/config", "get")]) -def test_endpoints_that_require_authn_but_not_authz_any_user_allowed( - endpoint: str, method: str, test_client_with_authz: TestClient -): - response: httpx.Response = getattr(test_client_with_authz, method)(endpoint) - assert response.status_code == 200 - - def test_get_current_user_returns_user_from_request_state(): request = MagicMock() user = User(fedid="abc12345")