diff --git a/shelfmark/api/websocket.py b/shelfmark/api/websocket.py index 12bfaec8..6c1be11e 100644 --- a/shelfmark/api/websocket.py +++ b/shelfmark/api/websocket.py @@ -8,6 +8,8 @@ from typing import TYPE_CHECKING, Any from flask_socketio import SocketIO, join_room, leave_room +from shelfmark.core.viewer_redaction import viewer_download_id + if TYPE_CHECKING: from collections.abc import Callable @@ -167,7 +169,12 @@ class WebSocketManager: logger.exception("Failed to send status update for room %s", room) def broadcast_download_progress( - self, book_id: str, progress: float, status: str, user_id: int | None = None + self, + book_id: str, + progress: float, + status: str, + user_id: int | None = None, + request_id: int | None = None, ) -> None: """Broadcast download progress update for a specific book.""" socketio = self._get_socketio() @@ -178,12 +185,14 @@ class WebSocketManager: data = {"book_id": book_id, "progress": progress, "status": status} # Admins always see all progress socketio.emit("download_progress", data, to="admins") - # If task belongs to a specific user, send to their room too + # If task belongs to a specific user, send to their room too, under the id + # their status uses: an opaque one for a request-linked download (#1418). if user_id is not None: room = f"user_{user_id}" with self._rooms_lock: if room in self._user_rooms: - socketio.emit("download_progress", data, to=room) + user_data = {**data, "book_id": viewer_download_id(book_id, request_id)} + socketio.emit("download_progress", user_data, to=room) logger.debug("Broadcasted progress for book %s: %s%%", book_id, progress) except Exception: logger.exception("Error broadcasting download progress") diff --git a/shelfmark/core/activity_routes.py b/shelfmark/core/activity_routes.py index 4bf8ffc3..de63457a 100644 --- a/shelfmark/core/activity_routes.py +++ b/shelfmark/core/activity_routes.py @@ -31,6 +31,15 @@ from shelfmark.core.request_helpers import ( populate_request_usernames, ) from shelfmark.core.request_validation import RequestStatus +from shelfmark.core.viewer_redaction import ( + is_public_download_id, + match_public_download_id, + public_download_id, + redact_download_payload, + redact_request_row, + redact_status, + viewer_download_id, +) if TYPE_CHECKING: from collections.abc import Callable @@ -501,6 +510,77 @@ def _request_history_entry( } +def _request_linked_task_ids( + actor: _ActorContext, + *, + queue_status: Callable[..., dict[str, dict[str, Any]]], + download_history_service: DownloadHistoryService, +) -> list[str]: + """Task ids of the actor's request-linked downloads, which they see by opaque id.""" + if actor.db_user_id is None: + return [] + live_task_ids = [ + task_id + for bucket in queue_status(user_id=actor.owner_scope).values() + if isinstance(bucket, dict) + for task_id, payload in bucket.items() + if isinstance(payload, dict) and payload.get("request_id") is not None + ] + return [ + *live_task_ids, + *download_history_service.list_request_task_ids(user_id=actor.db_user_id), + ] + + +def _viewer_dismissed_entries( + entries: list[dict[str, str]], + *, + request_ids_by_task: dict[str, object], +) -> list[dict[str, str]]: + """Rewrite a non-admin's dismissed download keys to the ids their snapshot uses. + + Downloads outside the recent rows and the live queue never reach the snapshot, + so their entries are dropped rather than looked up one by one. + """ + viewer_entries: list[dict[str, str]] = [] + for entry in entries: + if entry["item_type"] != "download": + viewer_entries.append(entry) + continue + task_id = _parse_item_key(entry["item_key"], "download") + if task_id is None or task_id not in request_ids_by_task: + continue + viewer_id = viewer_download_id(task_id, request_ids_by_task[task_id]) + viewer_entries.append({"item_type": "download", "item_key": f"download:{viewer_id}"}) + return viewer_entries + + +def _redact_history_entry(entry: dict[str, Any]) -> dict[str, Any]: + """Return a history entry with the same redactions as the non-admin snapshot.""" + redacted = dict(entry) + snapshot = entry.get("snapshot") + source_id = entry.get("source_id") + if entry.get("item_type") == "download": + if isinstance(source_id, str) and source_id: + viewer_id = viewer_download_id(source_id, entry.get("request_id")) + item_key = f"download:{viewer_id}" + redacted.update(id=item_key, item_key=item_key, source_id=viewer_id) + if isinstance(snapshot, dict) and isinstance(snapshot.get("download"), dict): + redacted["snapshot"] = { + **snapshot, + "download": redact_download_payload(snapshot["download"], task_id=source_id), + } + return redacted + + # A request's source_id names the download it was fulfilled with, which the + # requester sees by its opaque id. + if isinstance(source_id, str) and source_id: + redacted["source_id"] = public_download_id(source_id) + if isinstance(snapshot, dict) and isinstance(snapshot.get("request"), dict): + redacted["snapshot"] = {**snapshot, "request": redact_request_row(snapshot["request"])} + return redacted + + def register_activity_routes( app: Flask, user_db: UserDB, @@ -577,6 +657,25 @@ def register_activity_routes( continue visible_request_rows.append(row) + if not actor.is_admin: + # Redacted only now: the delivery-state sync above matches on real task ids. + request_ids_by_task: dict[str, object] = { + task_id: payload.get("request_id") + for bucket in live_queue.values() + if isinstance(bucket, dict) + for task_id, payload in bucket.items() + if isinstance(payload, dict) + } + request_ids_by_task.update( + (str(row.get("task_id") or "").strip(), row.get("request_id")) for row in db_rows + ) + dismissed_entries = _viewer_dismissed_entries( + dismissed_entries, + request_ids_by_task=request_ids_by_task, + ) + status = redact_status(status) + visible_request_rows = [redact_request_row(row) for row in visible_request_rows] + return jsonify( { "status": status, @@ -622,6 +721,21 @@ def register_activity_routes( item_key=item_key, ) + # The id as this viewer knows it, which is what the response echoes. + viewer_task_id = task_id + if not actor.is_admin and is_public_download_id(task_id): + task_id = ( + match_public_download_id( + task_id, + _request_linked_task_ids( + actor, + queue_status=queue_status, + download_history_service=download_history_service, + ), + ) + or task_id + ) + existing = download_history_service.get_by_task_id(task_id) if existing is None: return _activity_error_response( @@ -671,7 +785,7 @@ def register_activity_routes( ) dismissal_item = { "item_type": "download", - "item_key": f"download:{task_id}", + "item_key": f"download:{viewer_task_id}", } elif item_type == "request": @@ -796,6 +910,7 @@ def register_activity_routes( dismissal_items: list[dict[str, str]] = [] missing_item_keys: list[str] = [] live_queue_index: dict[str, tuple[str, dict[str, Any]]] | None = None + request_linked_task_ids: list[str] | None = None for item in items: if not isinstance(item, dict): @@ -824,9 +939,18 @@ def register_activity_routes( item_key=item_key, item_count=len(items), ) + viewer_task_id = task_id + if not actor.is_admin and is_public_download_id(task_id): + if request_linked_task_ids is None: + request_linked_task_ids = _request_linked_task_ids( + actor, + queue_status=queue_status, + download_history_service=download_history_service, + ) + task_id = match_public_download_id(task_id, request_linked_task_ids) or task_id existing = download_history_service.get_by_task_id(task_id) if existing is None: - missing_item_keys.append(f"download:{task_id}") + missing_item_keys.append(f"download:{viewer_task_id}") continue if live_queue_index is None: live_queue_index = _build_queue_index(queue_status(user_id=actor.owner_scope)) @@ -1060,6 +1184,8 @@ def register_activity_routes( msg = f"Unknown activity history item_type: {item_type}" raise RuntimeError(msg) + if not actor.is_admin: + payload = [_redact_history_entry(entry) for entry in payload] return jsonify(payload) @app.route("/api/activity/history", methods=["DELETE"]) diff --git a/shelfmark/core/download_history_service.py b/shelfmark/core/download_history_service.py index a3087e74..28dc0965 100644 --- a/shelfmark/core/download_history_service.py +++ b/shelfmark/core/download_history_service.py @@ -407,6 +407,23 @@ class DownloadHistoryService: finally: conn.close() + def list_request_task_ids(self, *, user_id: int) -> list[str]: + """Return the task ids of every request-linked download owned by ``user_id``. + + A non-admin addresses these downloads by an opaque id, so routes resolve it + against this set, which also confines the lookup to the user's own downloads. + """ + normalized_user_id = normalize_optional_positive_int(user_id, "user_id") + conn = self._connect() + try: + rows = conn.execute( + "SELECT task_id FROM download_history WHERE user_id = ? AND request_id IS NOT NULL", + (normalized_user_id,), + ).fetchall() + return [str(row["task_id"]) for row in rows] + finally: + conn.close() + def list_recent( self, *, diff --git a/shelfmark/core/request_routes.py b/shelfmark/core/request_routes.py index 2a0fd767..d34ec1ac 100644 --- a/shelfmark/core/request_routes.py +++ b/shelfmark/core/request_routes.py @@ -41,6 +41,7 @@ from shelfmark.core.requests_service import ( fulfil_request, reject_request, ) +from shelfmark.core.viewer_redaction import redact_request_row if TYPE_CHECKING: from collections.abc import Callable @@ -847,6 +848,9 @@ def register_request_routes( ) except ValueError as exc: return jsonify({"error": str(exc)}), 400 + if not session.get("is_admin", False): + # A fulfilled request carries the release an admin picked (#1418). + rows = [redact_request_row(row) for row in rows] return jsonify(rows) @app.route("/api/requests/", methods=["DELETE"]) @@ -901,6 +905,8 @@ def register_request_routes( room="admins", ) + if not session.get("is_admin", False): + updated = redact_request_row(updated) return jsonify(updated) @app.route("/api/admin/requests", methods=["GET"]) diff --git a/shelfmark/core/viewer_redaction.py b/shelfmark/core/viewer_redaction.py new file mode 100644 index 00000000..6a26f155 --- /dev/null +++ b/shelfmark/core/viewer_redaction.py @@ -0,0 +1,149 @@ +"""Hide release-source details from non-admin viewers (#1418). + +A download queued from an approved request carries the release an admin picked. +Its task id is that release's ``source_id``, which for Prowlarr is +``:``, and a private tracker's guid is a URL into the tracker. +The requester never browsed those releases, so for them a request-linked +download is addressed by an opaque id instead. Request rows keep only the +release fields the activity cards display, and a download path shrinks to the +file name, which the browser download reveals anyway. + +Downloads a user queued directly keep their real id: the user picked that +release from search results that already showed its ``source_id``, and the +release list matches its buttons to the queue by that id. +""" + +from __future__ import annotations + +import hashlib +import hmac +import os +from pathlib import PurePath +from typing import TYPE_CHECKING, Any + +from shelfmark.core.request_helpers import normalize_positive_int + +if TYPE_CHECKING: + from collections.abc import Iterable + +PUBLIC_DOWNLOAD_ID_PREFIX = "dl_" + +# Release fields a non-admin's activity cards display. Everything else (source_id, +# indexer, info and download URLs, torrent attributes) stays server-side. +_RELEASE_DISPLAY_FIELDS = frozenset( + { + "author", + "content_type", + "extension", + "filetype", + "format", + "language", + "preview", + "series_count", + "series_name", + "series_position", + "size", + "source", + "source_display_name", + "subtitle", + "title", + "year", + } +) + +# Replaced with a key derived from the app secret at startup, so public ids survive +# restarts. A random key still keeps them unguessable until then. +_public_id_key = os.urandom(32) + + +def configure_public_id_key(secret: bytes) -> None: + """Derive the public download id key from the app's persisted secret.""" + global _public_id_key + _public_id_key = hmac.new(secret, b"shelfmark-public-download-id", hashlib.sha256).digest() + + +def public_download_id(task_id: str) -> str: + """Return the opaque, stable id a non-admin sees for ``task_id``. + + Keyed so the task id can't be recovered by hashing guesses: tracker torrent + ids are sequential, so a plain hash would be easy to reverse. + """ + digest = hmac.new(_public_id_key, task_id.encode(), hashlib.sha256).hexdigest() + return f"{PUBLIC_DOWNLOAD_ID_PREFIX}{digest[:32]}" + + +def is_public_download_id(value: object) -> bool: + """Whether ``value`` is an opaque id produced by :func:`public_download_id`.""" + return isinstance(value, str) and value.startswith(PUBLIC_DOWNLOAD_ID_PREFIX) + + +def viewer_download_id(task_id: str, request_id: object) -> str: + """Return the id a non-admin sees for a download, given its request link.""" + if normalize_positive_int(request_id) is None: + return task_id + return public_download_id(task_id) + + +def match_public_download_id(download_id: str, task_ids: Iterable[str]) -> str | None: + """Return the task among ``task_ids`` whose public id is ``download_id``.""" + if not is_public_download_id(download_id): + return None + for task_id in task_ids: + if hmac.compare_digest(public_download_id(task_id), download_id): + return task_id + return None + + +def redact_release_data(release_data: object) -> object: + """Keep only the release fields a non-admin's activity cards display.""" + if not isinstance(release_data, dict): + return release_data + return {key: value for key, value in release_data.items() if key in _RELEASE_DISPLAY_FIELDS} + + +def redact_request_row(row: dict[str, Any]) -> dict[str, Any]: + """Return a copy of a request row that is safe to show its non-admin owner.""" + if "release_data" not in row: + return row + return {**row, "release_data": redact_release_data(row["release_data"])} + + +def redact_download_payload(payload: dict[str, Any], task_id: str | None = None) -> dict[str, Any]: + """Return a copy of a download payload that is safe to show its non-admin owner.""" + redacted = dict(payload) + raw_id = task_id if task_id is not None else payload.get("id") + if isinstance(raw_id, str) and raw_id: + viewer_id = viewer_download_id(raw_id, payload.get("request_id")) + if viewer_id != raw_id: + redacted["id"] = viewer_id + # Live queue covers are proxied under the task id as their cache key. + preview = payload.get("preview") + if isinstance(preview, str): + redacted["preview"] = preview.replace( + f"/api/covers/{raw_id}?", f"/api/covers/{viewer_id}?" + ) + + download_path = payload.get("download_path") + if isinstance(download_path, str) and download_path: + redacted["download_path"] = PurePath(download_path).name or None + return redacted + + +def redact_status(status: dict[str, Any]) -> dict[str, Any]: + """Redact every download in a status dict, re-keying request-linked ones.""" + redacted: dict[str, Any] = {} + for bucket, entries in status.items(): + if not isinstance(entries, dict): + redacted[bucket] = entries + continue + bucket_entries: dict[str, Any] = {} + for task_id, payload in entries.items(): + if not isinstance(payload, dict): + bucket_entries[task_id] = payload + continue + redacted_payload = redact_download_payload(payload, task_id=task_id) + bucket_entries[viewer_download_id(task_id, payload.get("request_id"))] = ( + redacted_payload + ) + redacted[bucket] = bucket_entries + return redacted diff --git a/shelfmark/download/clients/qbittorrent.py b/shelfmark/download/clients/qbittorrent.py index c65dc772..ec68fe80 100644 --- a/shelfmark/download/clients/qbittorrent.py +++ b/shelfmark/download/clients/qbittorrent.py @@ -214,13 +214,17 @@ class QBittorrentClient(DownloadClient): self._api_key = config_text(config.get("QBITTORRENT_API_KEY", "")) # qbittorrent-api accepts either a full URL or host:port; prefer the normalized URL - # for consistency. + # for consistency. It ignores the URL's scheme and probes HTTP and HTTPS instead, + # which can settle on plain HTTP against a TLS port behind a reverse proxy (#1417), + # so an explicit https:// is kept. http:// is still probed: normalization adds it to + # a bare host:port, and the probe follows a proxy's redirect to HTTPS. self._client = Client( host=self._base_url, username=username, password=password, api_key=self._api_key or None, VERIFY_WEBUI_CERTIFICATE=get_ssl_verify(self._base_url), + FORCE_SCHEME_FROM_HOST=self._base_url.lower().startswith("https://"), ) self._category = config_text(config.get("QBITTORRENT_CATEGORY", "books")) self._download_dir = config_text(config.get("QBITTORRENT_DOWNLOAD_DIR", "")) diff --git a/shelfmark/download/clients/settings.py b/shelfmark/download/clients/settings.py index 451aec85..3c7128af 100644 --- a/shelfmark/download/clients/settings.py +++ b/shelfmark/download/clients/settings.py @@ -177,6 +177,8 @@ def _test_qbittorrent_connection(current_values: dict[str, Any] | None = None) - password=password, api_key=api_key or None, VERIFY_WEBUI_CERTIFICATE=get_ssl_verify(url), + # Same scheme rule as QBittorrentClient (#1417). + FORCE_SCHEME_FROM_HOST=url.lower().startswith("https://"), ) client.auth_log_in() api_version = client.app.web_api_version diff --git a/shelfmark/download/clients/torbox.py b/shelfmark/download/clients/torbox.py index 85e1b743..4ab689de 100644 --- a/shelfmark/download/clients/torbox.py +++ b/shelfmark/download/clients/torbox.py @@ -5,6 +5,7 @@ from __future__ import annotations import math import shutil import threading +import time from dataclasses import dataclass, field from pathlib import Path, PurePosixPath, PureWindowsPath from typing import Any, ClassVar, NoReturn @@ -36,6 +37,8 @@ _API_BASE = "https://api.torbox.app/v1/api" _API_TIMEOUT = 30 _STATUS_TIMEOUT = 15 _WORKER_JOIN_TIMEOUT = 5.0 +# TorBox can report a torrent finished shortly before its files are present. +_UNAVAILABLE_GRACE_SECONDS = 120.0 _BOOK_EXTENSIONS = ( ".aac", @@ -81,6 +84,7 @@ class _DownloadState: phase: str = "waiting_torbox" error_message: str | None = None progress: float = 0.0 + unavailable_since: float | None = None download_thread: threading.Thread | None = None cancel_event: threading.Event = field(default_factory=threading.Event) lock: threading.Lock = field(default_factory=threading.Lock) @@ -397,10 +401,11 @@ class TorBoxClient(DownloadClient): finished = torrent.get("download_finished") is True present = torrent.get("download_present") is True if finished and not present: - message = "TorBox finished processing but the download is unavailable" - self._set_error(state, message) - return DownloadStatus.error(message) - if finished and present: + if self._unavailable_grace_expired(state): + message = "TorBox finished processing but the download is unavailable" + self._set_error(state, message) + return DownloadStatus.error(message) + elif finished and present: files = torrent.get("files") if not isinstance(files, list): message = "TorBox returned no file list for a completed torrent" @@ -429,6 +434,20 @@ class TorBoxClient(DownloadClient): eta=eta, ) + @staticmethod + def _unavailable_grace_expired(state: _DownloadState) -> bool: + """Return True once finished TorBox files have stayed absent past the grace period.""" + now = time.monotonic() + with state.lock: + if state.unavailable_since is None: + state.unavailable_since = now + logger.info( + "TorBox finished torrent %s but its files are not present yet; waiting", + state.torrent_id, + extra={"torrent_id": state.torrent_id}, + ) + return now - state.unavailable_since >= _UNAVAILABLE_GRACE_SECONDS + @staticmethod def _normalize_remote_progress(value: object) -> float: """Normalize fractional or percentage TorBox progress to 0 through 100.""" diff --git a/shelfmark/download/orchestrator.py b/shelfmark/download/orchestrator.py index 841cdcaa..26201603 100644 --- a/shelfmark/download/orchestrator.py +++ b/shelfmark/download/orchestrator.py @@ -889,9 +889,12 @@ def update_download_progress(book_id: str, progress: float) -> None: if should_broadcast: task = book_queue.get_task(book_id) - task_user_id = task.user_id if task else None ws_manager.broadcast_download_progress( - book_id, progress, "downloading", user_id=task_user_id + book_id, + progress, + "downloading", + user_id=task.user_id if task else None, + request_id=task.request_id if task else None, ) diff --git a/shelfmark/main.py b/shelfmark/main.py index 40d0b3bf..479167dd 100644 --- a/shelfmark/main.py +++ b/shelfmark/main.py @@ -86,6 +86,13 @@ from shelfmark.core.requests_service import ( ) from shelfmark.core.user_db import UserDB from shelfmark.core.utils import AUDIOBOOK_FORMATS, normalize_base_path +from shelfmark.core.viewer_redaction import ( + configure_public_id_key, + is_public_download_id, + match_public_download_id, + redact_status, + viewer_download_id, +) from shelfmark.download import orchestrator as backend from shelfmark.download import warmup from shelfmark.release_sources import ( @@ -157,7 +164,14 @@ socketio = SocketIO(app, **socketio_init_kwargs) # Initialize WebSocket manager ws_manager.init_app(app, socketio) -ws_manager.set_queue_status_fn(backend.queue_status) + + +def _user_room_queue_status(user_id: int | None = None) -> dict[str, dict[str, Any]]: + """Queue status for a non-admin user room, with release details hidden (#1418).""" + return redact_status(backend.queue_status(user_id=user_id)) + + +ws_manager.set_queue_status_fn(_user_room_queue_status) logger.info("Flask-SocketIO initialized with async_mode='%s'", async_mode) logger.info("Socket.IO CORS allowed origins: %s", socketio_cors_allowed_origins) @@ -673,6 +687,7 @@ app.config.update( SESSION_COOKIE_NAME=SESSION_COOKIE_NAME, PERMANENT_SESSION_LIFETIME=604800, # 7 days in seconds ) +configure_public_id_key(app.config["SECRET_KEY"]) logger.info( "Session cookie secure setting: %s (from env: %s)", @@ -1648,6 +1663,34 @@ def _queue_task_visible_to_actor( ) +def _resolve_viewer_download_id(book_id: str) -> str: + """Map a non-admin's opaque download id back to the task it names (#1418). + + Only the caller's own request-linked downloads are searched, so an id that + doesn't resolve comes back unchanged and fails the route's usual lookup. + """ + if not is_public_download_id(book_id): + return book_id + is_admin, db_user_id, _ = _resolve_status_scope() + if is_admin or db_user_id is None: + return book_id + candidates = [ + task_id + for tasks in backend.book_queue.get_status(user_id=db_user_id).values() + for task_id, task in tasks.items() + if getattr(task, "request_id", None) is not None + ] + if download_history_service is not None: + candidates.extend(download_history_service.list_request_task_ids(user_id=db_user_id)) + return match_public_download_id(book_id, candidates) or book_id + + +def _viewer_queue_task_id(task_id: str) -> str: + """Return the id a non-admin sees for a live queue task.""" + task = backend.book_queue.get_task(task_id) + return viewer_download_id(task_id, getattr(task, "request_id", None)) + + backend.book_queue.set_queue_hook(_record_download_queued) backend.book_queue.set_terminal_status_hook(_record_download_terminal_snapshot) @@ -1701,7 +1744,7 @@ def api_status() -> Response | tuple[Response, int]: user_id=user_id, ) _emit_request_update_events(updated_requests) - return jsonify(status) + return jsonify(status if is_admin else redact_status(status)) except _OPERATIONAL_ERRORS as e: logger.error_trace(f"Status error: {e}") return jsonify({"error": str(e)}), 500 @@ -1722,6 +1765,7 @@ def api_local_download() -> Response | tuple[Response, int]: book_id = request.args.get("id", "") if not book_id: return jsonify({"error": "No book ID provided"}), 400 + book_id = _resolve_viewer_download_id(book_id) try: # The path, not the bytes: send_file streams it off disk, where reading it @@ -1845,6 +1889,8 @@ def api_cancel_download(book_id: str) -> Response | tuple[Response, int]: flask.Response: JSON status indicating success or failure. """ + viewer_book_id = book_id + book_id = _resolve_viewer_download_id(book_id) try: task = backend.book_queue.get_task(book_id) if task is None: @@ -1873,7 +1919,7 @@ def api_cancel_download(book_id: str) -> Response | tuple[Response, int]: success = backend.cancel_download(book_id) if success: - return jsonify({"status": "cancelled", "book_id": book_id}) + return jsonify({"status": "cancelled", "book_id": viewer_book_id}) return jsonify({"error": "Failed to cancel download or book not found"}), 404 except _OPERATIONAL_ERRORS as e: logger.error_trace(f"Cancel download error: {e}") @@ -1884,6 +1930,8 @@ def api_cancel_download(book_id: str) -> Response | tuple[Response, int]: @login_required def api_retry_download(book_id: str) -> Response | tuple[Response, int]: """Retry a failed download.""" + viewer_book_id = book_id + book_id = _resolve_viewer_download_id(book_id) try: task = backend.book_queue.get_task(book_id) history_row = None @@ -1948,7 +1996,7 @@ def api_retry_download(book_id: str) -> Response | tuple[Response, int]: ) if success: - return jsonify({"status": "queued", "book_id": book_id}) + return jsonify({"status": "queued", "book_id": viewer_book_id}) if error == "Download not found": return jsonify({"error": error}), 404 @@ -1985,6 +2033,8 @@ def api_set_priority(book_id: str) -> Response | tuple[Response, int]: if identity_error is not None: return identity_error, 403 + viewer_book_id = book_id + book_id = _resolve_viewer_download_id(book_id) task = backend.book_queue.get_task(book_id) if task is None: return jsonify({"error": "Failed to update priority or book not found"}), 404 @@ -1999,7 +2049,7 @@ def api_set_priority(book_id: str) -> Response | tuple[Response, int]: success = backend.set_book_priority(book_id, priority) if success: - return jsonify({"status": "updated", "book_id": book_id, "priority": priority}) + return jsonify({"status": "updated", "book_id": viewer_book_id, "priority": priority}) return jsonify({"error": "Failed to update priority or book not found"}), 404 except ValueError: return jsonify({"error": "Invalid priority value"}), 400 @@ -2039,6 +2089,10 @@ def api_reorder_queue() -> Response | tuple[Response, int]: return identity_error, 403 if not is_admin: + book_priorities = { + _resolve_viewer_download_id(str(book_id)): priority + for book_id, priority in book_priorities.items() + } owned_book_priorities = {} for book_id in book_priorities: task = backend.book_queue.get_task(str(book_id)) @@ -2077,7 +2131,7 @@ def api_queue_order() -> Response | tuple[Response, int]: return identity_error, 403 if not is_admin: queue_order = [ - item + {**item, "id": _viewer_queue_task_id(str(item.get("id", "")))} for item in queue_order if _queue_task_visible_to_actor( str(item.get("id", "")), @@ -2108,7 +2162,7 @@ def api_active_downloads() -> Response | tuple[Response, int]: return identity_error, 403 if not is_admin: active_downloads = [ - task_id + _viewer_queue_task_id(task_id) for task_id in active_downloads if _queue_task_visible_to_actor( task_id, @@ -3518,7 +3572,7 @@ def handle_connect() -> None: user_id = None if is_admin else db_user_id status = backend.queue_status(user_id=user_id) - emit("status_update", status) + emit("status_update", status if is_admin else redact_status(status)) except _OPERATIONAL_ERRORS: logger.exception("Error sending initial status") @@ -3555,7 +3609,7 @@ def handle_status_request() -> None: user_id = None if is_admin else db_user_id status = backend.queue_status(user_id=user_id) - emit("status_update", status) + emit("status_update", status if is_admin else redact_status(status)) except _OPERATIONAL_ERRORS: logger.exception("Error handling status request") emit("error", {"message": "Failed to get status"}) diff --git a/src/frontend/src/components/activity/ActivitySidebar.tsx b/src/frontend/src/components/activity/ActivitySidebar.tsx index d64f38dd..45b88b65 100644 --- a/src/frontend/src/components/activity/ActivitySidebar.tsx +++ b/src/frontend/src/components/activity/ActivitySidebar.tsx @@ -7,7 +7,7 @@ import type { RequestRecord, StatusData } from '../../types'; import { Dropdown } from '../Dropdown'; import { ActivityCard } from './ActivityCard'; import type { DownloadStatusKey } from './activityMappers'; -import { downloadToActivityItem } from './activityMappers'; +import { downloadToActivityItem, linkedDownloadIdForRequest } from './activityMappers'; import type { ActivityItem } from './activityTypes'; interface ActivitySidebarProps { @@ -146,25 +146,6 @@ const getActivityCategory = (item: ActivityItem): ActivityCategoryKey => { return 'in_progress'; }; -const getLinkedDownloadIdFromRequestItem = (item: ActivityItem): string | null => { - if (item.kind !== 'request' || item.visualStatus !== 'fulfilled') { - return null; - } - - const releaseData = item.requestRecord?.release_data; - if (!releaseData || typeof releaseData !== 'object') { - return null; - } - - const sourceId = releaseData.source_id; - if (typeof sourceId !== 'string') { - return null; - } - - const trimmed = sourceId.trim(); - return trimmed ? trimmed : null; -}; - const mergeRequestWithDownload = ( requestItem: ActivityItem, downloadItem: ActivityItem, @@ -320,9 +301,17 @@ export const ActivitySidebar = ({ const { mergedRequestItems, mergedDownloadItems } = useMemo(() => { const downloadsById = new Map(); + // downloadItems is newest first, so a retried request keeps its latest download. + const latestDownloadIdByRequestId = new Map(); downloadItems.forEach((item) => { if (item.downloadBookId) { downloadsById.set(item.downloadBookId, item); + if ( + typeof item.requestId === 'number' && + !latestDownloadIdByRequestId.has(item.requestId) + ) { + latestDownloadIdByRequestId.set(item.requestId, item.downloadBookId); + } } }); @@ -345,7 +334,7 @@ export const ActivitySidebar = ({ }); const nextRequestItems = visibleRequestItems.map((requestItem) => { - const linkedDownloadId = getLinkedDownloadIdFromRequestItem(requestItem); + const linkedDownloadId = linkedDownloadIdForRequest(requestItem, latestDownloadIdByRequestId); if (!linkedDownloadId) { return requestItem; } diff --git a/src/frontend/src/components/activity/activityMappers.ts b/src/frontend/src/components/activity/activityMappers.ts index 697c3ccf..16642696 100644 --- a/src/frontend/src/components/activity/activityMappers.ts +++ b/src/frontend/src/components/activity/activityMappers.ts @@ -188,3 +188,25 @@ export const requestToActivityItem = ( requestRecord: record, }; }; + +/** The id of the download a fulfilled request item was delivered through, if listed. */ +export const linkedDownloadIdForRequest = ( + item: ActivityItem, + latestDownloadIdByRequestId: Map, +): string | null => { + if (item.kind !== 'request' || item.visualStatus !== 'fulfilled') { + return null; + } + + const sourceId = item.requestRecord?.release_data?.source_id; + if (typeof sourceId === 'string' && sourceId.trim()) { + return sourceId.trim(); + } + + // Non-admins don't receive the release's source_id (#1418), so their request is + // matched to the download queued for it instead. + if (typeof item.requestId !== 'number') { + return null; + } + return latestDownloadIdByRequestId.get(item.requestId) ?? null; +}; diff --git a/src/frontend/src/tests/activityMappers.test.ts b/src/frontend/src/tests/activityMappers.test.ts index c13312a6..95392ec8 100644 --- a/src/frontend/src/tests/activityMappers.test.ts +++ b/src/frontend/src/tests/activityMappers.test.ts @@ -2,6 +2,7 @@ import { describe, it, expect } from 'vitest'; import { downloadToActivityItem, + linkedDownloadIdForRequest, requestToActivityItem, } from '../components/activity/activityMappers'; import type { Book, RequestRecord } from '../types/index'; @@ -191,3 +192,32 @@ describe('activityMappers.requestToActivityItem', () => { expect(item.metaLine).toBe('EPUB · 2 MB · Prowlarr'); }); }); + +const fulfilled = (releaseData: Record | null) => + requestToActivityItem( + makeRequest({ status: 'fulfilled', request_level: 'book', release_data: releaseData }), + 'user', + ); + +describe('activityMappers.linkedDownloadIdForRequest', () => { + it("links through the release's source_id when the viewer has it", () => { + const item = fulfilled({ source_id: '21:https://tracker.example/t/1', format: 'epub' }); + + expect(linkedDownloadIdForRequest(item, new Map([[42, 'dl_other']]))).toBe( + '21:https://tracker.example/t/1', + ); + }); + + it('falls back to the download queued for the request when source_id is withheld', () => { + const item = fulfilled({ format: 'epub', size: '2 MB' }); + + expect(linkedDownloadIdForRequest(item, new Map([[42, 'dl_abc']]))).toBe('dl_abc'); + expect(linkedDownloadIdForRequest(item, new Map())).toBeNull(); + }); + + it('does not link a request that has not been fulfilled', () => { + const item = requestToActivityItem(makeRequest({ status: 'pending' }), 'user'); + + expect(linkedDownloadIdForRequest(item, new Map([[42, 'dl_abc']]))).toBeNull(); + }); +}); diff --git a/tests/core/test_activity_routes_api.py b/tests/core/test_activity_routes_api.py index f65b85b4..f1381d6a 100644 --- a/tests/core/test_activity_routes_api.py +++ b/tests/core/test_activity_routes_api.py @@ -5,12 +5,14 @@ from __future__ import annotations import importlib import sqlite3 import uuid +from pathlib import Path from types import SimpleNamespace from unittest.mock import ANY, patch import pytest from shelfmark.core.models import DownloadTask, QueueStatus +from shelfmark.core.viewer_redaction import public_download_id @pytest.fixture(scope="module") @@ -165,15 +167,16 @@ class TestActivityRoutes: assert dismiss_response.status_code == 200 assert dismiss_response.json["status"] == "dismissed" + # A request-linked download reaches its non-admin owner under an opaque id (#1418). assert snapshot_response.status_code == 200 assert { "item_type": "download", - "item_key": "download:test-task", + "item_key": f"download:{public_download_id('test-task')}", } in snapshot_response.json["dismissed"] assert history_response.status_code == 200 assert len(history_response.json) == 1 - assert history_response.json[0]["item_key"] == "download:test-task" + assert history_response.json[0]["item_key"] == f"download:{public_download_id('test-task')}" assert history_response.json[0]["snapshot"]["kind"] == "download" assert history_response.json[0]["snapshot"]["download"]["title"] == "Dismiss Me" @@ -737,7 +740,7 @@ class TestActivityRoutes: assert dismiss_many_response.json["status"] == "dismissed" assert history_response.status_code == 200 assert len(history_response.json) == 1 - assert history_response.json[0]["item_key"] == f"download:{task_id}" + assert history_response.json[0]["item_key"] == f"download:{public_download_id(task_id)}" assert history_response.json[0]["snapshot"]["download"]["status_message"] == "Interrupted" assert history_response.json[0]["snapshot"]["download"]["retry_available"] is True @@ -1133,8 +1136,9 @@ class TestActivityRoutes: response = client.get("/api/activity/snapshot") assert response.status_code == 200 - assert response.json["status"]["error"][task_id]["status_message"] == "Interrupted" - assert response.json["status"]["error"][task_id]["retry_available"] is True + viewer_id = public_download_id(task_id) + assert response.json["status"]["error"][viewer_id]["status_message"] == "Interrupted" + assert response.json["status"]["error"][viewer_id]["retry_available"] is True def test_snapshot_includes_retry_available_for_live_terminal_downloads( self, main_module, client @@ -1166,9 +1170,8 @@ class TestActivityRoutes: response = client.get("/api/activity/snapshot") assert response.status_code == 200 - assert ( - response.json["status"]["error"]["retryable-terminal-task"]["retry_available"] is True - ) + viewer_id = public_download_id("retryable-terminal-task") + assert response.json["status"]["error"][viewer_id]["retry_available"] is True def test_snapshot_reopens_request_when_error_retry_is_no_longer_available( self, main_module, client @@ -1670,3 +1673,217 @@ class TestActivityRoutes: ANY, to="admins", ) + + +class TestNonAdminReleaseRedaction: + """A requester never sees the release an admin picked for them (#1418).""" + + @staticmethod + def _approved_tracker_download(main_module, tmp_path, *, user: dict) -> tuple[dict, str, Path]: + """A book-level request fulfilled from a private tracker, and its finished file.""" + source_id = f"21:https://www.myanonamouse.net/t/{uuid.uuid4().int % 10**6}" + pending = main_module.user_db.create_request( + user_id=user["id"], + content_type="audiobook", + request_level="book", + policy_mode="request_book", + book_data={ + "title": "Example Book Title", + "author": "Example Author", + "provider": "openlibrary", + "provider_id": "ol-1418", + }, + ) + # Approval attaches the release the admin picked, as fulfil_request does. + request_row = main_module.user_db.update_request( + pending["id"], + status="fulfilled", + delivery_state="complete", + release_data={ + "source": "prowlarr", + "source_id": source_id, + "indexer": "MyAnonamouse", + "info_url": source_id.split(":", 1)[1], + "title": "Example Book Title", + "format": "m4b", + "size": "300 MB", + "extra": {"info_hash": "abc123", "indexer_id": 21}, + }, + ) + book_file = ( + tmp_path / "audiobooks" / "Example Author" / "Example Book Title" / "Book Title.m4b" + ) + book_file.parent.mkdir(parents=True) + book_file.write_bytes(b"audiobook bytes") + _record_terminal_download( + main_module, + task_id=source_id, + user_id=user["id"], + username=user["username"], + title="Example Book Title", + source="prowlarr", + source_display_name="Prowlarr", + origin="requested", + request_id=request_row["id"], + download_path=str(book_file), + ) + return request_row, source_id, book_file + + def test_snapshot_hides_the_tracker_and_server_paths(self, main_module, client, tmp_path): + user = _create_user(main_module, prefix="requester") + request_row, source_id, _ = self._approved_tracker_download( + main_module, tmp_path, user=user + ) + _set_session(client, user_id=user["username"], db_user_id=user["id"], is_admin=False) + + with patch.object(main_module, "get_auth_mode", return_value="builtin"): + with patch.object( + main_module.backend, "queue_status", return_value=_sample_status_payload() + ): + response = client.get("/api/activity/snapshot") + + assert response.status_code == 200 + body = response.get_data(as_text=True) + assert "myanonamouse" not in body.lower() + assert str(tmp_path) not in body + + request = next(row for row in response.json["requests"] if row["id"] == request_row["id"]) + assert request["release_data"] == { + "source": "prowlarr", + "title": "Example Book Title", + "format": "m4b", + "size": "300 MB", + } + viewer_id = public_download_id(source_id) + download = response.json["status"]["complete"][viewer_id] + assert download["id"] == viewer_id + assert download["request_id"] == request_row["id"] + assert download["download_path"] == "Book Title.m4b" + + def test_admin_snapshot_keeps_the_release_details(self, main_module, client, tmp_path): + user = _create_user(main_module, prefix="requester") + admin = _create_user(main_module, prefix="admin", role="admin") + _, source_id, book_file = self._approved_tracker_download(main_module, tmp_path, user=user) + _set_session(client, user_id=admin["username"], db_user_id=admin["id"], is_admin=True) + + with patch.object(main_module, "get_auth_mode", return_value="builtin"): + with patch.object( + main_module.backend, "queue_status", return_value=_sample_status_payload() + ): + response = client.get("/api/activity/snapshot") + + assert response.status_code == 200 + assert response.json["status"]["complete"][source_id]["download_path"] == str(book_file) + + def test_a_direct_download_keeps_its_release_id(self, main_module, client, tmp_path): + """The user picked that release from search results showing its id already.""" + user = _create_user(main_module, prefix="reader") + book_file = tmp_path / "library" / "Direct Book.epub" + book_file.parent.mkdir(parents=True) + book_file.write_bytes(b"epub") + task_id = f"direct-{uuid.uuid4().hex[:8]}" + _record_terminal_download( + main_module, + task_id=task_id, + user_id=user["id"], + username=user["username"], + download_path=str(book_file), + ) + _set_session(client, user_id=user["username"], db_user_id=user["id"], is_admin=False) + + with patch.object(main_module, "get_auth_mode", return_value="builtin"): + with patch.object( + main_module.backend, "queue_status", return_value=_sample_status_payload() + ): + response = client.get("/api/activity/snapshot") + + download = response.json["status"]["complete"][task_id] + assert download["id"] == task_id + assert download["download_path"] == "Direct Book.epub" + + def test_live_status_is_rekeyed_for_the_requester(self, main_module, client): + user = _create_user(main_module, prefix="requester") + source_id = "21:https://www.myanonamouse.net/t/777" + queue_status_payload = _sample_status_payload() + queue_status_payload["downloading"][source_id] = { + "id": source_id, + "title": "Live Book", + "request_id": 99, + "preview": f"/api/covers/{source_id}?url=aHR0cHM6Ly9leGFtcGxlLmNvbS9jLmpwZw==", + "download_path": None, + } + _set_session(client, user_id=user["username"], db_user_id=user["id"], is_admin=False) + + with patch.object(main_module, "get_auth_mode", return_value="builtin"): + with patch.object( + main_module.backend, "queue_status", return_value=queue_status_payload + ): + response = client.get("/api/status") + + assert response.status_code == 200 + assert "myanonamouse" not in response.get_data(as_text=True).lower() + viewer_id = public_download_id(source_id) + live = response.json["downloading"][viewer_id] + assert live["id"] == viewer_id + assert live["preview"].startswith(f"/api/covers/{viewer_id}?url=") + + def test_request_list_withholds_the_release(self, main_module, client, tmp_path): + user = _create_user(main_module, prefix="requester") + request_row, _, _ = self._approved_tracker_download(main_module, tmp_path, user=user) + _set_session(client, user_id=user["username"], db_user_id=user["id"], is_admin=False) + + with patch.object(main_module, "get_auth_mode", return_value="builtin"): + response = client.get("/api/requests") + + assert response.status_code == 200 + assert "myanonamouse" not in response.get_data(as_text=True).lower() + listed = next(row for row in response.json if row["id"] == request_row["id"]) + assert "source_id" not in listed["release_data"] + + def test_localdownload_resolves_the_opaque_id_only_for_its_owner( + self, main_module, client, tmp_path + ): + owner = _create_user(main_module, prefix="requester") + other = _create_user(main_module, prefix="other") + _, source_id, _ = self._approved_tracker_download(main_module, tmp_path, user=owner) + url = f"/api/localdownload?id={public_download_id(source_id)}" + + with patch.object(main_module, "get_auth_mode", return_value="builtin"): + _set_session(client, user_id=owner["username"], db_user_id=owner["id"], is_admin=False) + owner_response = client.get(url) + _set_session(client, user_id=other["username"], db_user_id=other["id"], is_admin=False) + other_response = client.get(url) + + assert owner_response.status_code == 200 + assert owner_response.data == b"audiobook bytes" + assert other_response.status_code == 404 + + def test_dismiss_and_history_speak_the_opaque_id(self, main_module, client, tmp_path): + user = _create_user(main_module, prefix="requester") + _, source_id, _ = self._approved_tracker_download(main_module, tmp_path, user=user) + viewer_key = f"download:{public_download_id(source_id)}" + _set_session(client, user_id=user["username"], db_user_id=user["id"], is_admin=False) + + with patch.object(main_module, "get_auth_mode", return_value="builtin"): + with patch.object( + main_module.backend, "queue_status", return_value=_sample_status_payload() + ): + dismiss_response = client.post( + "/api/activity/dismiss", + json={"item_type": "download", "item_key": viewer_key}, + ) + history_response = client.get("/api/activity/history?limit=10&offset=0") + + assert dismiss_response.status_code == 200 + assert dismiss_response.json["item"]["item_key"] == viewer_key + # Stored under the real task id, so the admin view and the queue hooks still match it. + assert f"download:{source_id}" in _hidden_item_keys( + main_module, viewer_scope=f"user:{user['id']}" + ) + + assert history_response.status_code == 200 + assert "myanonamouse" not in history_response.get_data(as_text=True).lower() + entry = history_response.json[0] + assert entry["item_key"] == viewer_key + assert entry["source_id"] == public_download_id(source_id) + assert entry["snapshot"]["download"]["download_path"] == "Book Title.m4b" diff --git a/tests/core/test_viewer_redaction.py b/tests/core/test_viewer_redaction.py new file mode 100644 index 00000000..f6e176d6 --- /dev/null +++ b/tests/core/test_viewer_redaction.py @@ -0,0 +1,117 @@ +"""Tests for hiding release-source details from non-admin viewers (#1418).""" + +from __future__ import annotations + +from unittest.mock import MagicMock + +import pytest + +from shelfmark.api.websocket import WebSocketManager +from shelfmark.core import viewer_redaction +from shelfmark.core.viewer_redaction import ( + match_public_download_id, + public_download_id, + redact_download_payload, + redact_release_data, + redact_status, + viewer_download_id, +) + +_TRACKER_ID = "21:https://www.myanonamouse.net/t/123456" + + +@pytest.fixture(autouse=True) +def _restore_key(): + original = viewer_redaction._public_id_key + yield + viewer_redaction._public_id_key = original + + +def test_public_id_is_stable_opaque_and_keyed(): + viewer_redaction.configure_public_id_key(b"a" * 64) + first = public_download_id(_TRACKER_ID) + + assert first == public_download_id(_TRACKER_ID) + assert first.startswith("dl_") + assert "myanonamouse" not in first + # Keyed: a plain hash of a guessed tracker URL must not reproduce it. + viewer_redaction.configure_public_id_key(b"b" * 64) + assert public_download_id(_TRACKER_ID) != first + + +def test_only_request_linked_downloads_get_an_opaque_id(): + assert viewer_download_id(_TRACKER_ID, 7) == public_download_id(_TRACKER_ID) + assert viewer_download_id("direct-task", None) == "direct-task" + + +def test_an_opaque_id_resolves_only_among_the_given_tasks(): + viewer_id = public_download_id(_TRACKER_ID) + + assert match_public_download_id(viewer_id, ["other", _TRACKER_ID]) == _TRACKER_ID + assert match_public_download_id(viewer_id, ["other"]) is None + assert match_public_download_id(_TRACKER_ID, [_TRACKER_ID]) is None + + +def test_release_data_keeps_only_display_fields(): + redacted = redact_release_data( + { + "source": "prowlarr", + "source_id": _TRACKER_ID, + "indexer": "MyAnonamouse", + "info_url": "https://www.myanonamouse.net/t/123456", + "download_url": "https://prowlarr.local/api/v1/indexer/21/download?apikey=k", + "protocol": "torrent", + "extra": {"info_hash": "abc"}, + "title": "Book", + "format": "m4b", + "size": "300 MB", + } + ) + + assert redacted == {"source": "prowlarr", "title": "Book", "format": "m4b", "size": "300 MB"} + assert redact_release_data(None) is None + + +def test_download_payload_loses_its_path_and_tracker_id(): + payload = { + "id": _TRACKER_ID, + "request_id": 7, + "preview": f"/api/covers/{_TRACKER_ID}?url=abc", + "download_path": "/audiobooks/Example Author/Example Title/Example Title.m4b", + } + + redacted = redact_download_payload(payload) + + viewer_id = public_download_id(_TRACKER_ID) + assert redacted["id"] == viewer_id + assert redacted["preview"] == f"/api/covers/{viewer_id}?url=abc" + assert redacted["download_path"] == "Example Title.m4b" + assert payload["id"] == _TRACKER_ID # the caller's copy is untouched + + +def test_status_is_rekeyed_only_for_request_linked_downloads(): + status = { + "complete": { + _TRACKER_ID: {"id": _TRACKER_ID, "request_id": 7}, + "direct-task": {"id": "direct-task", "request_id": None}, + }, + "queued": {}, + } + + redacted = redact_status(status) + + assert set(redacted["complete"]) == {public_download_id(_TRACKER_ID), "direct-task"} + assert redacted["queued"] == {} + + +def test_progress_reaches_the_requester_under_the_opaque_id(): + manager = WebSocketManager() + socketio = MagicMock() + manager.init_app(MagicMock(), socketio) + manager._user_rooms["user_5"] = 1 + + manager.broadcast_download_progress(_TRACKER_ID, 42.0, "downloading", user_id=5, request_id=7) + + emitted = {call.kwargs["to"]: call.args[1] for call in socketio.emit.call_args_list} + assert emitted["admins"]["book_id"] == _TRACKER_ID + assert emitted["user_5"]["book_id"] == public_download_id(_TRACKER_ID) diff --git a/tests/prowlarr/test_qbittorrent_client.py b/tests/prowlarr/test_qbittorrent_client.py index 48728b6c..991cf12a 100644 --- a/tests/prowlarr/test_qbittorrent_client.py +++ b/tests/prowlarr/test_qbittorrent_client.py @@ -203,6 +203,42 @@ class TestQBittorrentClientTestConnection: assert "401" in message or "failed" in message.lower() +class TestQBittorrentClientScheme: + """qbittorrent-api probes for a scheme unless told to keep the configured one (#1417).""" + + @pytest.mark.parametrize( + ("url", "forced"), + [ + ("https://qbittorrent.example.com", True), + ("HTTPS://qbittorrent.example.com:443", True), + ("http://localhost:8080", False), + ("localhost:8080", False), + ], + ) + def test_only_an_explicit_https_scheme_is_kept(self, monkeypatch, url, forced): + """https:// must not fall back to plain HTTP; http:// and bare hosts keep the probe.""" + config_values = { + "QBITTORRENT_URL": url, + "QBITTORRENT_USERNAME": "admin", + "QBITTORRENT_PASSWORD": "password", + } + monkeypatch.setattr( + "shelfmark.download.clients.qbittorrent.config.get", + lambda key, default="": config_values.get(key, default), + ) + mock_client_class = MagicMock(return_value=MagicMock()) + + with patch.dict("sys.modules", {"qbittorrentapi": MagicMock(Client=mock_client_class)}): + import importlib + + import shelfmark.download.clients.qbittorrent as qb_module + + importlib.reload(qb_module) + qb_module.QBittorrentClient() + + assert mock_client_class.call_args.kwargs["FORCE_SCHEME_FROM_HOST"] is forced + + class TestQBittorrentClientApiKeyAuth: """Tests for API key authentication (qBittorrent 5.2.0+).""" diff --git a/tests/prowlarr/test_qbittorrent_settings.py b/tests/prowlarr/test_qbittorrent_settings.py index 6b164952..436f90a2 100644 --- a/tests/prowlarr/test_qbittorrent_settings.py +++ b/tests/prowlarr/test_qbittorrent_settings.py @@ -110,3 +110,23 @@ def test_settings_test_connection_names_rejected_credential(monkeypatch, api_key ) assert result == {"success": False, "message": f"qBittorrent rejected the {rejected}"} + + +@pytest.mark.parametrize( + ("url", "forced"), + [("https://qbittorrent.example.com", True), ("http://localhost:8080", False)], +) +def test_settings_test_connection_keeps_an_explicit_https_scheme(monkeypatch, url, forced): + """The button connects the same way downloads do, so https:// can't drop to HTTP (#1417).""" + captured = {} + _run_test_connection( + monkeypatch, + { + "QBITTORRENT_URL": url, + "QBITTORRENT_USERNAME": "admin", + "QBITTORRENT_PASSWORD": "password", + }, + fake_qbittorrentapi(captured=captured), + ) + + assert captured["FORCE_SCHEME_FROM_HOST"] is forced diff --git a/tests/prowlarr/test_torbox_client.py b/tests/prowlarr/test_torbox_client.py index 811f9428..1c92ee2d 100644 --- a/tests/prowlarr/test_torbox_client.py +++ b/tests/prowlarr/test_torbox_client.py @@ -193,13 +193,45 @@ class TestTorBoxStatus: assert state.progress == 50.0 thread.start.assert_called_once() - def test_finished_torrent_without_available_content_is_error(self, tmp_path): + def test_finished_torrent_waits_for_content_to_become_present(self, monkeypatch, tmp_path): client = TorBoxClient.__new__(TorBoxClient) state = _state(tmp_path) + start = MagicMock() + monkeypatch.setattr(client, "_maybe_start_download_thread", start) + torrent = { + "download_state": "completed", + "download_finished": True, + "download_present": False, + "progress": 1, + "files": [{"id": 1, "name": "Dune.epub"}], + } - status = client._handle_torrent_status( - {"download_finished": True, "download_present": False}, state - ) + status = client._handle_torrent_status(torrent, state) + + assert status.state == DownloadState.DOWNLOADING + assert status.progress == 50.0 + assert state.phase == "waiting_torbox" + start.assert_not_called() + + status = client._handle_torrent_status({**torrent, "download_present": True}, state) + + assert status.state == DownloadState.DOWNLOADING + start.assert_called_once_with(state, torrent["files"]) + + def test_finished_torrent_without_available_content_errors_after_grace( + self, monkeypatch, tmp_path + ): + client = TorBoxClient.__new__(TorBoxClient) + state = _state(tmp_path) + now = [1000.0] + monkeypatch.setattr("shelfmark.download.clients.torbox.time.monotonic", lambda: now[0]) + torrent = {"download_finished": True, "download_present": False} + + assert client._handle_torrent_status(torrent, state).state == DownloadState.DOWNLOADING + now[0] += 119.0 + assert client._handle_torrent_status(torrent, state).state == DownloadState.DOWNLOADING + now[0] += 1.0 + status = client._handle_torrent_status(torrent, state) assert status.state == DownloadState.ERROR assert "unavailable" in status.message