diff --git a/shelfmark/main.py b/shelfmark/main.py index 85a125a5..19557d2b 100644 --- a/shelfmark/main.py +++ b/shelfmark/main.py @@ -1478,6 +1478,43 @@ def _download_row_owned_by_actor( return False +def _resolve_queue_actor() -> tuple[bool, int | None, str | None, Response | None]: + is_admin, db_user_id, can_access_status = _resolve_status_scope() + actor_username = session.get("user_id") + normalized_actor_username = actor_username if isinstance(actor_username, str) else None + + if not is_admin and (not can_access_status or db_user_id is None): + return ( + is_admin, + db_user_id, + normalized_actor_username, + jsonify({"error": "User identity unavailable", "code": "user_identity_unavailable"}), + ) + + return is_admin, db_user_id, normalized_actor_username, None + + +def _queue_task_visible_to_actor( + task_id: str, + *, + is_admin: bool, + actor_user_id: int | None, + actor_username: str | None, +) -> bool: + if is_admin: + return True + + task = backend.book_queue.get_task(task_id) + if task is None: + return False + + return _task_owned_by_actor( + task, + actor_user_id=actor_user_id, + actor_username=actor_username, + ) + + backend.book_queue.set_queue_hook(_record_download_queued) backend.book_queue.set_terminal_status_hook(_record_download_terminal_snapshot) @@ -1799,6 +1836,22 @@ def api_set_priority(book_id: str) -> Response | tuple[Response, int]: return jsonify({"error": "Priority not provided"}), 400 priority = int(data["priority"]) + + is_admin, db_user_id, actor_username, identity_error = _resolve_queue_actor() + if identity_error is not None: + return identity_error, 403 + + task = backend.book_queue.get_task(book_id) + if task is None: + return jsonify({"error": "Failed to update priority or book not found"}), 404 + + if not is_admin and not _task_owned_by_actor( + task, + actor_user_id=db_user_id, + actor_username=actor_username, + ): + return jsonify({"error": "Forbidden", "code": "download_not_owned"}), 403 + success = backend.set_book_priority(book_id, priority) if success: @@ -1837,6 +1890,23 @@ def api_reorder_queue() -> Response | tuple[Response, int]: if not isinstance(priority, int): return jsonify({"error": f"Invalid priority for book {book_id}"}), 400 + is_admin, db_user_id, actor_username, identity_error = _resolve_queue_actor() + if identity_error is not None: + return identity_error, 403 + + if not is_admin: + owned_book_priorities = {} + for book_id in book_priorities: + task = backend.book_queue.get_task(str(book_id)) + if task is None: + continue + if not _task_owned_by_actor( + task, actor_user_id=db_user_id, actor_username=actor_username + ): + return jsonify({"error": "Forbidden", "code": "download_not_owned"}), 403 + owned_book_priorities[book_id] = book_priorities[book_id] + book_priorities = owned_book_priorities + success = backend.reorder_queue(book_priorities) if success: @@ -1858,6 +1928,20 @@ def api_queue_order() -> Response | tuple[Response, int]: """ try: queue_order = backend.get_queue_order() + is_admin, db_user_id, actor_username, identity_error = _resolve_queue_actor() + if identity_error is not None: + return identity_error, 403 + if not is_admin: + queue_order = [ + item + for item in queue_order + if _queue_task_visible_to_actor( + str(item.get("id", "")), + is_admin=False, + actor_user_id=db_user_id, + actor_username=actor_username, + ) + ] return jsonify({"queue": queue_order}) except _OPERATIONAL_ERRORS as e: logger.error_trace(f"Queue order error: {e}") @@ -1875,6 +1959,20 @@ def api_active_downloads() -> Response | tuple[Response, int]: """ try: active_downloads = backend.get_active_downloads() + is_admin, db_user_id, actor_username, identity_error = _resolve_queue_actor() + if identity_error is not None: + return identity_error, 403 + if not is_admin: + active_downloads = [ + task_id + for task_id in active_downloads + if _queue_task_visible_to_actor( + task_id, + is_admin=False, + actor_user_id=db_user_id, + actor_username=actor_username, + ) + ] return jsonify({"active_downloads": active_downloads}) except _OPERATIONAL_ERRORS as e: logger.error_trace(f"Active downloads error: {e}") diff --git a/tests/core/test_download_api_guardrails.py b/tests/core/test_download_api_guardrails.py index 0afde78f..a7a708d3 100644 --- a/tests/core/test_download_api_guardrails.py +++ b/tests/core/test_download_api_guardrails.py @@ -856,3 +856,204 @@ def fake_queue_status(user_id=None): assert resp.status_code == 200 assert observed["user_id"] is None + + +class TestQueueManagementEndpointGuardrails: + def test_non_owner_cannot_set_priority(self, main_module, client): + owner = _create_user(main_module, prefix="owner") + actor = _create_user(main_module, prefix="actor") + _set_authenticated_session( + client, + user_id=actor["username"], + db_user_id=actor["id"], + is_admin=False, + ) + task = DownloadTask( + task_id="owned-priority-1", + source="direct_download", + title="Owned Task", + user_id=owner["id"], + username=owner["username"], + ) + + with patch.object(main_module, "get_auth_mode", return_value="builtin"): + with patch.object(main_module.backend.book_queue, "get_task", return_value=task): + with patch.object(main_module.backend, "set_book_priority") as mock_set_priority: + resp = client.put("/api/queue/owned-priority-1/priority", json={"priority": 1}) + + assert resp.status_code == 403 + assert resp.get_json()["code"] == "download_not_owned" + mock_set_priority.assert_not_called() + + def test_owner_can_set_priority(self, main_module, client): + user = _create_user(main_module, prefix="reader") + _set_authenticated_session( + client, + user_id=user["username"], + db_user_id=user["id"], + is_admin=False, + ) + task = DownloadTask( + task_id="reader-priority-1", + source="direct_download", + title="Reader Task", + user_id=user["id"], + username=user["username"], + ) + + with patch.object(main_module, "get_auth_mode", return_value="builtin"): + with patch.object(main_module.backend.book_queue, "get_task", return_value=task): + with patch.object( + main_module.backend, "set_book_priority", return_value=True + ) as mock_set_priority: + resp = client.put("/api/queue/reader-priority-1/priority", json={"priority": 2}) + + assert resp.status_code == 200 + assert resp.get_json() == { + "status": "updated", + "book_id": "reader-priority-1", + "priority": 2, + } + mock_set_priority.assert_called_once_with("reader-priority-1", 2) + + def test_non_owner_cannot_reorder_other_users_task(self, main_module, client): + owner = _create_user(main_module, prefix="owner") + actor = _create_user(main_module, prefix="actor") + _set_authenticated_session( + client, + user_id=actor["username"], + db_user_id=actor["id"], + is_admin=False, + ) + owned_task = DownloadTask( + task_id="actor-reorder-1", + source="direct_download", + title="Actor Task", + user_id=actor["id"], + username=actor["username"], + ) + other_task = DownloadTask( + task_id="owner-reorder-1", + source="direct_download", + title="Owner Task", + user_id=owner["id"], + username=owner["username"], + ) + + def fake_get_task(task_id): + return { + "actor-reorder-1": owned_task, + "owner-reorder-1": other_task, + }.get(task_id) + + with patch.object(main_module, "get_auth_mode", return_value="builtin"): + with patch.object( + main_module.backend.book_queue, "get_task", side_effect=fake_get_task + ): + with patch.object(main_module.backend, "reorder_queue") as mock_reorder: + resp = client.post( + "/api/queue/reorder", + json={ + "book_priorities": { + "actor-reorder-1": 1, + "owner-reorder-1": 0, + } + }, + ) + + assert resp.status_code == 403 + assert resp.get_json()["code"] == "download_not_owned" + mock_reorder.assert_not_called() + + def test_non_admin_queue_order_is_scoped_to_owned_tasks(self, main_module, client): + user = _create_user(main_module, prefix="reader") + other = _create_user(main_module, prefix="other") + _set_authenticated_session( + client, + user_id=user["username"], + db_user_id=user["id"], + is_admin=False, + ) + user_task = DownloadTask( + task_id="reader-order-1", + source="direct_download", + title="Reader Task", + user_id=user["id"], + username=user["username"], + ) + other_task = DownloadTask( + task_id="other-order-1", + source="direct_download", + title="Other Task", + user_id=other["id"], + username=other["username"], + ) + + def fake_get_task(task_id): + return { + "reader-order-1": user_task, + "other-order-1": other_task, + }.get(task_id) + + with patch.object(main_module, "get_auth_mode", return_value="builtin"): + with patch.object( + main_module.backend, + "get_queue_order", + return_value=[ + {"id": "reader-order-1", "title": "Reader Task", "priority": 0}, + {"id": "other-order-1", "title": "Other Task", "priority": 1}, + ], + ): + with patch.object( + main_module.backend.book_queue, "get_task", side_effect=fake_get_task + ): + resp = client.get("/api/queue/order") + + assert resp.status_code == 200 + assert resp.get_json()["queue"] == [ + {"id": "reader-order-1", "title": "Reader Task", "priority": 0} + ] + + def test_non_admin_active_downloads_are_scoped_to_owned_tasks(self, main_module, client): + user = _create_user(main_module, prefix="reader") + other = _create_user(main_module, prefix="other") + _set_authenticated_session( + client, + user_id=user["username"], + db_user_id=user["id"], + is_admin=False, + ) + user_task = DownloadTask( + task_id="reader-active-1", + source="direct_download", + title="Reader Task", + user_id=user["id"], + username=user["username"], + ) + other_task = DownloadTask( + task_id="other-active-1", + source="direct_download", + title="Other Task", + user_id=other["id"], + username=other["username"], + ) + + def fake_get_task(task_id): + return { + "reader-active-1": user_task, + "other-active-1": other_task, + }.get(task_id) + + with patch.object(main_module, "get_auth_mode", return_value="builtin"): + with patch.object( + main_module.backend, + "get_active_downloads", + return_value=["reader-active-1", "other-active-1"], + ): + with patch.object( + main_module.backend.book_queue, "get_task", side_effect=fake_get_task + ): + resp = client.get("/api/downloads/active") + + assert resp.status_code == 200 + assert resp.get_json() == {"active_downloads": ["reader-active-1"]}