From 65d3d8e4715fb97c2c06b5a23d81d77d568c8514 Mon Sep 17 00:00:00 2001 From: edde746 <86283021+edde746@users.noreply.github.com> Date: Sun, 12 Jul 2026 04:12:12 +0200 Subject: [PATCH] refactor(jellyfin): share auth and time handling --- lib/services/jellyfin_auth_service.dart | 164 ++++++++++-------- .../jellyfin_client/parts/live_tv.dart | 9 +- .../jellyfin_client/parts/playlists.dart | 10 +- .../jellyfin_sequential_launcher.dart | 26 ++- test/services/jellyfin_auth_service_test.dart | 106 ++++++++++- test/utils/jellyfin_time_test.dart | 30 ++++ 6 files changed, 240 insertions(+), 105 deletions(-) create mode 100644 test/utils/jellyfin_time_test.dart diff --git a/lib/services/jellyfin_auth_service.dart b/lib/services/jellyfin_auth_service.dart index 352ea86c..5dac5ee2 100644 --- a/lib/services/jellyfin_auth_service.dart +++ b/lib/services/jellyfin_auth_service.dart @@ -24,6 +24,20 @@ class JellyfinQuickConnectInitiation { const JellyfinQuickConnectInitiation({required this.code, required this.secret}); } +class _JellyfinAuthenticationResponse { + final String accessToken; + final String userId; + final String userName; + final bool isAdministrator; + + const _JellyfinAuthenticationResponse({ + required this.accessToken, + required this.userId, + required this.userName, + required this.isAdministrator, + }); +} + /// Auth flow for adding or refreshing a [JellyfinConnection]. /// /// Lifecycle for adding a server: @@ -114,53 +128,28 @@ class JellyfinConnectionAuthService implements ConnectionAuthService { headers: {'Authorization': authHeader, 'Content-Type': 'application/json'}, ); try { - final response = await client.post( - '/Users/AuthenticateByName', - body: jsonEncode({'Username': username, 'Pw': password}), - // Bound the auth POST so a hanging server can't freeze the auth - // screen indefinitely; mirrors the timeout on [probe]. - timeout: MediaServerTimeouts.jellyfinProbe, + final auth = await _readAuthenticationResponse( + client.post( + '/Users/AuthenticateByName', + body: jsonEncode({'Username': username, 'Pw': password}), + timeout: MediaServerTimeouts.jellyfinProbe, + ), + rejectedStatusCodes: const {401, 403}, + rejectionMessage: 'Invalid username or password', + responseLabel: 'Authentication response', + notJsonMessage: 'Authentication response was not JSON', ); - if (response.statusCode == 401 || response.statusCode == 403) { - throw MediaServerAuthException('Invalid username or password', statusCode: response.statusCode); - } - throwIfHttpError(response); - final data = response.data; - if (data is! Map) { - throw MediaServerAuthException('Authentication response was not JSON'); - } - final accessToken = data['AccessToken'] as String?; - final user = data['User'] as Map?; - if (accessToken == null || user == null) { - throw MediaServerAuthException('Authentication response missing AccessToken or User'); - } - final userId = user['Id'] as String?; - final userName = user['Name'] as String?; - if (userId == null || userName == null) { - throw MediaServerAuthException('Authentication response missing User.Id or User.Name'); - } - final policy = user['Policy'] as Map?; - final isAdmin = policy?['IsAdministrator'] as bool? ?? false; return _buildConnection( info: info, normalisedBaseUrl: normalised, baseUrls: baseUrls, - userId: userId, - userName: userName, - accessToken: accessToken, + userId: auth.userId, + userName: auth.userName, + accessToken: auth.accessToken, deviceId: deviceId, - isAdministrator: isAdmin, + isAdministrator: auth.isAdministrator, ); - } on TimeoutException { - // Defensive: most request timeouts are wrapped by MediaServerHttpClient. - // Surface raw timeouts as a URL-level error if one escapes. - throw MediaServerUrlException('Server did not respond in time'); - } on MediaServerHttpException catch (e) { - if (e.statusCode == 401 || e.statusCode == 403) { - throw MediaServerAuthException('Invalid username or password', statusCode: e.statusCode); - } - rethrow; } finally { client.close(); } @@ -304,49 +293,28 @@ class JellyfinConnectionAuthService implements ConnectionAuthService { headers: {'Authorization': authHeader, 'Content-Type': 'application/json'}, ); try { - final response = await exchangeClient.post( - '/Users/AuthenticateWithQuickConnect', - body: jsonEncode({'Secret': secret}), + final auth = await _readAuthenticationResponse( + exchangeClient.post( + '/Users/AuthenticateWithQuickConnect', + body: jsonEncode({'Secret': secret}), + timeout: MediaServerTimeouts.jellyfinProbe, + ), + rejectedStatusCodes: const {400, 401, 403}, + rejectionMessage: 'Quick Connect exchange rejected by server', + responseLabel: 'Quick Connect exchange', + notJsonMessage: 'Quick Connect exchange response was not JSON', ); - if (response.statusCode == 400) { - throw MediaServerAuthException('Quick Connect exchange rejected by server', statusCode: response.statusCode); - } - if (response.statusCode == 401 || response.statusCode == 403) { - throw MediaServerAuthException('Quick Connect exchange rejected by server', statusCode: response.statusCode); - } - throwIfHttpError(response); - final data = response.data; - if (data is! Map) { - throw MediaServerAuthException('Quick Connect exchange response was not JSON'); - } - final accessToken = data['AccessToken'] as String?; - final user = data['User'] as Map?; - if (accessToken == null || user == null) { - throw MediaServerAuthException('Quick Connect exchange missing AccessToken or User'); - } - final userId = user['Id'] as String?; - final userName = user['Name'] as String?; - if (userId == null || userName == null) { - throw MediaServerAuthException('Quick Connect exchange missing User.Id or User.Name'); - } - final policy = user['Policy'] as Map?; - final isAdmin = policy?['IsAdministrator'] as bool? ?? false; return _buildConnection( info: info, normalisedBaseUrl: normalised, baseUrls: baseUrls, - userId: userId, - userName: userName, - accessToken: accessToken, + userId: auth.userId, + userName: auth.userName, + accessToken: auth.accessToken, deviceId: deviceId, - isAdministrator: isAdmin, + isAdministrator: auth.isAdministrator, ); - } on MediaServerHttpException catch (e) { - if (e.statusCode == 400 || e.statusCode == 401 || e.statusCode == 403) { - throw MediaServerAuthException('Quick Connect exchange rejected by server', statusCode: e.statusCode); - } - rethrow; } finally { exchangeClient.close(); } @@ -412,6 +380,54 @@ class JellyfinConnectionAuthService implements ConnectionAuthService { /// produce double slashes. Delegates to the shared [stripTrailingSlash]. static String _normaliseBaseUrl(String input) => JellyfinEndpointDiscovery.normalizeBaseUrl(input); + static Future<_JellyfinAuthenticationResponse> _readAuthenticationResponse( + Future responseFuture, { + required Set rejectedStatusCodes, + required String rejectionMessage, + required String responseLabel, + required String notJsonMessage, + }) async { + try { + final response = await responseFuture; + if (rejectedStatusCodes.contains(response.statusCode)) { + throw MediaServerAuthException(rejectionMessage, statusCode: response.statusCode); + } + throwIfHttpError(response); + + final data = response.data; + if (data is! Map) { + throw MediaServerAuthException(notJsonMessage); + } + final accessToken = data['AccessToken'] as String?; + final user = data['User'] as Map?; + if (accessToken == null || user == null) { + throw MediaServerAuthException('$responseLabel missing AccessToken or User'); + } + final userId = user['Id'] as String?; + final userName = user['Name'] as String?; + if (userId == null || userName == null) { + throw MediaServerAuthException('$responseLabel missing User.Id or User.Name'); + } + final policy = user['Policy'] as Map?; + return _JellyfinAuthenticationResponse( + accessToken: accessToken, + userId: userId, + userName: userName, + isAdministrator: policy?['IsAdministrator'] as bool? ?? false, + ); + } on TimeoutException { + // MediaServerHttpClient normally wraps timeouts, but keep raw client + // implementations aligned with the same auth policy. + throw MediaServerUrlException('Server did not respond in time'); + } on MediaServerHttpException catch (e) { + final status = e.statusCode; + if (status != null && rejectedStatusCodes.contains(status)) { + throw MediaServerAuthException(rejectionMessage, statusCode: status); + } + rethrow; + } + } + /// Build a [JellyfinConnection] from a successful auth/exchange response. /// Connection id is derived from `(machineId, userId)` so each user on a /// given server has a single stable connection row. diff --git a/lib/services/jellyfin_client/parts/live_tv.dart b/lib/services/jellyfin_client/parts/live_tv.dart index bbac841c..77ff4a6c 100644 --- a/lib/services/jellyfin_client/parts/live_tv.dart +++ b/lib/services/jellyfin_client/parts/live_tv.dart @@ -73,11 +73,6 @@ mixin _JellyfinLiveTvMethods on MediaServerCacheMixin { LiveTvProgram _programFromJson(Map json) { final id = json['Id'] as String?; - int? toEpochSec(dynamic raw) { - if (raw is! String || raw.isEmpty) return null; - final ms = DateTime.tryParse(raw)?.toUtc().millisecondsSinceEpoch; - return ms != null ? ms ~/ 1000 : null; - } final tags = json['ImageTags']; String? primaryTag; @@ -95,8 +90,8 @@ mixin _JellyfinLiveTvMethods on MediaServerCacheMixin { summary: json['Overview'] as String?, type: 'episode', year: (json['ProductionYear'] as num?)?.toInt(), - beginsAt: toEpochSec(json['StartDate']), - endsAt: toEpochSec(json['EndDate']), + beginsAt: jellyfinIsoToEpochSeconds(json['StartDate'] as String?), + endsAt: jellyfinIsoToEpochSeconds(json['EndDate'] as String?), grandparentTitle: json['SeriesName'] as String?, parentTitle: json['SeasonName'] as String?, index: (json['IndexNumber'] as num?)?.toInt(), diff --git a/lib/services/jellyfin_client/parts/playlists.dart b/lib/services/jellyfin_client/parts/playlists.dart index 05c2030a..929d2fb7 100644 --- a/lib/services/jellyfin_client/parts/playlists.dart +++ b/lib/services/jellyfin_client/parts/playlists.dart @@ -239,8 +239,8 @@ mixin _JellyfinPlaylistMethods on MediaServerCacheMixin { smart: false, playlistType: (json['MediaType'] as String?)?.toLowerCase() ?? 'video', leafCount: json['ChildCount'] as int?, - addedAt: _epochSecondsFromJson(json['DateCreated'] as String?), - updatedAt: _epochSecondsFromJson(json['DateLastSaved'] as String?), + addedAt: jellyfinIsoToEpochSeconds(json['DateCreated'] as String?), + updatedAt: jellyfinIsoToEpochSeconds(json['DateLastSaved'] as String?), thumbPath: _absolutizeImagePath(_imageTagPath(id, json['ImageTags'])), serverId: serverId, serverName: serverName, @@ -259,12 +259,6 @@ mixin _JellyfinPlaylistMethods on MediaServerCacheMixin { return true; } - int? _epochSecondsFromJson(String? iso) { - if (iso == null || iso.isEmpty) return null; - final dt = DateTime.tryParse(iso); - return dt == null ? null : dt.millisecondsSinceEpoch ~/ 1000; - } - String? _imageTagPath(String id, Object? tags) { if (tags is! Map) return null; final tag = tags['Primary']; diff --git a/lib/services/jellyfin_sequential_launcher.dart b/lib/services/jellyfin_sequential_launcher.dart index e78163d3..944cf92d 100644 --- a/lib/services/jellyfin_sequential_launcher.dart +++ b/lib/services/jellyfin_sequential_launcher.dart @@ -70,11 +70,7 @@ class JellyfinSequentialLauncher extends MediaListPlaybackLauncher { execute: (dismissLoading) async { final client = clientForTesting ?? _resolveClient(ServerId(serverId)); if (client == null) { - await dismissLoading(); - if (context.mounted) { - showErrorSnackBar(context, t.errors.noClientAvailable); - } - return PlayQueueError(Exception('No client for server $serverId')); + return _missingClientError(serverId, dismissLoading); } // Playlists go through the dedicated `/Playlists/{id}/Items` endpoint @@ -145,11 +141,7 @@ class JellyfinSequentialLauncher extends MediaListPlaybackLauncher { execute: (dismissLoading) async { final client = clientForTesting ?? _resolveClient(ServerId(serverId)); if (client == null) { - await dismissLoading(); - if (context.mounted) { - showErrorSnackBar(context, t.errors.noClientAvailable); - } - return PlayQueueError(Exception('No client for server $serverId')); + return _missingClientError(serverId, dismissLoading); } final fetched = client is JellyfinClient @@ -221,11 +213,7 @@ class JellyfinSequentialLauncher extends MediaListPlaybackLauncher { execute: (dismissLoading) async { final client = clientForTesting ?? _resolveClient(ServerId(serverId)); if (client == null) { - await dismissLoading(); - if (context.mounted) { - showErrorSnackBar(context, t.errors.noClientAvailable); - } - return PlayQueueError(Exception('No client for server $serverId')); + return _missingClientError(serverId, dismissLoading); } final raw = await client.fetchClientSideEpisodeQueue(seriesId); @@ -266,4 +254,12 @@ class JellyfinSequentialLauncher extends MediaListPlaybackLauncher { final provider = Provider.of(context, listen: false); return provider.serverManager.getClient(serverId); } + + Future _missingClientError(String serverId, Future Function() dismissLoading) async { + await dismissLoading(); + if (context.mounted) { + showErrorSnackBar(context, t.errors.noClientAvailable); + } + return PlayQueueError(Exception('No client for server $serverId')); + } } diff --git a/test/services/jellyfin_auth_service_test.dart b/test/services/jellyfin_auth_service_test.dart index dd395605..d4069ab4 100644 --- a/test/services/jellyfin_auth_service_test.dart +++ b/test/services/jellyfin_auth_service_test.dart @@ -1,15 +1,19 @@ +import 'dart:async'; import 'dart:convert'; +import 'package:fake_async/fake_async.dart'; import 'package:flutter_test/flutter_test.dart'; import 'package:http/http.dart' as http; import 'package:http/testing.dart'; import 'package:plezy/connection/connection.dart'; import 'package:plezy/exceptions/media_server_exceptions.dart'; import 'package:plezy/services/jellyfin_auth_service.dart'; +import 'package:plezy/services/jellyfin_endpoint_discovery.dart'; import 'package:plezy/utils/log_redaction_manager.dart'; +import 'package:plezy/utils/media_server_timeouts.dart'; /// Helpers for stubbing http responses keyed by request path. -typedef _Handler = http.Response Function(http.BaseRequest req); +typedef _Handler = FutureOr Function(http.BaseRequest req); http.Response _ok(Object json) => http.Response(jsonEncode(json), 200, headers: {'content-type': 'application/json'}); http.Response _bareOk(String body) => http.Response(body, 200, headers: {'content-type': 'application/json'}); @@ -37,6 +41,17 @@ JellyfinConnectionAuthService _service({required _Handler handler}) { ); } +Future _captureError(Future future) async { + try { + await future; + } catch (error) { + return error; + } + throw StateError('Expected future to fail'); +} + +const _serverInfo = JellyfinServerInfo(serverName: 'Home', machineId: 'srv-1', version: '10.9.0'); + void main() { setUp(LogRedactionManager.clearTrackedValues); tearDown(LogRedactionManager.clearTrackedValues); @@ -451,6 +466,95 @@ void main() { }); }); + group('Jellyfin authentication response parity', () { + test('password and Quick Connect exchange use the same timeout', () { + fakeAsync((async) { + final passwordResponse = Completer(); + final quickConnectResponse = Completer(); + final passwordService = _service(handler: (_) => passwordResponse.future); + final quickConnectService = _service( + handler: (req) { + if (req.url.path == '/QuickConnect/Connect') return _ok({'Authenticated': true}); + return quickConnectResponse.future; + }, + ); + + Object? passwordError; + Object? quickConnectError; + unawaited( + _captureError( + passwordService.authenticateByName( + baseUrl: 'https://jf.example.com', + username: 'edde', + password: 'pw', + deviceId: 'dev-xyz', + serverInfo: _serverInfo, + ), + ).then((error) => passwordError = error), + ); + unawaited( + _captureError( + quickConnectService.authenticateByQuickConnect( + baseUrl: 'https://jf.example.com', + secret: 'sec', + deviceId: 'dev-xyz', + serverInfo: _serverInfo, + ), + ).then((error) => quickConnectError = error), + ); + + async.flushMicrotasks(); + expect(passwordError, isNull); + expect(quickConnectError, isNull); + + async.elapse(MediaServerTimeouts.jellyfinProbe + const Duration(milliseconds: 1)); + async.flushMicrotasks(); + + for (final error in [passwordError, quickConnectError]) { + expect( + error, + isA().having( + (exception) => exception.type, + 'type', + MediaServerHttpErrorType.connectionTimeout, + ), + ); + } + }); + }); + + test('password and Quick Connect exchange preserve non-auth HTTP errors', () async { + final passwordService = _service(handler: (_) => _status(500)); + final quickConnectService = _service( + handler: (req) => req.url.path == '/QuickConnect/Connect' ? _ok({'Authenticated': true}) : _status(500), + ); + + final errors = [ + await _captureError( + passwordService.authenticateByName( + baseUrl: 'https://jf.example.com', + username: 'edde', + password: 'pw', + deviceId: 'dev-xyz', + serverInfo: _serverInfo, + ), + ), + await _captureError( + quickConnectService.authenticateByQuickConnect( + baseUrl: 'https://jf.example.com', + secret: 'sec', + deviceId: 'dev-xyz', + serverInfo: _serverInfo, + ), + ), + ]; + + for (final error in errors) { + expect(error, isA().having((exception) => exception.statusCode, 'statusCode', 500)); + } + }); + }); + group('JellyfinConnectionAuthService.validate', () { test('returns true when /Users/Me responds 200', () async { final svc = _service( diff --git a/test/utils/jellyfin_time_test.dart b/test/utils/jellyfin_time_test.dart new file mode 100644 index 00000000..b4aa996a --- /dev/null +++ b/test/utils/jellyfin_time_test.dart @@ -0,0 +1,30 @@ +import 'package:flutter_test/flutter_test.dart'; +import 'package:plezy/utils/jellyfin_time.dart'; + +void main() { + group('Jellyfin time conversions', () { + test('converts ticks and milliseconds in both directions', () { + expect(jellyfinTicksToMs(12_345_678), 1234); + expect(jellyfinTicksToMs(12.5), 0); + expect(jellyfinTicksToMs('10000'), isNull); + expect(msToJellyfinTicks(1234), 12_340_000); + }); + + test('converts ISO timestamps to UTC epoch seconds', () { + expect(jellyfinIsoToEpochSeconds('1970-01-01T00:00:01.999Z'), 1); + expect(jellyfinIsoToEpochSeconds('1970-01-01T01:00:01+01:00'), 1); + }); + + test('returns null for missing or invalid ISO timestamps', () { + expect(jellyfinIsoToEpochSeconds(null), isNull); + expect(jellyfinIsoToEpochSeconds(''), isNull); + expect(jellyfinIsoToEpochSeconds('not-a-date'), isNull); + }); + + test('truncates ISO timestamps to the calendar date', () { + expect(jellyfinIsoToYmd('2026-07-12T09:30:00Z'), '2026-07-12'); + expect(jellyfinIsoToYmd('2026-07-12'), '2026-07-12'); + expect(jellyfinIsoToYmd(''), isNull); + }); + }); +}