From ccdc678ae8dcf101093e6e784b9756c5a07396e4 Mon Sep 17 00:00:00 2001 From: Peter Holloway Date: Tue, 26 May 2026 12:01:54 +0100 Subject: [PATCH 01/31] Add opa dependency function to create OpaUserClient --- src/blueapi/service/authorization.py | 17 +++++++++++++++-- 1 file changed, 15 insertions(+), 2 deletions(-) diff --git a/src/blueapi/service/authorization.py b/src/blueapi/service/authorization.py index a4a7b5c98..c4f72ec45 100644 --- a/src/blueapi/service/authorization.py +++ b/src/blueapi/service/authorization.py @@ -1,12 +1,14 @@ import logging from collections.abc import Mapping from contextlib import AbstractAsyncContextManager, aclosing, nullcontext -from typing import Any, Self +from typing import Any, Self, cast from aiohttp import ClientSession +from fastapi import Depends, HTTPException, Request +from starlette import status from blueapi.config import OIDCConfig, OpaConfig, ServiceAccount -from blueapi.service.authentication import TiledAuth +from blueapi.service.authentication import TiledAuth, unchecked_bearer_token LOGGER = logging.getLogger(__name__) @@ -72,3 +74,14 @@ async def validate_tiled_config( tiled.token_url = oidc.token_endpoint auth = TiledAuth(tiled) await opa.require_tiled_service_account(auth.get_access_token()) + + +async def opa( + request: Request, token: str | None = Depends(unchecked_bearer_token) +) -> OpaUserClient | None: + + if opa := cast(OpaClient | None, getattr(request.app.state, "authz", None)): + if not token: + raise HTTPException(status_code=status.HTTP_401_UNAUTHORIZED) + return opa.for_token(token) + return None From f79a14975776fd7c529935f49da549b3d801100b Mon Sep 17 00:00:00 2001 From: Peter Holloway Date: Thu, 28 May 2026 17:24:54 +0100 Subject: [PATCH 02/31] test opa dependency function --- src/blueapi/service/authorization.py | 2 +- .../unit_tests/service/test_authorization.py | 27 +++++++++++++++++++ 2 files changed, 28 insertions(+), 1 deletion(-) diff --git a/src/blueapi/service/authorization.py b/src/blueapi/service/authorization.py index c4f72ec45..dbdc1f685 100644 --- a/src/blueapi/service/authorization.py +++ b/src/blueapi/service/authorization.py @@ -83,5 +83,5 @@ async def opa( if opa := cast(OpaClient | None, getattr(request.app.state, "authz", None)): if not token: raise HTTPException(status_code=status.HTTP_401_UNAUTHORIZED) - return opa.for_token(token) + return OpaUserClient(opa, token) return None diff --git a/tests/unit_tests/service/test_authorization.py b/tests/unit_tests/service/test_authorization.py index 249198580..ff3949174 100644 --- a/tests/unit_tests/service/test_authorization.py +++ b/tests/unit_tests/service/test_authorization.py @@ -2,11 +2,13 @@ from unittest.mock import AsyncMock, MagicMock, Mock, patch import pytest +from fastapi import HTTPException from pydantic import HttpUrl from blueapi.config import OIDCConfig, OpaConfig, ServiceAccount from blueapi.service.authorization import ( OpaClient, + opa, validate_tiled_config, ) @@ -149,3 +151,28 @@ async def test_validate_tiled_config_with_missing_config( assert await validate_tiled_config(tiled_auth, oidc, opa_client) is None if opa_client is not None: opa_client.require_tiled_service_account.assert_not_called() + + +async def test_opa_dependency_method(): + request = MagicMock() + + user_client = await opa(request, "foo_bar") + + assert user_client is not None + assert user_client.client == request.app.state.authz + assert user_client.token == "foo_bar" + + +async def test_opa_dependency_without_token(): + request = MagicMock() + + with pytest.raises(HTTPException, match="401"): + await opa(request, None) + + +@pytest.mark.parametrize("token", ["foo_bar", None]) +async def test_opa_dependency_without_authz(token): + request = MagicMock() + del request.app.state.authz + user_client = await opa(request, token) + assert user_client is None From 9ac00cc5fc2d79d1d8946cede66ab94ed2f8ebe5 Mon Sep 17 00:00:00 2001 From: Peter Holloway Date: Tue, 26 May 2026 12:17:35 +0100 Subject: [PATCH 03/31] Add can_submit_task auth check method and config --- helm/blueapi/config_schema.json | 7 ++++- helm/blueapi/values.schema.json | 7 ++++- src/blueapi/config.py | 1 + src/blueapi/service/authorization.py | 30 +++++++++++++++++++ .../unit_tests/service/test_authorization.py | 1 + tests/unit_tests/test_config.py | 1 + 6 files changed, 45 insertions(+), 2 deletions(-) diff --git a/helm/blueapi/config_schema.json b/helm/blueapi/config_schema.json index 2c43910a4..d64f3ce6b 100644 --- a/helm/blueapi/config_schema.json +++ b/helm/blueapi/config_schema.json @@ -303,10 +303,15 @@ "tiled_service_account_check": { "title": "Tiled Service Account Check", "type": "string" + }, + "submit_task_check": { + "title": "Submit Task Check", + "type": "string" } }, "required": [ - "tiled_service_account_check" + "tiled_service_account_check", + "submit_task_check" ], "title": "OpaConfig", "type": "object", diff --git a/helm/blueapi/values.schema.json b/helm/blueapi/values.schema.json index 20bdf376d..1cb719910 100644 --- a/helm/blueapi/values.schema.json +++ b/helm/blueapi/values.schema.json @@ -711,7 +711,8 @@ "title": "OpaConfig", "type": "object", "required": [ - "tiled_service_account_check" + "tiled_service_account_check", + "submit_task_check" ], "properties": { "audience": { @@ -727,6 +728,10 @@ "maxLength": 2083, "minLength": 1 }, + "submit_task_check": { + "title": "Submit Task Check", + "type": "string" + }, "tiled_service_account_check": { "title": "Tiled Service Account Check", "type": "string" diff --git a/src/blueapi/config.py b/src/blueapi/config.py index 29cf242a9..5b2b4453d 100644 --- a/src/blueapi/config.py +++ b/src/blueapi/config.py @@ -313,6 +313,7 @@ class OpaConfig(BlueapiBaseModel): root: HttpUrl = HttpUrl("http://localhost:8181") audience: str = "account" tiled_service_account_check: str + submit_task_check: str class ApplicationConfig(BlueapiBaseModel): diff --git a/src/blueapi/service/authorization.py b/src/blueapi/service/authorization.py index dbdc1f685..9cc4ea7df 100644 --- a/src/blueapi/service/authorization.py +++ b/src/blueapi/service/authorization.py @@ -1,4 +1,5 @@ import logging +import re from collections.abc import Mapping from contextlib import AbstractAsyncContextManager, aclosing, nullcontext from typing import Any, Self, cast @@ -9,8 +10,10 @@ from blueapi.config import OIDCConfig, OpaConfig, ServiceAccount from blueapi.service.authentication import TiledAuth, unchecked_bearer_token +from blueapi.service.model import TaskRequest LOGGER = logging.getLogger(__name__) +INSTRUMENT_SESSION_RE = re.compile(r"^[a-z]{2}(?P\d+)-(?P\d+)$") class OpaClient: @@ -58,6 +61,33 @@ async def require_tiled_service_account(self, token: str): f"Tiled service account is not valid for '{self._instrument}'" ) + async def require_submit_task(self, instrument_session: str, token: str): + if not (match := INSTRUMENT_SESSION_RE.match(instrument_session)): + raise ValueError("Invalid instrument session") + + if not await self._call_opa( + self._conf.submit_task_check, + { + "token": token, + "proposal": int(match["proposal"]), + "visit": int(match["visit"]), + }, + ): + raise HTTPException(status_code=status.HTTP_403_UNORTHORIZED) + + +class OpaUserClient: + client: OpaClient + token: str + + def __init__(self, client: OpaClient, token: str): + self.client = client + self.token = token + + async def can_submit_task(self, task: TaskRequest): + LOGGER.info("Checking permissions to run task") + await self.client.require_submit_task(task.instrument_session, self.token) + async def validate_tiled_config( tiled: ServiceAccount | str | None, oidc: OIDCConfig | None, opa: OpaClient | None diff --git a/tests/unit_tests/service/test_authorization.py b/tests/unit_tests/service/test_authorization.py index ff3949174..37c1d7e3f 100644 --- a/tests/unit_tests/service/test_authorization.py +++ b/tests/unit_tests/service/test_authorization.py @@ -24,6 +24,7 @@ def opa_config() -> OpaConfig: return OpaConfig( root=HttpUrl("http://auth.example.com"), + submit_task_check="/auth/submit", tiled_service_account_check="/auth/tiled", ) diff --git a/tests/unit_tests/test_config.py b/tests/unit_tests/test_config.py index bd136d477..999e01d23 100644 --- a/tests/unit_tests/test_config.py +++ b/tests/unit_tests/test_config.py @@ -344,6 +344,7 @@ def test_config_yaml_parsed(temp_yaml_config_file): "root": "http://opa.example.com/", "audience": "account", "tiled_service_account_check": "v1/tiled_service_account", + "submit_task_check": "v1/submit_task", }, }, { From ac5af8c80eb2fdc0f6855ed4390874dba87cbd44 Mon Sep 17 00:00:00 2001 From: root Date: Fri, 15 May 2026 08:27:14 +0000 Subject: [PATCH 04/31] feat: add authz dependency injection --- src/blueapi/service/main.py | 10 +++++++++- 1 file changed, 9 insertions(+), 1 deletion(-) diff --git a/src/blueapi/service/main.py b/src/blueapi/service/main.py index 432cc5455..43562fbfe 100644 --- a/src/blueapi/service/main.py +++ b/src/blueapi/service/main.py @@ -41,7 +41,7 @@ from blueapi.worker import TrackableTask, WorkerState from blueapi.worker.event import TaskStatusEnum -from .authorization import OpaClient, validate_tiled_config +from .authorization import OpaClient, OpaUserClient, opa, validate_tiled_config from .model import ( DeviceModel, DeviceResponse, @@ -260,6 +260,13 @@ def get_device_by_name( ) +async def submission_check( + opa: Annotated[OpaUserClient, Depends(opa)], + task_request: TaskRequest, +): + await opa.can_submit_task(task_request) + + @secure_router_v1.post("/tasks", status_code=status.HTTP_201_CREATED, tags=[Tag.TASK]) @secure_router.post("/tasks", status_code=status.HTTP_201_CREATED, tags=[Tag.TASK]) @start_as_current_span( @@ -273,6 +280,7 @@ def submit_task( request: Request, response: Response, task_request: Annotated[TaskRequest, Body(..., examples=[example_task_request])], + authz_check: Annotated[None, Depends(submission_check)], runner: Annotated[WorkerDispatcher, Depends(_runner)], user: Fedid, ) -> TaskResponse: From 663ae17a82c9a83d9398fa92f095ef63d1b63f1f Mon Sep 17 00:00:00 2001 From: root Date: Mon, 18 May 2026 15:01:34 +0000 Subject: [PATCH 05/31] feat: add auth check dependency injections to task endpoints --- src/blueapi/service/authorization.py | 8 +++++++- src/blueapi/service/main.py | 16 +++++++--------- 2 files changed, 14 insertions(+), 10 deletions(-) diff --git a/src/blueapi/service/authorization.py b/src/blueapi/service/authorization.py index 9cc4ea7df..9d9c96ddd 100644 --- a/src/blueapi/service/authorization.py +++ b/src/blueapi/service/authorization.py @@ -2,7 +2,7 @@ import re from collections.abc import Mapping from contextlib import AbstractAsyncContextManager, aclosing, nullcontext -from typing import Any, Self, cast +from typing import Annotated, Any, Self, cast from aiohttp import ClientSession from fastapi import Depends, HTTPException, Request @@ -115,3 +115,9 @@ async def opa( raise HTTPException(status_code=status.HTTP_401_UNAUTHORIZED) return OpaUserClient(opa, token) return None + +async def submit_permission( + opa: Annotated[OpaUserClient, Depends(opa)], + task_request: TaskRequest, +): + await opa.can_submit_task(task_request) diff --git a/src/blueapi/service/main.py b/src/blueapi/service/main.py index 43562fbfe..16d1e984b 100644 --- a/src/blueapi/service/main.py +++ b/src/blueapi/service/main.py @@ -41,7 +41,7 @@ from blueapi.worker import TrackableTask, WorkerState from blueapi.worker.event import TaskStatusEnum -from .authorization import OpaClient, OpaUserClient, opa, validate_tiled_config +from .authorization import OpaClient, submit_permission, validate_tiled_config from .model import ( DeviceModel, DeviceResponse, @@ -260,13 +260,6 @@ def get_device_by_name( ) -async def submission_check( - opa: Annotated[OpaUserClient, Depends(opa)], - task_request: TaskRequest, -): - await opa.can_submit_task(task_request) - - @secure_router_v1.post("/tasks", status_code=status.HTTP_201_CREATED, tags=[Tag.TASK]) @secure_router.post("/tasks", status_code=status.HTTP_201_CREATED, tags=[Tag.TASK]) @start_as_current_span( @@ -280,7 +273,7 @@ def submit_task( request: Request, response: Response, task_request: Annotated[TaskRequest, Body(..., examples=[example_task_request])], - authz_check: Annotated[None, Depends(submission_check)], + _: Annotated[None, Depends(submit_permission)], runner: Annotated[WorkerDispatcher, Depends(_runner)], user: Fedid, ) -> TaskResponse: @@ -319,6 +312,7 @@ def submit_task( @start_as_current_span(TRACER, "task_id") def delete_submitted_task( task_id: str, + _: Annotated[None, Depends(submit_permission)], runner: Annotated[WorkerDispatcher, Depends(_runner)], ) -> TaskResponse: return TaskResponse(task_id=runner.run(interface.clear_task, task_id)) @@ -337,6 +331,7 @@ def validate_task_status(v: str) -> TaskStatusEnum: @start_as_current_span(TRACER) def get_tasks( runner: Annotated[WorkerDispatcher, Depends(_runner)], + _: Annotated[None, Depends(submit_permission)], task_status: str | SkipJsonSchema[None] = None, ) -> TasksListResponse: """ @@ -373,6 +368,7 @@ def get_tasks( def set_active_task( request: Request, task: WorkerTask, + _: Annotated[None, Depends(submit_permission)], runner: Annotated[WorkerDispatcher, Depends(_runner)], ) -> WorkerTask: """Set a task to active status, the worker should begin it as soon as possible. @@ -403,6 +399,7 @@ def get_passthrough_headers(request: Request) -> dict[str, str]: @start_as_current_span(TRACER, "task_id") def get_task( task_id: str, + _: Annotated[None, Depends(submit_permission)], runner: Annotated[WorkerDispatcher, Depends(_runner)], ) -> TrackableTask: """Retrieve a task""" @@ -480,6 +477,7 @@ def get_state(runner: Annotated[WorkerDispatcher, Depends(_runner)]) -> WorkerSt def set_state( state_change_request: StateChangeRequest, response: Response, + _: Annotated[None, Depends(submit_permission)], runner: Annotated[WorkerDispatcher, Depends(_runner)], ) -> WorkerState: """ From a63192fcaae7e130d12f21f57b874d64b25e2e2f Mon Sep 17 00:00:00 2001 From: root Date: Wed, 20 May 2026 08:13:42 +0000 Subject: [PATCH 06/31] feat: create new access task permission fns and add as dependencies --- src/blueapi/service/main.py | 57 +++++++++++++++++++++++++++++++++---- 1 file changed, 52 insertions(+), 5 deletions(-) diff --git a/src/blueapi/service/main.py b/src/blueapi/service/main.py index 16d1e984b..7e9c2f017 100644 --- a/src/blueapi/service/main.py +++ b/src/blueapi/service/main.py @@ -148,6 +148,41 @@ def get_app(config: ApplicationConfig): return app +def access_task_permission( + request: Request, + task_id: str, + runner: Annotated[WorkerDispatcher, Depends(_runner)], +): + access_token: dict[str, Any] | None = getattr( + request.state, "decoded_access_token", None + ) + try: + task = runner.run(interface.get_task_by_id, task_id) + except KeyError: + raise HTTPException(status_code=status.HTTP_401_UNAUTHORIZED) from None + + if ( + access_token + and task + and access_token.get("fedid") != task.task.metadata.get("user") + ): + raise HTTPException(status_code=status.HTTP_401_UNAUTHORIZED) + + +# start_task_permission is used when there is WorkerTask +def start_task_permission( + request: Request, + task: WorkerTask, + runner: Annotated[WorkerDispatcher, Depends(_runner)], +): + if not task.task_id: + raise HTTPException( + status_code=status.HTTP_422_UNPROCESSABLE_CONTENT, + detail="No task id provided", + ) + access_task_permission(request, task.task_id, runner) + + async def on_key_error_404(_: Request, __: Exception): return JSONResponse( status_code=status.HTTP_404_NOT_FOUND, @@ -312,7 +347,7 @@ def submit_task( @start_as_current_span(TRACER, "task_id") def delete_submitted_task( task_id: str, - _: Annotated[None, Depends(submit_permission)], + _: Annotated[None, Depends(access_task_permission)], runner: Annotated[WorkerDispatcher, Depends(_runner)], ) -> TaskResponse: return TaskResponse(task_id=runner.run(interface.clear_task, task_id)) @@ -330,8 +365,8 @@ def validate_task_status(v: str) -> TaskStatusEnum: @secure_router.get("/tasks", status_code=status.HTTP_200_OK, tags=[Tag.TASK]) @start_as_current_span(TRACER) def get_tasks( + request: Request, runner: Annotated[WorkerDispatcher, Depends(_runner)], - _: Annotated[None, Depends(submit_permission)], task_status: str | SkipJsonSchema[None] = None, ) -> TasksListResponse: """ @@ -351,6 +386,15 @@ def get_tasks( tasks = runner.run(interface.get_tasks_by_status, desired_status) else: tasks = runner.run(interface.get_tasks) + + access_token: dict[str, Any] | None = getattr( + request.state, "decoded_access_token", None + ) + user = access_token.get("fedid") if access_token else None + + if user: + tasks = [t for t in tasks if t.task.metadata.get("user") == user] + return TasksListResponse(tasks=tasks) @@ -368,7 +412,7 @@ def get_tasks( def set_active_task( request: Request, task: WorkerTask, - _: Annotated[None, Depends(submit_permission)], + _: Annotated[None, Depends(start_task_permission)], runner: Annotated[WorkerDispatcher, Depends(_runner)], ) -> WorkerTask: """Set a task to active status, the worker should begin it as soon as possible. @@ -399,7 +443,7 @@ def get_passthrough_headers(request: Request) -> dict[str, str]: @start_as_current_span(TRACER, "task_id") def get_task( task_id: str, - _: Annotated[None, Depends(submit_permission)], + _: Annotated[None, Depends(access_task_permission)], runner: Annotated[WorkerDispatcher, Depends(_runner)], ) -> TrackableTask: """Retrieve a task""" @@ -477,7 +521,7 @@ def get_state(runner: Annotated[WorkerDispatcher, Depends(_runner)]) -> WorkerSt def set_state( state_change_request: StateChangeRequest, response: Response, - _: Annotated[None, Depends(submit_permission)], + _: Annotated[None, Depends(access_task_permission)], runner: Annotated[WorkerDispatcher, Depends(_runner)], ) -> WorkerState: """ @@ -509,6 +553,9 @@ def set_state( elif new_state == WorkerState.RUNNING: runner.run(interface.resume_worker) elif new_state in {WorkerState.ABORTING, WorkerState.STOPPING}: + # active = runner.run(interface.get_active_task) + # if active.task.metadata.get("user"): + try: runner.run( interface.cancel_active_task, From c106db28a789a8fb73257375123202f0aa35964b Mon Sep 17 00:00:00 2001 From: root Date: Wed, 20 May 2026 08:30:53 +0000 Subject: [PATCH 07/31] refactor: update rest api version --- docs/reference/openapi.yaml | 2 +- src/blueapi/config.py | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/docs/reference/openapi.yaml b/docs/reference/openapi.yaml index 94a1f1540..68c538342 100644 --- a/docs/reference/openapi.yaml +++ b/docs/reference/openapi.yaml @@ -441,7 +441,7 @@ info: name: Apache 2.0 url: https://www.apache.org/licenses/LICENSE-2.0.html title: BlueAPI Control - version: 1.4.0 + version: 1.4.1 openapi: 3.1.0 paths: /api/v1/devices: diff --git a/src/blueapi/config.py b/src/blueapi/config.py index 5b2b4453d..b9a71db31 100644 --- a/src/blueapi/config.py +++ b/src/blueapi/config.py @@ -323,7 +323,7 @@ class ApplicationConfig(BlueapiBaseModel): """ #: API version to publish in OpenAPI schema - REST_API_VERSION: ClassVar[str] = "1.4.0" + REST_API_VERSION: ClassVar[str] = "1.4.1" LICENSE_INFO: ClassVar[dict[str, str]] = { "name": "Apache 2.0", From 0eba9ce66079714384b13fbabb2353e3c50e5dd5 Mon Sep 17 00:00:00 2001 From: root Date: Wed, 20 May 2026 10:13:46 +0000 Subject: [PATCH 08/31] comment out dependency addition in set_state --- src/blueapi/service/main.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/blueapi/service/main.py b/src/blueapi/service/main.py index 7e9c2f017..bc593ef1e 100644 --- a/src/blueapi/service/main.py +++ b/src/blueapi/service/main.py @@ -521,7 +521,7 @@ def get_state(runner: Annotated[WorkerDispatcher, Depends(_runner)]) -> WorkerSt def set_state( state_change_request: StateChangeRequest, response: Response, - _: Annotated[None, Depends(access_task_permission)], + # _: Annotated[None, Depends(access_task_permission)], runner: Annotated[WorkerDispatcher, Depends(_runner)], ) -> WorkerState: """ From 659872b30f12b22aa3eaae3bb8a09b8b8d8f18fc Mon Sep 17 00:00:00 2001 From: root Date: Wed, 20 May 2026 14:16:24 +0000 Subject: [PATCH 09/31] refactor: add admin check and check to set state function --- src/blueapi/service/authentication.py | 3 +++ src/blueapi/service/main.py | 30 ++++++++++++++--------- tests/unit_tests/service/test_rest_api.py | 2 +- 3 files changed, 22 insertions(+), 13 deletions(-) diff --git a/src/blueapi/service/authentication.py b/src/blueapi/service/authentication.py index 6761256de..fd1a8a5f1 100644 --- a/src/blueapi/service/authentication.py +++ b/src/blueapi/service/authentication.py @@ -290,6 +290,9 @@ def unchecked_bearer_token(req: Request) -> str | None: return None return param.strip() + def admin(self): + return False + UncheckedBearerToken = Annotated[str | None, Depends(unchecked_bearer_token)] diff --git a/src/blueapi/service/main.py b/src/blueapi/service/main.py index bc593ef1e..68415f031 100644 --- a/src/blueapi/service/main.py +++ b/src/blueapi/service/main.py @@ -149,6 +149,7 @@ def get_app(config: ApplicationConfig): def access_task_permission( + opa: Annotated[OPAClient, Depends(get_opa_client)], request: Request, task_id: str, runner: Annotated[WorkerDispatcher, Depends(_runner)], @@ -156,21 +157,19 @@ def access_task_permission( access_token: dict[str, Any] | None = getattr( request.state, "decoded_access_token", None ) - try: - task = runner.run(interface.get_task_by_id, task_id) - except KeyError: - raise HTTPException(status_code=status.HTTP_401_UNAUTHORIZED) from None + task = runner.run(interface.get_task_by_id, task_id) - if ( + if not opa.admin() and ( access_token and task and access_token.get("fedid") != task.task.metadata.get("user") ): - raise HTTPException(status_code=status.HTTP_401_UNAUTHORIZED) + raise HTTPException(status_code=status.HTTP_404_NOT_FOUND) # start_task_permission is used when there is WorkerTask def start_task_permission( + opa: Annotated[OPAClient, Depends(get_opa_client)], request: Request, task: WorkerTask, runner: Annotated[WorkerDispatcher, Depends(_runner)], @@ -180,7 +179,7 @@ def start_task_permission( status_code=status.HTTP_422_UNPROCESSABLE_CONTENT, detail="No task id provided", ) - access_task_permission(request, task.task_id, runner) + access_task_permission(opa, request, task.task_id, runner) async def on_key_error_404(_: Request, __: Exception): @@ -392,8 +391,7 @@ def get_tasks( ) user = access_token.get("fedid") if access_token else None - if user: - tasks = [t for t in tasks if t.task.metadata.get("user") == user] + tasks = [t for t in tasks if t.task.metadata.get("user") == user] return TasksListResponse(tasks=tasks) @@ -519,8 +517,10 @@ def get_state(runner: Annotated[WorkerDispatcher, Depends(_runner)]) -> WorkerSt ) @start_as_current_span(TRACER, "state_change_request.new_state") def set_state( + request: Request, state_change_request: StateChangeRequest, response: Response, + opa: Annotated[OPAClient, Depends(get_opa_client)], # _: Annotated[None, Depends(access_task_permission)], runner: Annotated[WorkerDispatcher, Depends(_runner)], ) -> WorkerState: @@ -548,14 +548,20 @@ def set_state( current_state in _ALLOWED_TRANSITIONS and new_state in _ALLOWED_TRANSITIONS[current_state] ): + active = runner.run(interface.get_active_task) + access_token: dict[str, Any] | None = getattr( + request.state, "decoded_access_token", None + ) + user = access_token.get("fedid") if access_token else None + + if not opa.admin() and active and active.task.metadata.get("user") != user: + raise HTTPException(status_code=status.HTTP_403_FORBIDDEN) + if new_state == WorkerState.PAUSED: runner.run(interface.pause_worker, state_change_request.defer) elif new_state == WorkerState.RUNNING: runner.run(interface.resume_worker) elif new_state in {WorkerState.ABORTING, WorkerState.STOPPING}: - # active = runner.run(interface.get_active_task) - # if active.task.metadata.get("user"): - try: runner.run( interface.cancel_active_task, diff --git a/tests/unit_tests/service/test_rest_api.py b/tests/unit_tests/service/test_rest_api.py index 1ddf2c6ca..14439f111 100644 --- a/tests/unit_tests/service/test_rest_api.py +++ b/tests/unit_tests/service/test_rest_api.py @@ -251,7 +251,7 @@ def test_create_task(mock_runner: Mock, client: TestClient) -> None: response = client.post("/tasks", json=task.model_dump()) - mock_runner.run.assert_called_with(submit_task, task, {"user": "Unknown"}) + mock_runner.run.assert_called_with(submit_task, task, {"user": None}) assert response.json() == {"task_id": task_id} From cae87e904311fcd52c021612e63d63aa89d267a4 Mon Sep 17 00:00:00 2001 From: Peter Holloway Date: Tue, 26 May 2026 12:34:41 +0100 Subject: [PATCH 10/31] Update dependency names --- src/blueapi/service/main.py | 14 ++++++++++---- 1 file changed, 10 insertions(+), 4 deletions(-) diff --git a/src/blueapi/service/main.py b/src/blueapi/service/main.py index 68415f031..eef5149d5 100644 --- a/src/blueapi/service/main.py +++ b/src/blueapi/service/main.py @@ -41,7 +41,13 @@ from blueapi.worker import TrackableTask, WorkerState from blueapi.worker.event import TaskStatusEnum -from .authorization import OpaClient, submit_permission, validate_tiled_config +from .authorization import ( + OpaClient, + OpaUserClient, + opa, + submit_permission, + validate_tiled_config, +) from .model import ( DeviceModel, DeviceResponse, @@ -149,7 +155,7 @@ def get_app(config: ApplicationConfig): def access_task_permission( - opa: Annotated[OPAClient, Depends(get_opa_client)], + opa: Annotated[OpaUserClient, Depends(opa)], request: Request, task_id: str, runner: Annotated[WorkerDispatcher, Depends(_runner)], @@ -169,7 +175,7 @@ def access_task_permission( # start_task_permission is used when there is WorkerTask def start_task_permission( - opa: Annotated[OPAClient, Depends(get_opa_client)], + opa: Annotated[OpaUserClient, Depends(opa)], request: Request, task: WorkerTask, runner: Annotated[WorkerDispatcher, Depends(_runner)], @@ -520,7 +526,7 @@ def set_state( request: Request, state_change_request: StateChangeRequest, response: Response, - opa: Annotated[OPAClient, Depends(get_opa_client)], + opa: Annotated[OpaUserClient, Depends(opa)], # _: Annotated[None, Depends(access_task_permission)], runner: Annotated[WorkerDispatcher, Depends(_runner)], ) -> WorkerState: From df893592db17c5e8245d9f10447c07a467ff1f1c Mon Sep 17 00:00:00 2001 From: Peter Holloway Date: Tue, 26 May 2026 12:53:37 +0100 Subject: [PATCH 11/31] Add missing admin check --- helm/blueapi/config_schema.json | 7 ++++++- helm/blueapi/values.schema.json | 7 ++++++- src/blueapi/config.py | 1 + src/blueapi/service/authorization.py | 7 +++++++ tests/unit_tests/test_config.py | 1 + 5 files changed, 21 insertions(+), 2 deletions(-) diff --git a/helm/blueapi/config_schema.json b/helm/blueapi/config_schema.json index d64f3ce6b..3b9d03138 100644 --- a/helm/blueapi/config_schema.json +++ b/helm/blueapi/config_schema.json @@ -307,11 +307,16 @@ "submit_task_check": { "title": "Submit Task Check", "type": "string" + }, + "admin_check": { + "title": "Admin Check", + "type": "string" } }, "required": [ "tiled_service_account_check", - "submit_task_check" + "submit_task_check", + "admin_check" ], "title": "OpaConfig", "type": "object", diff --git a/helm/blueapi/values.schema.json b/helm/blueapi/values.schema.json index 1cb719910..c16ca03dc 100644 --- a/helm/blueapi/values.schema.json +++ b/helm/blueapi/values.schema.json @@ -712,9 +712,14 @@ "type": "object", "required": [ "tiled_service_account_check", - "submit_task_check" + "submit_task_check", + "admin_check" ], "properties": { + "admin_check": { + "title": "Admin Check", + "type": "string" + }, "audience": { "title": "Audience", "default": "account", diff --git a/src/blueapi/config.py b/src/blueapi/config.py index b9a71db31..9367c9f88 100644 --- a/src/blueapi/config.py +++ b/src/blueapi/config.py @@ -314,6 +314,7 @@ class OpaConfig(BlueapiBaseModel): audience: str = "account" tiled_service_account_check: str submit_task_check: str + admin_check: str class ApplicationConfig(BlueapiBaseModel): diff --git a/src/blueapi/service/authorization.py b/src/blueapi/service/authorization.py index 9d9c96ddd..c84a7d1aa 100644 --- a/src/blueapi/service/authorization.py +++ b/src/blueapi/service/authorization.py @@ -75,6 +75,9 @@ async def require_submit_task(self, instrument_session: str, token: str): ): raise HTTPException(status_code=status.HTTP_403_UNORTHORIZED) + async def is_admin(self, token: str) -> bool: + return await self._call_opa(self._conf.admin_check, {"token": token}) + class OpaUserClient: client: OpaClient @@ -88,6 +91,9 @@ async def can_submit_task(self, task: TaskRequest): LOGGER.info("Checking permissions to run task") await self.client.require_submit_task(task.instrument_session, self.token) + async def admin(self) -> bool: + return await self.client.is_admin(self.token) + async def validate_tiled_config( tiled: ServiceAccount | str | None, oidc: OIDCConfig | None, opa: OpaClient | None @@ -116,6 +122,7 @@ async def opa( return OpaUserClient(opa, token) return None + async def submit_permission( opa: Annotated[OpaUserClient, Depends(opa)], task_request: TaskRequest, diff --git a/tests/unit_tests/test_config.py b/tests/unit_tests/test_config.py index 999e01d23..ed00587a1 100644 --- a/tests/unit_tests/test_config.py +++ b/tests/unit_tests/test_config.py @@ -345,6 +345,7 @@ def test_config_yaml_parsed(temp_yaml_config_file): "audience": "account", "tiled_service_account_check": "v1/tiled_service_account", "submit_task_check": "v1/submit_task", + "admin_check": "v1/admin_check", }, }, { From a63d9c1350b29b231afac0f86e37f8fdc44c9e72 Mon Sep 17 00:00:00 2001 From: Peter Holloway Date: Tue, 26 May 2026 15:45:58 +0100 Subject: [PATCH 12/31] Handle missing opa and fix tests --- src/blueapi/service/authorization.py | 3 +- src/blueapi/service/main.py | 12 ++++++-- tests/unit_tests/service/test_rest_api.py | 34 +++++++++++++++++++---- 3 files changed, 41 insertions(+), 8 deletions(-) diff --git a/src/blueapi/service/authorization.py b/src/blueapi/service/authorization.py index c84a7d1aa..c669b627c 100644 --- a/src/blueapi/service/authorization.py +++ b/src/blueapi/service/authorization.py @@ -127,4 +127,5 @@ async def submit_permission( opa: Annotated[OpaUserClient, Depends(opa)], task_request: TaskRequest, ): - await opa.can_submit_task(task_request) + if opa: + await opa.can_submit_task(task_request) diff --git a/src/blueapi/service/main.py b/src/blueapi/service/main.py index eef5149d5..0c236f64b 100644 --- a/src/blueapi/service/main.py +++ b/src/blueapi/service/main.py @@ -155,11 +155,14 @@ def get_app(config: ApplicationConfig): def access_task_permission( - opa: Annotated[OpaUserClient, Depends(opa)], + opa: Annotated[OpaUserClient | None, Depends(opa)], request: Request, task_id: str, runner: Annotated[WorkerDispatcher, Depends(_runner)], ): + if not opa: + return + access_token: dict[str, Any] | None = getattr( request.state, "decoded_access_token", None ) @@ -560,7 +563,12 @@ def set_state( ) user = access_token.get("fedid") if access_token else None - if not opa.admin() and active and active.task.metadata.get("user") != user: + if ( + opa + and not opa.admin() + and active + and active.task.metadata.get("user") != user + ): raise HTTPException(status_code=status.HTTP_403_FORBIDDEN) if new_state == WorkerState.PAUSED: diff --git a/tests/unit_tests/service/test_rest_api.py b/tests/unit_tests/service/test_rest_api.py index 14439f111..79258fc7b 100644 --- a/tests/unit_tests/service/test_rest_api.py +++ b/tests/unit_tests/service/test_rest_api.py @@ -574,7 +574,12 @@ def test_get_state(mock_runner: Mock, client: TestClient): def test_set_state_running_to_paused(mock_runner: Mock, client: TestClient): current_state = WorkerState.RUNNING final_state = WorkerState.PAUSED - mock_runner.run.side_effect = [current_state, None, final_state] + mock_runner.run.side_effect = [ + current_state, + TrackableTask(task_id="foobar", task=Task(name="foo")), + None, + final_state, + ] response = client.put( "/worker/state", json=StateChangeRequest(new_state=final_state).model_dump() @@ -588,7 +593,12 @@ def test_set_state_running_to_paused(mock_runner: Mock, client: TestClient): def test_set_state_paused_to_running(mock_runner: Mock, client: TestClient): current_state = WorkerState.PAUSED final_state = WorkerState.RUNNING - mock_runner.run.side_effect = [current_state, None, final_state] + mock_runner.run.side_effect = [ + current_state, + TrackableTask(task_id="foobar", task=Task(name="foo")), + None, + final_state, + ] response = client.put( "/worker/state", json=StateChangeRequest(new_state=final_state).model_dump() @@ -602,7 +612,12 @@ def test_set_state_paused_to_running(mock_runner: Mock, client: TestClient): def test_set_state_running_to_aborting(mock_runner: Mock, client: TestClient): current_state = WorkerState.RUNNING final_state = WorkerState.ABORTING - mock_runner.run.side_effect = [current_state, None, final_state] + mock_runner.run.side_effect = [ + current_state, + TrackableTask(task_id="foobar", task=Task(name="foo")), + None, + final_state, + ] response = client.put( "/worker/state", json=StateChangeRequest(new_state=final_state).model_dump() @@ -619,7 +634,12 @@ def test_set_state_running_to_stopping_including_reason( current_state = WorkerState.RUNNING final_state = WorkerState.STOPPING reason = "blueapi is being stopped" - mock_runner.run.side_effect = [current_state, None, final_state] + mock_runner.run.side_effect = [ + current_state, + TrackableTask(task_id="foobar", task=Task(name="foo")), + None, + final_state, + ] response = client.put( "/worker/state", @@ -635,7 +655,11 @@ def test_set_state_transition_error(mock_runner: Mock, client: TestClient): current_state = WorkerState.RUNNING final_state = WorkerState.STOPPING - mock_runner.run.side_effect = [current_state, TransitionError()] + mock_runner.run.side_effect = [ + current_state, + TrackableTask(task_id="foobar", task=Task(name="foo")), + TransitionError(), + ] response = client.put( "/worker/state", From d1a62ee218400b55416b8cb1a370ab6266b416d8 Mon Sep 17 00:00:00 2001 From: Peter Holloway Date: Tue, 26 May 2026 15:46:43 +0100 Subject: [PATCH 13/31] Remove old admin method --- src/blueapi/service/authentication.py | 3 -- src/blueapi/service/main.py | 42 +++++++-------------------- 2 files changed, 11 insertions(+), 34 deletions(-) diff --git a/src/blueapi/service/authentication.py b/src/blueapi/service/authentication.py index fd1a8a5f1..6761256de 100644 --- a/src/blueapi/service/authentication.py +++ b/src/blueapi/service/authentication.py @@ -290,9 +290,6 @@ def unchecked_bearer_token(req: Request) -> str | None: return None return param.strip() - def admin(self): - return False - UncheckedBearerToken = Annotated[str | None, Depends(unchecked_bearer_token)] diff --git a/src/blueapi/service/main.py b/src/blueapi/service/main.py index 0c236f64b..b5c1189de 100644 --- a/src/blueapi/service/main.py +++ b/src/blueapi/service/main.py @@ -156,31 +156,21 @@ def get_app(config: ApplicationConfig): def access_task_permission( opa: Annotated[OpaUserClient | None, Depends(opa)], - request: Request, task_id: str, + fedid: Fedid, runner: Annotated[WorkerDispatcher, Depends(_runner)], ): - if not opa: - return - - access_token: dict[str, Any] | None = getattr( - request.state, "decoded_access_token", None - ) task = runner.run(interface.get_task_by_id, task_id) - if not opa.admin() and ( - access_token - and task - and access_token.get("fedid") != task.task.metadata.get("user") - ): + if opa and not opa.admin() and (task and fedid != task.task.metadata.get("user")): raise HTTPException(status_code=status.HTTP_404_NOT_FOUND) # start_task_permission is used when there is WorkerTask def start_task_permission( - opa: Annotated[OpaUserClient, Depends(opa)], - request: Request, task: WorkerTask, + opa: Annotated[OpaUserClient, Depends(opa)], + fedid: Fedid, runner: Annotated[WorkerDispatcher, Depends(_runner)], ): if not task.task_id: @@ -188,7 +178,7 @@ def start_task_permission( status_code=status.HTTP_422_UNPROCESSABLE_CONTENT, detail="No task id provided", ) - access_task_permission(opa, request, task.task_id, runner) + access_task_permission(opa, task.task_id, fedid, runner) async def on_key_error_404(_: Request, __: Exception): @@ -318,12 +308,11 @@ def submit_task( task_request: Annotated[TaskRequest, Body(..., examples=[example_task_request])], _: Annotated[None, Depends(submit_permission)], runner: Annotated[WorkerDispatcher, Depends(_runner)], - user: Fedid, + fedid: Fedid, ) -> TaskResponse: """Submit a task to the worker.""" try: - user = user or "Unknown" - task_id: str = runner.run(interface.submit_task, task_request, {"user": user}) + task_id: str = runner.run(interface.submit_task, task_request, {"user": fedid}) response.headers["Location"] = f"{request.url}/{task_id}" return TaskResponse(task_id=task_id) except ValidationError as e: @@ -373,7 +362,7 @@ def validate_task_status(v: str) -> TaskStatusEnum: @secure_router.get("/tasks", status_code=status.HTTP_200_OK, tags=[Tag.TASK]) @start_as_current_span(TRACER) def get_tasks( - request: Request, + fedid: Fedid, runner: Annotated[WorkerDispatcher, Depends(_runner)], task_status: str | SkipJsonSchema[None] = None, ) -> TasksListResponse: @@ -395,12 +384,7 @@ def get_tasks( else: tasks = runner.run(interface.get_tasks) - access_token: dict[str, Any] | None = getattr( - request.state, "decoded_access_token", None - ) - user = access_token.get("fedid") if access_token else None - - tasks = [t for t in tasks if t.task.metadata.get("user") == user] + tasks = [t for t in tasks if t.task.metadata.get("user") == fedid] return TasksListResponse(tasks=tasks) @@ -526,9 +510,9 @@ def get_state(runner: Annotated[WorkerDispatcher, Depends(_runner)]) -> WorkerSt ) @start_as_current_span(TRACER, "state_change_request.new_state") def set_state( - request: Request, state_change_request: StateChangeRequest, response: Response, + fedid: Fedid, opa: Annotated[OpaUserClient, Depends(opa)], # _: Annotated[None, Depends(access_task_permission)], runner: Annotated[WorkerDispatcher, Depends(_runner)], @@ -558,16 +542,12 @@ def set_state( and new_state in _ALLOWED_TRANSITIONS[current_state] ): active = runner.run(interface.get_active_task) - access_token: dict[str, Any] | None = getattr( - request.state, "decoded_access_token", None - ) - user = access_token.get("fedid") if access_token else None if ( opa and not opa.admin() and active - and active.task.metadata.get("user") != user + and active.task.metadata.get("user") != fedid ): raise HTTPException(status_code=status.HTTP_403_FORBIDDEN) From 0fbb6dd1ba8898cda7d2231aee87e28a05ef132f Mon Sep 17 00:00:00 2001 From: Peter Holloway Date: Thu, 28 May 2026 17:17:51 +0100 Subject: [PATCH 14/31] Use starlette statuses directly --- src/blueapi/service/authorization.py | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/src/blueapi/service/authorization.py b/src/blueapi/service/authorization.py index c669b627c..edcec5cbd 100644 --- a/src/blueapi/service/authorization.py +++ b/src/blueapi/service/authorization.py @@ -6,7 +6,7 @@ from aiohttp import ClientSession from fastapi import Depends, HTTPException, Request -from starlette import status +from starlette.status import HTTP_401_UNAUTHORIZED, HTTP_403_FORBIDDEN from blueapi.config import OIDCConfig, OpaConfig, ServiceAccount from blueapi.service.authentication import TiledAuth, unchecked_bearer_token @@ -73,7 +73,7 @@ async def require_submit_task(self, instrument_session: str, token: str): "visit": int(match["visit"]), }, ): - raise HTTPException(status_code=status.HTTP_403_UNORTHORIZED) + raise HTTPException(status_code=HTTP_403_FORBIDDEN) async def is_admin(self, token: str) -> bool: return await self._call_opa(self._conf.admin_check, {"token": token}) @@ -118,13 +118,13 @@ async def opa( if opa := cast(OpaClient | None, getattr(request.app.state, "authz", None)): if not token: - raise HTTPException(status_code=status.HTTP_401_UNAUTHORIZED) + raise HTTPException(status_code=HTTP_401_UNAUTHORIZED) return OpaUserClient(opa, token) return None async def submit_permission( - opa: Annotated[OpaUserClient, Depends(opa)], + opa: Annotated[OpaUserClient | None, Depends(opa)], task_request: TaskRequest, ): if opa: From 5c225028157e98295aac22967a387e74d72088f9 Mon Sep 17 00:00:00 2001 From: Peter Holloway Date: Thu, 28 May 2026 17:27:47 +0100 Subject: [PATCH 15/31] test task submission authz --- .../unit_tests/service/test_authorization.py | 121 ++++++++++++++++++ 1 file changed, 121 insertions(+) diff --git a/tests/unit_tests/service/test_authorization.py b/tests/unit_tests/service/test_authorization.py index 37c1d7e3f..65f5c44a6 100644 --- a/tests/unit_tests/service/test_authorization.py +++ b/tests/unit_tests/service/test_authorization.py @@ -8,9 +8,12 @@ from blueapi.config import OIDCConfig, OpaConfig, ServiceAccount from blueapi.service.authorization import ( OpaClient, + OpaUserClient, opa, + submit_permission, validate_tiled_config, ) +from blueapi.service.model import TaskRequest # Reusable client patch decorator patch_client_session = patch( @@ -25,6 +28,7 @@ def opa_config() -> OpaConfig: return OpaConfig( root=HttpUrl("http://auth.example.com"), submit_task_check="/auth/submit", + admin_check="/auth/admin", tiled_service_account_check="/auth/tiled", ) @@ -108,6 +112,105 @@ async def test_opa_adds_input_fields(session: MagicMock, opa_config: OpaConfig): ) +@pytest.mark.parametrize( + "result,context", + [(True, nullcontext()), (False, pytest.raises(HTTPException, match="403"))], +) +@patch_client_session +async def test_require_submit_task( + session: MagicMock, + opa_config: OpaConfig, + result: bool, + context: AbstractContextManager, +): + session.return_value.post = AsyncMock( + return_value=MagicMock(json=AsyncMock(return_value={"result": result})) + ) + + client = OpaClient(instrument="p99", config=opa_config) + + session.assert_called_once_with(base_url="http://auth.example.com/") + with context: + await client.require_submit_task( + instrument_session="cm12345-1", token="foo_bar" + ) + + session().post.assert_called_once_with( + "/auth/submit", + json={ + "input": { + "token": "foo_bar", + "beamline": "p99", + "audience": "account", + "visit": 1, + "proposal": 12345, + } + }, + ) + + +@patch_client_session +async def test_opa_require_submit_task_invalid_session( + session: MagicMock, opa_config: OpaConfig +): + client = OpaClient(instrument="p45", config=opa_config) + + with pytest.raises(ValueError): + await client.require_submit_task( + instrument_session="not a session", token="foo_bar" + ) + + +@pytest.mark.parametrize("result", [True, False]) +@patch_client_session +async def test_opa_is_admin(session: MagicMock, opa_config: OpaConfig, result: bool): + session.return_value.post = AsyncMock( + return_value=MagicMock(json=AsyncMock(return_value={"result": result})) + ) + client = OpaClient(instrument="p45", config=opa_config) + + admin = await client.is_admin("foo_bar") + + assert admin == result + + session().post.assert_called_once_with( + "/auth/admin", + json={"input": {"token": "foo_bar", "beamline": "p45", "audience": "account"}}, + ) + + +@pytest.mark.parametrize( + "result,context", + [ + (None, nullcontext()), + (HTTPException(status_code=403), pytest.raises(HTTPException, match="403")), + ], +) +async def test_user_client_can_submit_task(result, context: AbstractContextManager): + opa = MagicMock(spec=OpaUserClient) + opa.require_submit_task = AsyncMock(side_effect=result) + + user_client = OpaUserClient(opa, "foo_bar") + + with context: + await user_client.can_submit_task( + TaskRequest(name="foo", params={}, instrument_session="cm12345-1") + ) + opa.require_submit_task.assert_called_once_with("cm12345-1", "foo_bar") + + +@pytest.mark.parametrize("result", [True, False]) +async def test_user_client_admin(result: bool): + opa = MagicMock(spec=OpaUserClient) + opa.is_admin = AsyncMock(return_value=result) + + user_client = OpaUserClient(opa, "foo_bar") + + admin = await user_client.admin() + + assert admin == result + + async def test_validate_tiled_config(): opa = MagicMock(spec=OpaClient) tiled = ServiceAccount() @@ -177,3 +280,21 @@ async def test_opa_dependency_without_authz(token): del request.app.state.authz user_client = await opa(request, token) assert user_client is None + + +@pytest.mark.parametrize( + "result,context", + [ + (None, nullcontext()), + (HTTPException(status_code=403), pytest.raises(HTTPException, match="403")), + ], +) +async def test_submit_permission_dependency(result, context: AbstractContextManager): + opa = MagicMock(spec=OpaUserClient) + opa.can_submit_task.side_effect = result + with context: + await submit_permission(opa, Mock()) + + +async def test_submit_permission_dependency_without_opa(): + assert await submit_permission(None, Mock()) is None From 56d20c5f9047003875e07150f0ee0f24972f3843 Mon Sep 17 00:00:00 2001 From: Peter Holloway Date: Fri, 5 Jun 2026 15:19:28 +0100 Subject: [PATCH 16/31] Use _config instead of _conf --- src/blueapi/service/authorization.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/blueapi/service/authorization.py b/src/blueapi/service/authorization.py index edcec5cbd..5f1e2e980 100644 --- a/src/blueapi/service/authorization.py +++ b/src/blueapi/service/authorization.py @@ -66,7 +66,7 @@ async def require_submit_task(self, instrument_session: str, token: str): raise ValueError("Invalid instrument session") if not await self._call_opa( - self._conf.submit_task_check, + self._config.submit_task_check, { "token": token, "proposal": int(match["proposal"]), @@ -76,7 +76,7 @@ async def require_submit_task(self, instrument_session: str, token: str): raise HTTPException(status_code=HTTP_403_FORBIDDEN) async def is_admin(self, token: str) -> bool: - return await self._call_opa(self._conf.admin_check, {"token": token}) + return await self._call_opa(self._config.admin_check, {"token": token}) class OpaUserClient: From c812969a3d450365f9ee73f36f634dee6af7da10 Mon Sep 17 00:00:00 2001 From: Peter Holloway Date: Fri, 5 Jun 2026 16:36:19 +0100 Subject: [PATCH 17/31] Re-use instrument session regex --- src/blueapi/service/authorization.py | 3 +-- src/blueapi/utils/__init__.py | 3 +++ src/blueapi/utils/serialization.py | 10 +++------- 3 files changed, 7 insertions(+), 9 deletions(-) diff --git a/src/blueapi/service/authorization.py b/src/blueapi/service/authorization.py index 5f1e2e980..e54d0daa9 100644 --- a/src/blueapi/service/authorization.py +++ b/src/blueapi/service/authorization.py @@ -1,5 +1,4 @@ import logging -import re from collections.abc import Mapping from contextlib import AbstractAsyncContextManager, aclosing, nullcontext from typing import Annotated, Any, Self, cast @@ -11,9 +10,9 @@ from blueapi.config import OIDCConfig, OpaConfig, ServiceAccount from blueapi.service.authentication import TiledAuth, unchecked_bearer_token from blueapi.service.model import TaskRequest +from blueapi.utils import INSTRUMENT_SESSION_RE LOGGER = logging.getLogger(__name__) -INSTRUMENT_SESSION_RE = re.compile(r"^[a-z]{2}(?P\d+)-(?P\d+)$") class OpaClient: diff --git a/src/blueapi/utils/__init__.py b/src/blueapi/utils/__init__.py index e5cc85166..f722c5b42 100644 --- a/src/blueapi/utils/__init__.py +++ b/src/blueapi/utils/__init__.py @@ -1,3 +1,4 @@ +import re from collections.abc import Callable, Mapping from functools import wraps from logging import Logger @@ -30,6 +31,8 @@ Args = ParamSpec("Args") Return = TypeVar("Return") +INSTRUMENT_SESSION_RE = re.compile(r"^[a-z]{2}(?P\d+)-(?P\d+)$") + def report_successful_devices( devices: Mapping[str, Any], sim_backend: bool, logger: Logger diff --git a/src/blueapi/utils/serialization.py b/src/blueapi/utils/serialization.py index deee82b1e..8918cf882 100644 --- a/src/blueapi/utils/serialization.py +++ b/src/blueapi/utils/serialization.py @@ -1,9 +1,10 @@ import json -import re from typing import Any from pydantic import BaseModel +from blueapi import utils + def serialize(obj: Any) -> Any: """ @@ -28,13 +29,8 @@ def serialize(obj: Any) -> Any: return obj -_INSTRUMENT_SESSION_AUTHZ_REGEX: re.Pattern = re.compile( - r"^[a-zA-Z]{2}(?P\d+)-(?P\d+)$" -) - - def access_blob(instrument_session: str, beamline: str) -> str: - m = _INSTRUMENT_SESSION_AUTHZ_REGEX.match(instrument_session) + m = utils.INSTRUMENT_SESSION_RE.match(instrument_session) if m is None: raise ValueError( "Unable to extract proposal and visit from " From eed7160074d97b7c38b7943da8946e352e239a5e Mon Sep 17 00:00:00 2001 From: Peter Holloway Date: Fri, 5 Jun 2026 17:04:21 +0100 Subject: [PATCH 18/31] remove task access check --- src/blueapi/service/main.py | 1 - 1 file changed, 1 deletion(-) diff --git a/src/blueapi/service/main.py b/src/blueapi/service/main.py index b5c1189de..e9a148af5 100644 --- a/src/blueapi/service/main.py +++ b/src/blueapi/service/main.py @@ -514,7 +514,6 @@ def set_state( response: Response, fedid: Fedid, opa: Annotated[OpaUserClient, Depends(opa)], - # _: Annotated[None, Depends(access_task_permission)], runner: Annotated[WorkerDispatcher, Depends(_runner)], ) -> WorkerState: """ From 4c534f7669622ca7e1788bd4f2833cf70185c48c Mon Sep 17 00:00:00 2001 From: Peter Holloway Date: Fri, 5 Jun 2026 17:06:25 +0100 Subject: [PATCH 19/31] Add match to raises check --- tests/unit_tests/service/test_authorization.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/unit_tests/service/test_authorization.py b/tests/unit_tests/service/test_authorization.py index 65f5c44a6..a2e602f21 100644 --- a/tests/unit_tests/service/test_authorization.py +++ b/tests/unit_tests/service/test_authorization.py @@ -155,7 +155,7 @@ async def test_opa_require_submit_task_invalid_session( ): client = OpaClient(instrument="p45", config=opa_config) - with pytest.raises(ValueError): + with pytest.raises(ValueError, match="Invalid instrument session"): await client.require_submit_task( instrument_session="not a session", token="foo_bar" ) From c2449e76cc4b837227a4fc8f1db9503bbc84ba8d Mon Sep 17 00:00:00 2001 From: Peter Holloway Date: Fri, 5 Jun 2026 17:12:30 +0100 Subject: [PATCH 20/31] Add exception detail --- src/blueapi/service/authorization.py | 8 ++++++-- src/blueapi/service/main.py | 5 ++++- 2 files changed, 10 insertions(+), 3 deletions(-) diff --git a/src/blueapi/service/authorization.py b/src/blueapi/service/authorization.py index e54d0daa9..f9008138a 100644 --- a/src/blueapi/service/authorization.py +++ b/src/blueapi/service/authorization.py @@ -72,7 +72,9 @@ async def require_submit_task(self, instrument_session: str, token: str): "visit": int(match["visit"]), }, ): - raise HTTPException(status_code=HTTP_403_FORBIDDEN) + raise HTTPException( + status_code=HTTP_403_FORBIDDEN, detail="Not authorized to submit task" + ) async def is_admin(self, token: str) -> bool: return await self._call_opa(self._config.admin_check, {"token": token}) @@ -117,7 +119,9 @@ async def opa( if opa := cast(OpaClient | None, getattr(request.app.state, "authz", None)): if not token: - raise HTTPException(status_code=HTTP_401_UNAUTHORIZED) + raise HTTPException( + status_code=HTTP_401_UNAUTHORIZED, detail="Authentication missing" + ) return OpaUserClient(opa, token) return None diff --git a/src/blueapi/service/main.py b/src/blueapi/service/main.py index e9a148af5..95117e014 100644 --- a/src/blueapi/service/main.py +++ b/src/blueapi/service/main.py @@ -548,7 +548,10 @@ def set_state( and active and active.task.metadata.get("user") != fedid ): - raise HTTPException(status_code=status.HTTP_403_FORBIDDEN) + raise HTTPException( + status_code=status.HTTP_403_FORBIDDEN, + detail="Not authorized to set worker state", + ) if new_state == WorkerState.PAUSED: runner.run(interface.pause_worker, state_change_request.defer) From 9d5340739b4d4aa0d34b9164c3afb0cec61c7b2c Mon Sep 17 00:00:00 2001 From: Peter Holloway Date: Fri, 5 Jun 2026 17:15:15 +0100 Subject: [PATCH 21/31] Let admin see all tasks --- src/blueapi/service/main.py | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/src/blueapi/service/main.py b/src/blueapi/service/main.py index 95117e014..2fc83d8f5 100644 --- a/src/blueapi/service/main.py +++ b/src/blueapi/service/main.py @@ -364,6 +364,7 @@ def validate_task_status(v: str) -> TaskStatusEnum: def get_tasks( fedid: Fedid, runner: Annotated[WorkerDispatcher, Depends(_runner)], + opa: Annotated[OpaUserClient, Depends(opa)], task_status: str | SkipJsonSchema[None] = None, ) -> TasksListResponse: """ @@ -384,7 +385,8 @@ def get_tasks( else: tasks = runner.run(interface.get_tasks) - tasks = [t for t in tasks if t.task.metadata.get("user") == fedid] + if opa and not opa.admin(): + tasks = [t for t in tasks if t.task.metadata.get("user") == fedid] return TasksListResponse(tasks=tasks) From 747bce5fd6ff85568418a129f22f9b302c54f4a2 Mon Sep 17 00:00:00 2001 From: Peter Holloway Date: Fri, 5 Jun 2026 17:15:55 +0100 Subject: [PATCH 22/31] Start of api authz tests --- tests/unit_tests/service/test_rest_api.py | 56 ++++++++++++++++++++++- 1 file changed, 55 insertions(+), 1 deletion(-) diff --git a/tests/unit_tests/service/test_rest_api.py b/tests/unit_tests/service/test_rest_api.py index 79258fc7b..e903e74a5 100644 --- a/tests/unit_tests/service/test_rest_api.py +++ b/tests/unit_tests/service/test_rest_api.py @@ -14,9 +14,15 @@ from pydantic_core import InitErrorDetails from super_state_machine.errors import TransitionError -from blueapi.config import ApplicationConfig, CORSConfig, OIDCConfig, RestConfig +from blueapi.config import ( + ApplicationConfig, + CORSConfig, + OIDCConfig, + RestConfig, +) from blueapi.core.bluesky_types import Plan from blueapi.service import main +from blueapi.service.authorization import OpaUserClient, opa from blueapi.service.interface import ( cancel_active_task, get_device, @@ -54,6 +60,11 @@ def mock_runner() -> Mock: return Mock(spec=WorkerDispatcher) +@pytest.fixture +def mock_opa_client() -> Mock: + return Mock(spec=OpaUserClient) + + @pytest.fixture def client(mock_runner: Mock) -> Iterator[TestClient]: with patch("blueapi.service.interface.worker"): @@ -79,6 +90,27 @@ def client_with_auth( main.teardown_runner() +@pytest.fixture +def access_token(valid_token_with_jwt: dict[str, Any]) -> str: + return valid_token_with_jwt["access_token"] + + +@pytest.fixture +def client_with_opa( + mock_runner: Mock, + oidc_config: OIDCConfig, + mock_opa_client: Mock, + mock_authn_server, +): + with patch("blueapi.service.interface.worker"): + main.setup_runner(runner=mock_runner) + app = main.get_app(ApplicationConfig(oidc=oidc_config)) + app.dependency_overrides[opa] = lambda: mock_opa_client + client = TestClient(app) + yield client + main.teardown_runner() + + @pytest.fixture def rest_config_with_cors() -> RestConfig: cors_config = CORSConfig( @@ -416,6 +448,28 @@ def test_get_tasks_by_status_invalid(client: TestClient) -> None: assert response.status_code == status.HTTP_400_BAD_REQUEST +def test_get_tasks_filters_by_user( + mock_runner: Mock, + client_with_opa: TestClient, + access_token: str, + mock_opa_client: Mock, +): + + print("Start of test") + mock_runner.run.return_value = [ + TrackableTask(task_id="foo", task=Task(name="f1", metadata={"user": "jd1"})), + TrackableTask(task_id="bar", task=Task(name="f2", metadata={"user": "jd2"})), + ] + print(f"in test: {mock_opa_client=}") + mock_opa_client.admin.return_value = False + client_with_opa.headers["Authorization"] = f"Bearer {access_token}" + tasks = client_with_opa.get("/tasks").json().get("tasks") + print(tasks) + + assert len(tasks) == 1 + assert tasks[0]["task_id"] == "foo" + + def test_delete_submitted_task(mock_runner: Mock, client: TestClient) -> None: task_id = str(uuid.uuid4()) mock_runner.run.return_value = task_id From cce0db1f7a5167865de70b166d429b7a49d2dd47 Mon Sep 17 00:00:00 2001 From: Peter Holloway Date: Mon, 8 Jun 2026 14:30:20 +0100 Subject: [PATCH 23/31] Make get_tasks async to access authz check --- src/blueapi/service/main.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/blueapi/service/main.py b/src/blueapi/service/main.py index 2fc83d8f5..a429b7d61 100644 --- a/src/blueapi/service/main.py +++ b/src/blueapi/service/main.py @@ -361,7 +361,7 @@ def validate_task_status(v: str) -> TaskStatusEnum: @secure_router_v1.get("/tasks", status_code=status.HTTP_200_OK, tags=[Tag.TASK]) @secure_router.get("/tasks", status_code=status.HTTP_200_OK, tags=[Tag.TASK]) @start_as_current_span(TRACER) -def get_tasks( +async def get_tasks( fedid: Fedid, runner: Annotated[WorkerDispatcher, Depends(_runner)], opa: Annotated[OpaUserClient, Depends(opa)], @@ -385,7 +385,7 @@ def get_tasks( else: tasks = runner.run(interface.get_tasks) - if opa and not opa.admin(): + if opa and not await opa.admin(): tasks = [t for t in tasks if t.task.metadata.get("user") == fedid] return TasksListResponse(tasks=tasks) From 8880acc22ee1a58d462e89c32d9542a72f955cc7 Mon Sep 17 00:00:00 2001 From: Peter Holloway Date: Mon, 8 Jun 2026 14:45:41 +0100 Subject: [PATCH 24/31] Parametrise filter test to check with and without admin --- tests/unit_tests/service/test_rest_api.py | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/tests/unit_tests/service/test_rest_api.py b/tests/unit_tests/service/test_rest_api.py index e903e74a5..548ac3078 100644 --- a/tests/unit_tests/service/test_rest_api.py +++ b/tests/unit_tests/service/test_rest_api.py @@ -448,11 +448,14 @@ def test_get_tasks_by_status_invalid(client: TestClient) -> None: assert response.status_code == status.HTTP_400_BAD_REQUEST +@pytest.mark.parametrize("admin,task_ids", [(True, ["foo", "bar"]), (False, ["foo"])]) def test_get_tasks_filters_by_user( mock_runner: Mock, client_with_opa: TestClient, access_token: str, mock_opa_client: Mock, + admin: bool, + task_ids: list[str], ): print("Start of test") @@ -461,13 +464,12 @@ def test_get_tasks_filters_by_user( TrackableTask(task_id="bar", task=Task(name="f2", metadata={"user": "jd2"})), ] print(f"in test: {mock_opa_client=}") - mock_opa_client.admin.return_value = False + mock_opa_client.admin.return_value = admin client_with_opa.headers["Authorization"] = f"Bearer {access_token}" tasks = client_with_opa.get("/tasks").json().get("tasks") print(tasks) - assert len(tasks) == 1 - assert tasks[0]["task_id"] == "foo" + assert [t["task_id"] for t in tasks] == task_ids def test_delete_submitted_task(mock_runner: Mock, client: TestClient) -> None: From 55f61a3740ec557dc7ae4fb50cc84dad8ee72ce0 Mon Sep 17 00:00:00 2001 From: Peter Holloway Date: Mon, 8 Jun 2026 15:17:11 +0100 Subject: [PATCH 25/31] Add test for deleting tasks --- src/blueapi/service/main.py | 12 ++++++++---- tests/unit_tests/service/test_rest_api.py | 24 ++++++++++++++++++++++- 2 files changed, 31 insertions(+), 5 deletions(-) diff --git a/src/blueapi/service/main.py b/src/blueapi/service/main.py index a429b7d61..21ab4bf56 100644 --- a/src/blueapi/service/main.py +++ b/src/blueapi/service/main.py @@ -154,7 +154,7 @@ def get_app(config: ApplicationConfig): return app -def access_task_permission( +async def access_task_permission( opa: Annotated[OpaUserClient | None, Depends(opa)], task_id: str, fedid: Fedid, @@ -162,12 +162,16 @@ def access_task_permission( ): task = runner.run(interface.get_task_by_id, task_id) - if opa and not opa.admin() and (task and fedid != task.task.metadata.get("user")): + if ( + opa + and not await opa.admin() + and (task and fedid != task.task.metadata.get("user")) + ): raise HTTPException(status_code=status.HTTP_404_NOT_FOUND) # start_task_permission is used when there is WorkerTask -def start_task_permission( +async def start_task_permission( task: WorkerTask, opa: Annotated[OpaUserClient, Depends(opa)], fedid: Fedid, @@ -178,7 +182,7 @@ def start_task_permission( status_code=status.HTTP_422_UNPROCESSABLE_CONTENT, detail="No task id provided", ) - access_task_permission(opa, task.task_id, fedid, runner) + await access_task_permission(opa, task.task_id, fedid, runner) async def on_key_error_404(_: Request, __: Exception): diff --git a/tests/unit_tests/service/test_rest_api.py b/tests/unit_tests/service/test_rest_api.py index 548ac3078..bb61d0a57 100644 --- a/tests/unit_tests/service/test_rest_api.py +++ b/tests/unit_tests/service/test_rest_api.py @@ -21,7 +21,7 @@ RestConfig, ) from blueapi.core.bluesky_types import Plan -from blueapi.service import main +from blueapi.service import interface, main from blueapi.service.authorization import OpaUserClient, opa from blueapi.service.interface import ( cancel_active_task, @@ -479,6 +479,28 @@ def test_delete_submitted_task(mock_runner: Mock, client: TestClient) -> None: assert response.json() == {"task_id": f"{task_id}"} +def test_cant_delete_other_users_task( + mock_runner: Mock, + client_with_opa: TestClient, + access_token: str, + mock_opa_client: Mock, +): + mock_opa_client.admin.return_value = False + mock_runner.run.side_effect = lambda mth, *args: { + interface.get_task_by_id: TrackableTask( + task_id="bar", task=Task(name="t2", metadata={"user": "jd2"}) + ), + }[mth] + client_with_opa.headers["Authorization"] = f"Bearer {access_token}" + + resp = client_with_opa.delete("/tasks/bar") + + # 404 to obfuscate whether task exists when inaccessible + assert resp.status_code == 404 + + mock_runner.run.assert_called_once() + + def test_set_active_task(client: TestClient) -> None: task_id = str(uuid.uuid4()) task = WorkerTask(task_id=task_id) From f8d5a91503bbb2a67f733465806eb7e124e7770e Mon Sep 17 00:00:00 2001 From: Peter Holloway Date: Mon, 8 Jun 2026 16:42:20 +0100 Subject: [PATCH 26/31] Add test for submit without permission --- tests/unit_tests/service/test_rest_api.py | 19 ++++++++++++++++++- 1 file changed, 18 insertions(+), 1 deletion(-) diff --git a/tests/unit_tests/service/test_rest_api.py b/tests/unit_tests/service/test_rest_api.py index bb61d0a57..6495c0c35 100644 --- a/tests/unit_tests/service/test_rest_api.py +++ b/tests/unit_tests/service/test_rest_api.py @@ -7,7 +7,7 @@ import jwt import pytest from bluesky.protocols import Stoppable -from fastapi import status +from fastapi import HTTPException, status from fastapi.testclient import TestClient from httpx import Headers from pydantic import BaseModel, ValidationError @@ -287,6 +287,23 @@ def test_create_task(mock_runner: Mock, client: TestClient) -> None: assert response.json() == {"task_id": task_id} +def test_submit_task_requires_permission( + mock_runner: Mock, + client_with_opa: TestClient, + mock_opa_client: Mock, + access_token: str, +): + task = TaskRequest(name="sleep", params={"time": 2}, instrument_session="cm12345-2") + client_with_opa.headers["Authorization"] = f"Bearer {access_token}" + mock_opa_client.can_submit_task.side_effect = HTTPException(status_code=403) + mock_runner.run.side_effect = RuntimeError("Task should not be submitted") + + resp = client_with_opa.post("/tasks", json=task.model_dump()) + + assert resp.status_code == 403 + mock_runner.run.assert_not_called() + + def test_create_task_inserts_auth_metadata( mock_runner: Mock, client_with_auth: TestClient, From a7eb74a1469b9ccc88f5b5a6dd3bd8cf565afc69 Mon Sep 17 00:00:00 2001 From: Peter Holloway Date: Tue, 9 Jun 2026 09:27:58 +0100 Subject: [PATCH 27/31] Test setting other user's task active --- tests/unit_tests/service/test_rest_api.py | 29 +++++++++++++++++++++++ 1 file changed, 29 insertions(+) diff --git a/tests/unit_tests/service/test_rest_api.py b/tests/unit_tests/service/test_rest_api.py index 6495c0c35..d63720f78 100644 --- a/tests/unit_tests/service/test_rest_api.py +++ b/tests/unit_tests/service/test_rest_api.py @@ -566,6 +566,35 @@ def test_set_active_task_worker_already_running( assert response.json() == {"detail": "Worker already active"} +@pytest.mark.parametrize("admin,status", [(True, 200), (False, 404)]) +def test_set_other_users_task_active( + mock_runner: Mock, + client_with_opa: TestClient, + mock_opa_client: Mock, + access_token: str, + admin: bool, + status: int, +): + + task_id = "foo" + task = WorkerTask(task_id=task_id) + mock_opa_client.admin.return_value = admin + + client_with_opa.headers["Authorization"] = f"Bearer {access_token}" + + mock_runner.run.side_effect = lambda mth, *a, **kw: { + interface.get_task_by_id: TrackableTask( + task_id="foo", task=Task(name="bar", metadata={"user": "jd2"}) + ), + interface.get_active_task: None, + interface.begin_task: None, + }[mth] + + resp = client_with_opa.put("/worker/task", json=task.model_dump()) + + assert resp.status_code == status + + def test_get_task(mock_runner: Mock, client: TestClient): task_id = str(uuid.uuid4()) task = TrackableTask( From 760868d8adcf9e0d79da512fdbc7a70caf85b546 Mon Sep 17 00:00:00 2001 From: Peter Holloway Date: Tue, 9 Jun 2026 09:44:14 +0100 Subject: [PATCH 28/31] Test getting other users task --- tests/unit_tests/service/test_rest_api.py | 19 +++++++++++++++++++ 1 file changed, 19 insertions(+) diff --git a/tests/unit_tests/service/test_rest_api.py b/tests/unit_tests/service/test_rest_api.py index d63720f78..3c78dc5ab 100644 --- a/tests/unit_tests/service/test_rest_api.py +++ b/tests/unit_tests/service/test_rest_api.py @@ -627,6 +627,25 @@ def test_get_task(mock_runner: Mock, client: TestClient): } +@pytest.mark.parametrize("admin,status", [(True, 200), (False, 404)]) +def test_get_other_users_task( + mock_runner: Mock, + client_with_opa: TestClient, + mock_opa_client: Mock, + access_token: str, + admin: bool, + status: int, +): + client_with_opa.headers["Authorization"] = f"Bearer {access_token}" + mock_runner.run.return_value = TrackableTask( + task_id="foo", task=Task(name="bar", metadata={"user": "jd2"}) + ) + mock_opa_client.admin.return_value = admin + + resp = client_with_opa.get("/tasks/foo") + assert resp.status_code == status + + def test_get_all_tasks(mock_runner: Mock, client: TestClient): task_id = str(uuid.uuid4()) tasks = [ From 4c247b9a92f1cca1a044f5438ced65b0ff319ce5 Mon Sep 17 00:00:00 2001 From: Peter Holloway Date: Tue, 9 Jun 2026 11:05:06 +0100 Subject: [PATCH 29/31] Add tests for set state --- src/blueapi/service/main.py | 4 ++-- tests/unit_tests/service/test_rest_api.py | 29 +++++++++++++++++++++++ 2 files changed, 31 insertions(+), 2 deletions(-) diff --git a/src/blueapi/service/main.py b/src/blueapi/service/main.py index 21ab4bf56..d35ffa38b 100644 --- a/src/blueapi/service/main.py +++ b/src/blueapi/service/main.py @@ -515,7 +515,7 @@ def get_state(runner: Annotated[WorkerDispatcher, Depends(_runner)]) -> WorkerSt tags=[Tag.TASK], ) @start_as_current_span(TRACER, "state_change_request.new_state") -def set_state( +async def set_state( state_change_request: StateChangeRequest, response: Response, fedid: Fedid, @@ -550,7 +550,7 @@ def set_state( if ( opa - and not opa.admin() + and not await opa.admin() and active and active.task.metadata.get("user") != fedid ): diff --git a/tests/unit_tests/service/test_rest_api.py b/tests/unit_tests/service/test_rest_api.py index 3c78dc5ab..81ab6a97e 100644 --- a/tests/unit_tests/service/test_rest_api.py +++ b/tests/unit_tests/service/test_rest_api.py @@ -833,6 +833,35 @@ def test_set_state_invalid_transition(mock_runner: Mock, client: TestClient): } +@pytest.mark.parametrize("admin,status", [(True, 202), (False, 403)]) +def test_set_state_of_other_users_task( + mock_runner: Mock, + client_with_opa: TestClient, + mock_opa_client: Mock, + access_token: str, + admin: bool, + status: int, +): + + mock_opa_client.admin.return_value = admin + mock_runner.run.side_effect = lambda mth, *a, **kw: { + interface.get_active_task: TrackableTask( + task_id="foo", task=Task(name="bar", metadata={"user": "jd2"}) + ), + interface.get_worker_state: WorkerState.RUNNING, + interface.cancel_active_task: WorkerState.ABORTING, + }[mth] + + client_with_opa.headers["Authorization"] = f"Bearer {access_token}" + + resp = client_with_opa.put( + "/worker/state", + json=StateChangeRequest(new_state=WorkerState.ABORTING).model_dump(), + ) + + assert resp.status_code == status + + def test_get_environment_idle(mock_runner: Mock, client: TestClient) -> None: environment_id = uuid.uuid4() mock_runner.state = EnvironmentResponse( From aaa33b44bb4eada10a91678a48ec0ea074caac2f Mon Sep 17 00:00:00 2001 From: Peter Holloway Date: Wed, 10 Jun 2026 10:27:17 +0100 Subject: [PATCH 30/31] Remove print debugging --- tests/unit_tests/service/test_rest_api.py | 3 --- 1 file changed, 3 deletions(-) diff --git a/tests/unit_tests/service/test_rest_api.py b/tests/unit_tests/service/test_rest_api.py index 81ab6a97e..8ec1b9b65 100644 --- a/tests/unit_tests/service/test_rest_api.py +++ b/tests/unit_tests/service/test_rest_api.py @@ -475,16 +475,13 @@ def test_get_tasks_filters_by_user( task_ids: list[str], ): - print("Start of test") mock_runner.run.return_value = [ TrackableTask(task_id="foo", task=Task(name="f1", metadata={"user": "jd1"})), TrackableTask(task_id="bar", task=Task(name="f2", metadata={"user": "jd2"})), ] - print(f"in test: {mock_opa_client=}") mock_opa_client.admin.return_value = admin client_with_opa.headers["Authorization"] = f"Bearer {access_token}" tasks = client_with_opa.get("/tasks").json().get("tasks") - print(tasks) assert [t["task_id"] for t in tasks] == task_ids From 4e7885ad855c5d1007cf9bbb47cfb10ab33433d9 Mon Sep 17 00:00:00 2001 From: Peter Holloway Date: Mon, 29 Jun 2026 11:30:07 +0100 Subject: [PATCH 31/31] Make API change a minor version --- docs/reference/openapi.yaml | 2 +- src/blueapi/config.py | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/docs/reference/openapi.yaml b/docs/reference/openapi.yaml index 68c538342..4ab7548bb 100644 --- a/docs/reference/openapi.yaml +++ b/docs/reference/openapi.yaml @@ -441,7 +441,7 @@ info: name: Apache 2.0 url: https://www.apache.org/licenses/LICENSE-2.0.html title: BlueAPI Control - version: 1.4.1 + version: 1.5.0 openapi: 3.1.0 paths: /api/v1/devices: diff --git a/src/blueapi/config.py b/src/blueapi/config.py index 9367c9f88..a181f4c34 100644 --- a/src/blueapi/config.py +++ b/src/blueapi/config.py @@ -324,7 +324,7 @@ class ApplicationConfig(BlueapiBaseModel): """ #: API version to publish in OpenAPI schema - REST_API_VERSION: ClassVar[str] = "1.4.1" + REST_API_VERSION: ClassVar[str] = "1.5.0" LICENSE_INFO: ClassVar[dict[str, str]] = { "name": "Apache 2.0",