Fix queue management ownership checks (#970)

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}")

tests/core/test_download_api_guardrails.py

@@ -856,3 +856,204 @@ class TestStatusEndpointGuardrails:
 
         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"]}