Skip to content

Commit 0518099

Browse files
committed
Add test for deleting tasks
1 parent e6a6917 commit 0518099

2 files changed

Lines changed: 31 additions & 5 deletions

File tree

src/blueapi/service/main.py

Lines changed: 8 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -152,20 +152,24 @@ def get_app(config: ApplicationConfig):
152152
return app
153153

154154

155-
def access_task_permission(
155+
async def access_task_permission(
156156
opa: Annotated[OpaUserClient | None, Depends(opa)],
157157
task_id: str,
158158
fedid: Fedid,
159159
runner: Annotated[WorkerDispatcher, Depends(_runner)],
160160
):
161161
task = runner.run(interface.get_task_by_id, task_id)
162162

163-
if opa and not opa.admin() and (task and fedid != task.task.metadata.get("user")):
163+
if (
164+
opa
165+
and not await opa.admin()
166+
and (task and fedid != task.task.metadata.get("user"))
167+
):
164168
raise HTTPException(status_code=status.HTTP_404_NOT_FOUND)
165169

166170

167171
# start_task_permission is used when there is WorkerTask
168-
def start_task_permission(
172+
async def start_task_permission(
169173
task: WorkerTask,
170174
opa: Annotated[OpaUserClient, Depends(opa)],
171175
fedid: Fedid,
@@ -176,7 +180,7 @@ def start_task_permission(
176180
status_code=status.HTTP_422_UNPROCESSABLE_CONTENT,
177181
detail="No task id provided",
178182
)
179-
access_task_permission(opa, task.task_id, fedid, runner)
183+
await access_task_permission(opa, task.task_id, fedid, runner)
180184

181185

182186
async def on_key_error_404(_: Request, __: Exception):

tests/unit_tests/service/test_rest_api.py

Lines changed: 23 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -21,7 +21,7 @@
2121
RestConfig,
2222
)
2323
from blueapi.core.bluesky_types import Plan
24-
from blueapi.service import main
24+
from blueapi.service import interface, main
2525
from blueapi.service.authorization import OpaUserClient, opa
2626
from blueapi.service.interface import (
2727
cancel_active_task,
@@ -479,6 +479,28 @@ def test_delete_submitted_task(mock_runner: Mock, client: TestClient) -> None:
479479
assert response.json() == {"task_id": f"{task_id}"}
480480

481481

482+
def test_cant_delete_other_users_task(
483+
mock_runner: Mock,
484+
client_with_opa: TestClient,
485+
access_token: str,
486+
mock_opa_client: Mock,
487+
):
488+
mock_opa_client.admin.return_value = False
489+
mock_runner.run.side_effect = lambda mth, *args: {
490+
interface.get_task_by_id: TrackableTask(
491+
task_id="bar", task=Task(name="t2", metadata={"user": "jd2"})
492+
),
493+
}[mth]
494+
client_with_opa.headers["Authorization"] = f"Bearer {access_token}"
495+
496+
resp = client_with_opa.delete("/tasks/bar")
497+
498+
# 404 to obfuscate whether task exists when inaccessible
499+
assert resp.status_code == 404
500+
501+
mock_runner.run.assert_called_once()
502+
503+
482504
def test_set_active_task(client: TestClient) -> None:
483505
task_id = str(uuid.uuid4())
484506
task = WorkerTask(task_id=task_id)

0 commit comments

Comments
 (0)