From 7e53edfcc60b873728080602739de0daaab78883 Mon Sep 17 00:00:00 2001 From: Developer Date: Sun, 12 Jul 2026 10:52:31 +0000 Subject: [PATCH] fix(qbittorrent): recognize SID cookie from raw Set-Cookie header on login MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit qBittorrent (or its reverse proxy) was returning 204 No Content with an SID cookie and no body on a successful login, but the client only treated the response as success if body == "Ok." or the cookie was in the parsed requests cookie jar (resp.cookies.get("SID")). In the reported case the Set-Cookie header was present (set-cookie=yes) yet the jar was empty — requests doesn't always populate the jar from such headers (proxy-set/oddly- attributed cookies) — so a valid login was reported as "Unexpected response". The user re-entering correct credentials never helped. Detect a successful login from EITHER the parsed jar OR the raw Set-Cookie header (cookie name == SID). qBittorrent only sets SID on a valid login, so this remains authoritative. The diagnostic now lists the cookie names it saw for future-proofing. New regression test reproduces the exact 204 + Set-Cookie SID + empty jar case. 387/387 backend tests pass; ruff clean. --- .../clients/.pi-map.index.md | 2 +- .../clients/.pi-map.md | 6 +++--- .../clients/qbittorrent.py | 17 ++++++++++------ backend/tests/test_qbittorrent_client.py | 20 +++++++++++++++++++ 4 files changed, 35 insertions(+), 10 deletions(-) diff --git a/backend/src/media_library_viewer_api/clients/.pi-map.index.md b/backend/src/media_library_viewer_api/clients/.pi-map.index.md index 5ad96ea..1158e62 100644 --- a/backend/src/media_library_viewer_api/clients/.pi-map.index.md +++ b/backend/src/media_library_viewer_api/clients/.pi-map.index.md @@ -2,7 +2,7 @@ dir: backend/src/media_library_viewer_api/clients ## role -Collection of API and protocol client wrappers for external services (Jellyfin, Authentik, Jellyseerr, qBittorrent, SSH, local execution) with shared HTTP timeout configuration. +Provides external service integration clients for authenticating users and aggregating media, filesystem, and torrent data from APIs and remote/local hosts. ## parent index: backend/src/media_library_viewer_api/.pi-map.index.md map: backend/src/media_library_viewer_api/.pi-map.md diff --git a/backend/src/media_library_viewer_api/clients/.pi-map.md b/backend/src/media_library_viewer_api/clients/.pi-map.md index f825c84..1684ef5 100644 --- a/backend/src/media_library_viewer_api/clients/.pi-map.md +++ b/backend/src/media_library_viewer_api/clients/.pi-map.md @@ -4,7 +4,7 @@ dir: backend/src/media_library_viewer_api/clients index: backend/src/media_library_viewer_api/clients/.pi-map.index.md ## role -Collection of API and protocol client wrappers for external services (Jellyfin, Authentik, Jellyseerr, qBittorrent, SSH, local execution) with shared HTTP timeout configuration. +Provides external service integration clients for authenticating users and aggregating media, filesystem, and torrent data from APIs and remote/local hosts. ## files - __init__.py | Swaps the position of two tmux panes within a window or between windows | dep: tmux, sh - authentik.py | API client wrapper for Authentik directory service providing paginated user browsing and search via REST API. | exp: class:AuthentikClient, method:__init__(self, base_url: str, api_token: str, timeout), call:base_url.rstrip, call:self.base_url.endswith, call:http_timeout, call:requests.Session, call:self.session.headers.update, raise:ValueError, method:get(self, path: str, **params: Any) → Any, call:params.items, call:logger.debug, call:sorted, call:clean_params.keys, call:self.session.get, call:response.raise_for_status, call:logger.warning, call:response.json, raise:requests.HTTPError, method:users(self, search, page, page_size) → dict[str, Any], call:self.get, call:isinstance, call:logger.warning, call:type, call:payload.get, call:int, call:pagination.get, call:logger.info, call:len | dep: logging, typing, requests, media_library_viewer_api.clients.http_timeout @@ -12,10 +12,10 @@ Collection of API and protocol client wrappers for external services (Jellyfin, - jellyfin.py | Wraps the Jellyfin/Emby HTTP API to provide methods for fetching users, libraries, media items, playback sessions, and image URLs as plain Python dictionaries. | exp: class:JellyfinClient, method:__init__(self, base_url: str, api_key: str, timeout), call:base_url.rstrip, call:self.base_url.endswith, call:http_timeout, call:requests.Session, call:self.session.headers.update, raise:ValueError, method:get(self, path: str, **params: Any) → Any, call:params.items, call:logger.debug, call:sorted, call:clean_params.keys, call:self.session.get, call:response.raise_for_status, call:logger.warning, call:response.json, raise:requests.HTTPError, method:users(self) → list[dict[str, Any]], call:self.get, call:logger.info, call:len, method:resolve_user_id(self, identifier: str | None) → str, call:self.users, call:any, call:str, call:u.get, call:next, call:logger.info, call:logger.warning, raise:RuntimeError, method:libraries(self, user_id: str) → list[dict[str, Any]], call:self.get(f"/Users/{user_id}/Views").get, call:logger.info, call:len, method:items(self, user_id: str, parent_id, start_index, limit, search, include_item_types, recursive, sort_by, sort_order) → dict[str, Any], call:logger.debug, call:self.get, call:str(recursive).lower, method:item_count(self, user_id: str, include_item_types: str, parent_id) → int, call:self.get, call:int, call:response.get, call:logger.debug, method:media_counts(self, user_id: str) → dict[str, int], call:self.item_count, method:library_item_counts(self, user_id: str, libraries: list[dict[str, Any]]) → list[dict[str, Any]], call:lib.get, call:self.item_count, call:results.append, method:sessions(self, active_within_seconds) → list[dict[str, Any]], call:self.get, call:cast, call:isinstance, method:active_sessions(self, active_within_seconds) → list[dict[str, Any]], call:self.sessions, call:session.get, call:logger.info, call:len, method:image_url(self, item_id: str, image_type) → str | dep: logging, typing, requests, media_library_viewer_api.clients.http_timeout - jellyseerr.py | HTTP API client wrapper for Jellyseerr to fetch user data and enrich Jellyfin user lists. | exp: class:JellyseerrClient, method:__init__(self, base_url: str, api_key: str, timeout), call:base_url.rstrip, call:self.base_url.endswith, call:http_timeout, call:requests.Session, call:self.session.headers.update, raise:ValueError, method:get(self, path: str, **params: Any) → Any, call:params.items, call:logger.debug, call:sorted, call:clean_params.keys, call:self.session.get, call:response.raise_for_status, call:logger.warning, call:response.json, raise:requests.HTTPError, method:absolute_url(self, path: str | None) → str, call:path.startswith, method:jellyfin_users(self) → list[dict[str, Any]], call:self.get, call:isinstance, call:logger.info, call:len, call:payload.get, method:users(self, page_size) → list[dict[str, Any]], call:max, call:int, call:self.get, call:isinstance, call:payload.get, call:results.extend, call:page_info.get, call:logger.debug, call:len, call:logger.info | dep: logging, typing, requests, media_library_viewer_api.clients.http_timeout - local.py | Provides a local command execution client that mirrors remote SSH helpers to run POSIX shell commands, list directories, stat paths, and run ffprobe on the API host for built-in local monitoring. | exp: class:CommandResult, class:LocalCommandClient, method:__init__(self, timeout), method:run(self, command: str, timeout) → CommandResult, call:logger.debug, call:subprocess.run, call:CommandResult, call:logger.warning, call:result.stderr.strip, call:result.stdout.strip, method:list_dir(self, path: str) → CommandResult, call:shlex.quote, call:self.run, method:stat_path(self, path: str) → CommandResult, call:shlex.quote, call:self.run, method:ffprobe_json(self, path: str) → dict[str, object], call:shlex.quote, call:self.run, call:json.loads, raise:RuntimeError | dep: json, logging, posixpath, shlex, subprocess, dataclasses -- qbittorrent.py | Provides a minimal read-only qBittorrent Web API client that handles authentication and fetches torrent main data. | exp: class:QbittorrentClient, method:__init__(self, base_url: str, username: str, password: str, timeout) → None, call:base_url.rstrip, call:self.base_url.endswith, call:http_timeout, call:requests.Session, raise:ValueError, method:_login(self) → None, call:self._session.post, call:resp.raise_for_status, call:resp.text.strip, call:bool, call:resp.cookies.get, call:logger.info, call:resp.headers.get, raise:RuntimeError, method:_get(self, path: str, **params: Any) → dict[str, Any], call:self._login, call:self._session.get, call:logger.debug, call:resp.raise_for_status, call:resp.json, method:maindata(self) → dict[str, Any], call:self._get | dep: logging, typing, requests, media_library_viewer_api.clients.http_timeout +- qbittorrent.py | Provides a minimal, read-only qBittorrent Web API client that manages session authentication and fetches sync data. | exp: class:QbittorrentClient, method:__init__(self, base_url: str, username: str, password: str, timeout) → None, call:base_url.rstrip, call:self.base_url.endswith, call:http_timeout, call:requests.Session, raise:ValueError, method:_login(self) → None, call:self._session.post, call:resp.raise_for_status, call:resp.text.strip, call:resp.headers.get, call:set_cookie_hdr.split("=", 1)[0].strip().upper, call:bool, call:resp.cookies.get, call:logger.info, call:sorted, call:resp.cookies.keys, raise:RuntimeError, method:_get(self, path: str, **params: Any) → dict[str, Any], call:self._login, call:self._session.get, call:logger.debug, call:resp.raise_for_status, call:resp.json, method:maindata(self) → dict[str, Any], call:self._get | dep: logging, typing, requests, media_library_viewer_api.clients.http_timeout - ssh.py | Provides an SSH client wrapper for remote filesystem inspection and media analysis using paramiko, with POSIX shell command execution and host key management. | exp: class:CommandResult, class:RemoteSSHClient, method:__init__(self, host: str, username: str, port, key_filename, private_key, private_key_passphrase, password, known_hosts_path, timeout), raise:ValueError, method:connect(self) → paramiko.SSHClient, call:paramiko.SSHClient, call:client.load_system_host_keys, call:Path, call:bool, call:has_known_host, call:known_hosts_file.is_file, call:client.load_host_keys, call:client.set_missing_host_key_policy, call:paramiko.RejectPolicy, call:paramiko.AutoAddPolicy, call:self._load_private_key, call:client.connect, call:str(exc).lower, call:known_hosts_file.parent.mkdir, call:client.save_host_keys, raise:RuntimeError, method:close(self) → None, call:self._client.close, method:run(self, command: str, timeout) → CommandResult, call:self.connect, call:shlex.quote, call:logger.debug, call:client.exec_command, call:stdout.channel.recv_exit_status, call:CommandResult, call:stdout.read().decode, call:stderr.read().decode, call:logger.warning, call:result.stderr.strip, call:result.stdout.strip, method:list_dir(self, path: str) → CommandResult, call:shlex.quote, call:self.run, call:logger.info, method:stat_path(self, path: str) → CommandResult, call:shlex.quote, call:self.run, call:logger.info, method:ffprobe_json(self, path: str) → dict[str, Any], call:shlex.quote, call:self.run, call:logger.info, call:json.loads, raise:RuntimeError | dep: json, logging, posixpath, shlex, dataclasses, io, pathlib, typing, paramiko, media_library_viewer_api.services.known_hosts ## arch -Adapter/wrapper pattern around third-party APIs and protocols, abstracting remote services into uniform Python dictionary-returning interfaces with decoupled timeout management via the requests library. +Thin wrapper pattern around `requests` and SSH/local execution, with each client class encapsulating connection management, authentication, and response normalization for a specific upstream service. ## tags call:logger.info, error, call:logger.debug, call:self.get, client, init, timeout, call:logger.warning ## symbols diff --git a/backend/src/media_library_viewer_api/clients/qbittorrent.py b/backend/src/media_library_viewer_api/clients/qbittorrent.py index f0b179b..e99799f 100644 --- a/backend/src/media_library_viewer_api/clients/qbittorrent.py +++ b/backend/src/media_library_viewer_api/clients/qbittorrent.py @@ -74,20 +74,25 @@ class QbittorrentClient: ) resp.raise_for_status() body = resp.text.strip() - # Some reverse proxies forward the SID cookie but mangle the text body; - # accept either success signal. Guard the cookie read behind an empty - # body so a mocked response never accidentally reads as success. - sid_ok = body == "" and bool(resp.cookies.get("SID")) + # qBittorrent signals a successful login with the body "Ok." and/or by + # setting a SID session cookie. Some setups (newer qBittorrent, or some + # reverse proxies) return 204 No Content with the SID cookie and no + # body. ``requests`` can also fail to populate the cookie jar for + # cookies with attributes it doesn't parse, so check the raw + # Set-Cookie header as well. qBittorrent only sets SID on a valid login. + set_cookie_hdr = resp.headers.get("Set-Cookie", "") or "" + first_cookie_name = set_cookie_hdr.split("=", 1)[0].strip().upper() + sid_ok = bool(resp.cookies.get("SID")) or first_cookie_name == "SID" if body == "Ok." or sid_ok: self._logged_in = True logger.info("qBittorrent login successful for %s", self.base_url) return if body == "Fails.": raise RuntimeError(f"qBittorrent login failed (HTTP {resp.status_code}): invalid username or password") - set_cookie = "yes" if resp.headers.get("Set-Cookie") else "no" + cookie_names = sorted(resp.cookies.keys()) or ([""] if set_cookie_hdr else []) raise RuntimeError( f"Unexpected response from qBittorrent login endpoint (HTTP {resp.status_code}, " - f"body={body!r}, set-cookie={set_cookie}). Expected the text 'Ok.' (or an SID cookie) " + f"body={body!r}, cookies={cookie_names}). Expected the text 'Ok.' or an SID cookie " "from /api/v2/auth/login — this usually means base_url does not reach the qBittorrent " "Web API (check the URL, path, and any reverse proxy in front of qBittorrent)." ) diff --git a/backend/tests/test_qbittorrent_client.py b/backend/tests/test_qbittorrent_client.py index a2c8ca0..330765c 100644 --- a/backend/tests/test_qbittorrent_client.py +++ b/backend/tests/test_qbittorrent_client.py @@ -21,6 +21,8 @@ class QbittorrentClientTests(unittest.TestCase): resp.text = text resp.raise_for_status.return_value = None resp.status_code = 200 + resp.headers = {} + resp.cookies = {} return resp def _get_response(self, json_data: dict, status_code: int = 200) -> MagicMock: @@ -161,6 +163,24 @@ class QbittorrentClientTests(unittest.TestCase): self.assertTrue(self.client._logged_in) + def test_login_accepts_sid_cookie_in_header_on_204(self) -> None: + """204 + Set-Cookie SID with no body (no jar entry) is a valid login. + + Reproduces the reported case: qBittorrent (or its reverse proxy) returns + 204 No Content with an SID cookie, and requests doesn't always populate + the cookie jar from such a header, so the cookie must be detected from + the raw Set-Cookie header. + """ + resp = self._login_response("") + resp.status_code = 204 + resp.cookies = {} # NOT in the parsed jar + resp.headers = {"Set-Cookie": "SID=abc123; HttpOnly; path=/"} + self.session.post.return_value = resp + + self.client._login() + + self.assertTrue(self.client._logged_in) + def test_login_empty_body_without_cookie_is_diagnostic(self) -> None: """Empty 200 body with no SID surfaces a URL/proxy diagnostic hint.""" resp = self._login_response("")