From 89af341b37e5f885c79bdb029af36dcb8a68b5f3 Mon Sep 17 00:00:00 2001 From: neon <18559658+neonsolstice@users.noreply.github.com> Date: Mon, 21 Sep 2026 01:00:45 +0000 Subject: [PATCH] fix(server): authenticate against qbittorrent 5.2 and newer qBittorrent 5.2 renamed its session cookie to QBT_SID_ and removed the setting that overrode the name, so reading only SID left the session empty and every call answered 403. Match both names, and report a 401 login as rejected credentials now that 5.2 answers a bad password that way instead of with "Fails.". Fixes #1306 --- .../adapters/qbittorrent.adapter.test.ts | 73 ++++++++++++++++++- .../adapters/qbittorrent.adapter.ts | 29 +++++++- 2 files changed, 97 insertions(+), 5 deletions(-) diff --git a/server/src/modules/book-request/download-clients/adapters/qbittorrent.adapter.test.ts b/server/src/modules/book-request/download-clients/adapters/qbittorrent.adapter.test.ts index c879ea812..bcb55ade8 100644 --- a/server/src/modules/book-request/download-clients/adapters/qbittorrent.adapter.test.ts +++ b/server/src/modules/book-request/download-clients/adapters/qbittorrent.adapter.test.ts @@ -1,5 +1,5 @@ import { Test } from '@nestjs/testing'; -import { BadRequestException } from '@nestjs/common'; +import { BadRequestException, Logger } from '@nestjs/common'; import type { ResolvedClientConfig } from '../download-client-adapter'; import { QbittorrentAdapter } from './qbittorrent.adapter'; @@ -25,8 +25,9 @@ function config(overrides: Partial = {}): ResolvedClientCo function response(body: string | object, init: { status?: number; setCookie?: string } = {}): Response { const headers = new Headers(); if (init.setCookie) headers.append('set-cookie', init.setCookie); + const status = init.status ?? 200; const payload = typeof body === 'string' ? body : JSON.stringify(body); - return new Response(payload, { status: init.status ?? 200, headers }); + return new Response(status === 204 ? null : payload, { status, headers }); } function mockFetch() { @@ -548,11 +549,77 @@ describe('QbittorrentAdapter', () => { expect(calls.filter((call) => call.url.includes('/auth/login'))).toHaveLength(2); }); + /** + * qBittorrent 5.2 renamed the cookie to `QBT_SID_` and answers the login with an + * empty 204. Reading only `SID` left the session empty, which every later call answered with a + * 403 that looked like a permission problem rather than an unrecognised cookie. + */ + it('accepts the 5.2 session cookie and its empty 204 login', async () => { + const { calls, handlers } = mockFetch(); + handlers.set('/auth/login', () => + response('', { status: 204, setCookie: 'QBT_SID_8080=xZE+G8m4; HttpOnly; expires=Mon, 21-Sep-2026 01:34:10 GMT; path=/' }), + ); + handlers.set('/torrents/info', () => response([])); + + await adapter.status([INFO_HASH], config()); + + const infoCall = calls.find((call) => call.url.includes('/torrents/info')); + expect((infoCall?.init.headers as Record).Cookie).toBe('QBT_SID_8080=xZE+G8m4'); + }); + + /** The cookie is named after qBittorrent's own WebUI port, not the port BookOrbit dials. */ + it('accepts a session cookie whose port differs from the configured one', async () => { + const { calls, handlers } = mockFetch(); + handlers.set('/auth/login', () => response('', { status: 204, setCookie: 'QBT_SID_18022=abc123; HttpOnly; path=/' })); + handlers.set('/torrents/info', () => response([])); + + await adapter.status([INFO_HASH], config({ baseUrl: 'http://127.0.0.1:9091' })); + + const infoCall = calls.find((call) => call.url.includes('/torrents/info')); + expect((infoCall?.init.headers as Record).Cookie).toBe('QBT_SID_18022=abc123'); + }); + it('reports rejected credentials rather than retrying forever', async () => { const { handlers } = mockFetch(); handlers.set('/auth/login', () => response('Fails.')); - await expect(adapter.test(config())).resolves.toMatchObject({ success: false }); + await expect(adapter.test(config())).resolves.toMatchObject({ success: false, error: 'qBittorrent rejected those credentials' }); + }); + + /** 5.2 answers a bad password with 401 rather than the older 200 "Fails.". */ + it('reports rejected credentials when the client answers 401', async () => { + const { handlers } = mockFetch(); + handlers.set('/auth/login', () => response('Unauthorized', { status: 401 })); + + await expect(adapter.test(config())).resolves.toMatchObject({ success: false, error: 'qBittorrent rejected those credentials' }); + }); + + /** A client that bans the caller after repeated failures says so, and the operator needs it. */ + it('passes on why the client refused the login with a 403', async () => { + const { handlers } = mockFetch(); + handlers.set('/auth/login', () => response('Your IP address has been banned after too many failed authentication attempts.', { status: 403 })); + + await expect(adapter.test(config())).resolves.toMatchObject({ + success: false, + error: 'qBittorrent refused the login: Your IP address has been banned after too many failed authentication attempts.', + }); + }); + + /** + * A login that sets nothing means authentication is disabled for this subnet, and the adapter + * carries on without a cookie. That assumption is also what an unrecognised cookie looks like, + * so it is worth a line in the log rather than a 403 three calls later. + */ + it('says so when a successful login sets no session cookie', async () => { + const warn = vi.spyOn(Logger.prototype, 'warn').mockImplementation(() => undefined); + const { calls, handlers } = mockFetch(); + handlers.set('/auth/login', () => response('Ok.')); + handlers.set('/app/version', () => response('v5.2.3')); + + await expect(adapter.test(config())).resolves.toEqual({ success: true, version: 'v5.2.3' }); + + expect((calls.at(-1)?.init.headers as Record).Cookie).toBe(''); + expect(warn).toHaveBeenCalledWith(expect.stringContaining('[download_client.login]')); }); }); diff --git a/server/src/modules/book-request/download-clients/adapters/qbittorrent.adapter.ts b/server/src/modules/book-request/download-clients/adapters/qbittorrent.adapter.ts index 6f4add7ed..8f5861f5d 100644 --- a/server/src/modules/book-request/download-clients/adapters/qbittorrent.adapter.ts +++ b/server/src/modules/book-request/download-clients/adapters/qbittorrent.adapter.ts @@ -17,6 +17,13 @@ import { endpointUrl, fetchClient, readClientJson, readClientText, throwForClien const LABEL = 'qBittorrent'; /** qBittorrent's SID cookie lasts an hour by default; re-login well before it lapses. */ const SESSION_TTL_MS = 30 * 60 * 1000; +/** + * qBittorrent 5.2 renamed the session cookie from `SID` to `QBT_SID_` and dropped the + * setting that used to override the name, so both spellings have to be recognised. The port is the + * one qBittorrent itself listens on, which is not the port BookOrbit dials when a reverse proxy or + * a published container port sits in between. + */ +const SESSION_COOKIE_NAME = /^(?:SID|QBT_SID_\d+)$/; /** One `torrents/info` URL has to stay a sane length, so very large fleets poll in chunks. */ const STATUS_BATCH_SIZE = 100; /** @@ -313,13 +320,29 @@ export class QbittorrentAdapter implements DownloadClientAdapter { if (!response.ok) { throwForClientServerError(response, LABEL, 'login'); + // 5.2 answers a bad password with 401 and the body "Unauthorized"; older clients answer + // 200 "Fails.". A ban after repeated failures is its own 403, and says so in its body. + if (response.status === 401) throw new BadRequestException('qBittorrent rejected those credentials'); + if (response.status === 403) { + // The client writes this, so it is bounded before it reaches the settings form. + const reason = (await readClientText(response, LABEL)).trim().slice(0, 200); + throw new BadRequestException(reason ? `qBittorrent refused the login: ${reason}` : 'qBittorrent refused the login with 403'); + } throw new BadRequestException(`qBittorrent refused the login with ${response.status}`); } + // 5.2 answers a successful login with an empty 204, so the body is only evidence of failure. const text = (await readClientText(response, LABEL)).trim(); if (text.toLowerCase().startsWith('fail')) throw new BadRequestException('qBittorrent rejected those credentials'); const cookie = extractSidCookie(response); // A client with authentication disabled for the local subnet answers "Ok." and sets nothing. + // So does one whose cookie BookOrbit failed to recognise, and that case is indistinguishable + // here but fails on the very next call with a 403, so say which assumption was made. + if (!cookie) { + this.logger.warn( + `[download_client.login] [end] clientId=${config.id} cookie=none - qBittorrent set no session cookie; assuming authentication is disabled for this subnet`, + ); + } const resolved = cookie ?? ''; this.sessions.set(config.id, { cookie: resolved, expiresAt: Date.now() + SESSION_TTL_MS }); return resolved; @@ -399,8 +422,10 @@ function extractSidCookie(response: Response): string | null { const headers = response.headers as Headers & { getSetCookie?: () => string[] }; const raw = headers.getSetCookie?.() ?? [response.headers.get('set-cookie') ?? '']; for (const value of raw) { - const match = /(?:^|;\s*)(SID=[^;]+)/.exec(value); - if (match) return match[1]; + // One `Set-Cookie` header carries one cookie; everything after the first `;` is its attributes. + const pair = value.split(';')[0]!.trim(); + const separator = pair.indexOf('='); + if (separator > 0 && SESSION_COOKIE_NAME.test(pair.slice(0, separator))) return pair; } return null; }