From a2321c46a3fb80c52d3256cf18b160b98aa9aded Mon Sep 17 00:00:00 2001 From: Pablo Fontanilla Arranz Date: Thu, 24 Sep 2026 13:02:47 +0200 Subject: [PATCH 1/2] Send the Jellyfin download token in the Authorization header MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The download request put the access token only in an `?api_key=` query item. A Jellyfin 12.1.0 server rejects that with a 401, so downloads failed. The request now sets `Authorization: MediaBrowser Token="…"` after the custom headers, so a custom header cannot replace it. The `?api_key=` query item is removed, so the token does not go into the download task's persisted `taskDescription`. Refs #1598 Co-Authored-By: Claude Opus 5.5 --- .../Network/JellyfinConnectionService.swift | 22 +++++---- .../JellyfinQuickConnectTests.swift | 49 +++++++++++++++++++ 2 files changed, 62 insertions(+), 9 deletions(-) diff --git a/BookPlayer/Jellyfin/Network/JellyfinConnectionService.swift b/BookPlayer/Jellyfin/Network/JellyfinConnectionService.swift index f99a54aaf..65ecfd599 100644 --- a/BookPlayer/Jellyfin/Network/JellyfinConnectionService.swift +++ b/BookPlayer/Jellyfin/Network/JellyfinConnectionService.swift @@ -724,16 +724,12 @@ class JellyfinConnectionService: BPLogger { } func createItemDownloadUrl(_ item: JellyfinLibraryItem) throws -> URL { - guard let client else { + guard client != nil else { throw IntegrationError.noClient("Jellyfin") } let request = Paths.getDownload(itemID: item.id) - var components = try createUrlComponentsForApiRequest(request) - - var queryItems = components.queryItems ?? [] - queryItems.append(URLQueryItem(name: "api_key", value: client.accessToken)) - components.queryItems = queryItems + let components = try createUrlComponentsForApiRequest(request) guard let url = components.url else { throw IntegrationError.urlFromComponents(components) @@ -742,11 +738,19 @@ class JellyfinConnectionService: BPLogger { return url } - /// Returns a URLRequest for downloading a library item, carrying the user-defined - /// custom HTTP headers (needed for servers behind Cloudflare Access etc.). + /// Returns a URLRequest for downloading a library item, carrying the access token plus the + /// user-defined custom HTTP headers (needed for servers behind Cloudflare Access etc.). + /// + /// The token goes in the `Authorization` header, not in an `?api_key=` query item: Jellyfin 12 + /// servers can reject the query token with a 401, and a token in the URL leaks into the download task's + /// persisted `taskDescription`. It is set after the custom headers, so they can't clobber it. func createItemDownloadRequest(_ item: JellyfinLibraryItem) throws -> URLRequest { let url = try createItemDownloadUrl(item) - return wrapWithCustomHeaders(url) + var request = wrapWithCustomHeaders(url) + if let accessToken = client?.accessToken { + request.setValue("MediaBrowser Token=\"\(accessToken)\"", forHTTPHeaderField: "Authorization") + } + return request } /// Wraps an arbitrary URL (e.g. a cover image) in a URLRequest carrying the current diff --git a/BookPlayerTests/MediaServerIntegration/JellyfinQuickConnectTests.swift b/BookPlayerTests/MediaServerIntegration/JellyfinQuickConnectTests.swift index c4f42df80..b41589fb0 100644 --- a/BookPlayerTests/MediaServerIntegration/JellyfinQuickConnectTests.swift +++ b/BookPlayerTests/MediaServerIntegration/JellyfinQuickConnectTests.swift @@ -169,3 +169,52 @@ final class JellyfinQuickConnectTests: XCTestCase { } } } + +/// Covers the download request, which is built by hand instead of going through `JellyfinClient` +/// and so gets none of the client's automatic auth. Jellyfin 12 servers can reject the `?api_key=` query +/// token with a 401 and accept only the token in the `Authorization` header (#1598). +@MainActor +final class JellyfinDownloadRequestTests: XCTestCase { + private var sut: JellyfinConnectionService! + + override func setUp() { + super.setUp() + sut = JellyfinConnectionService(keychainService: KeychainServiceMock()) + sut.client = JellyfinClient( + configuration: JellyfinClient.Configuration( + url: URL(string: "https://jellyfin.example.com/jellyfin")!, + client: "BookPlayer", + deviceName: "Test Device", + deviceID: "test-device", + version: "1" + ), + accessToken: "s3cr3t" + ) + } + + override func tearDown() { + sut = nil + super.tearDown() + } + + private let audiobook = JellyfinLibraryItem(id: "item-1", name: "Book", kind: .audiobook) + + func testDownloadRequestCarriesTheTokenInTheAuthorizationHeader() throws { + let request = try sut.createItemDownloadRequest(audiobook) + + XCTAssertEqual( + request.value(forHTTPHeaderField: "Authorization"), + "MediaBrowser Token=\"s3cr3t\"" + ) + } + + /// The URL ends up in the download task's persisted `taskDescription`, so it must not carry the token. + func testDownloadURLDoesNotCarryTheToken() throws { + let request = try sut.createItemDownloadRequest(audiobook) + let url = try XCTUnwrap(request.url) + + XCTAssertEqual(url.path, "/jellyfin/Items/item-1/Download") + XCTAssertFalse(url.absoluteString.contains("s3cr3t")) + XCTAssertFalse(url.absoluteString.contains("api_key")) + } +} From 46bae6c23a324689630eb9d64f9487e6eaa52313 Mon Sep 17 00:00:00 2001 From: Gianni Carlo Date: Sun, 4 Oct 2026 23:33:15 -0500 Subject: [PATCH 2/2] Keep the bare Jellyfin download URL private createItemDownloadUrl no longer carries the token, so the URL alone gets a 401 on every Jellyfin version. Make it private with the same warning the AudiobookShelf builder has, so new callers go through createItemDownloadRequest, which adds the Authorization header. The client guard is gone: createUrlComponentsForApiRequest already throws noClient. The Sentry query-stripping comment no longer lists Jellyfin download URLs; AudiobookShelf's OIDC query still needs the stripping. --- BookPlayer/AppDelegate.swift | 5 ++--- .../Jellyfin/Network/JellyfinConnectionService.swift | 9 ++++----- 2 files changed, 6 insertions(+), 8 deletions(-) diff --git a/BookPlayer/AppDelegate.swift b/BookPlayer/AppDelegate.swift index 770d6d25b..3428fa247 100644 --- a/BookPlayer/AppDelegate.swift +++ b/BookPlayer/AppDelegate.swift @@ -351,9 +351,8 @@ class AppDelegate: UIResponder, UIApplicationDelegate, BPLogger { // the raw query back under `http.query`, on both network breadcrumbs and `http.client` spans. // Only the userinfo portion is redacted for us. That leaks real secrets from hosts we don't // control: AudiobookShelf's OIDC exchange has to carry `code` and `code_verifier` in the query - // (its endpoint is GET-only), and Jellyfin's download URLs still carry `api_key`. Either one - // would be uploaded verbatim alongside the user's self-hosted hostname on the next captured - // event. + // (its endpoint is GET-only). Both would be uploaded verbatim alongside the user's self-hosted + // hostname on the next captured event. // // Our own backend authenticates with a bearer header and puts nothing sensitive in a query, so // its query strings are kept — that's where this data actually helps debugging. diff --git a/BookPlayer/Jellyfin/Network/JellyfinConnectionService.swift b/BookPlayer/Jellyfin/Network/JellyfinConnectionService.swift index 65ecfd599..032182834 100644 --- a/BookPlayer/Jellyfin/Network/JellyfinConnectionService.swift +++ b/BookPlayer/Jellyfin/Network/JellyfinConnectionService.swift @@ -723,11 +723,10 @@ class JellyfinConnectionService: BPLogger { return client } - func createItemDownloadUrl(_ item: JellyfinLibraryItem) throws -> URL { - guard client != nil else { - throw IntegrationError.noClient("Jellyfin") - } - + /// Kept private because the returned URL is *not* self-authenticating — handing it + /// straight to `URLSession`/`AVURLAsset` would 401. Go through + /// `createItemDownloadRequest(_:)`, which attaches the token. + private func createItemDownloadUrl(_ item: JellyfinLibraryItem) throws -> URL { let request = Paths.getDownload(itemID: item.id) let components = try createUrlComponentsForApiRequest(request)