yorickr/plezy
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

fix(jellyfin): ask the server who may delete before offering it

Browse Source 
Jellyfin never consults IsAdministrator when authorizing a library
delete: BaseItem.IsAuthorizedToDelete looks at EnableContentDeletion and
the per-library grant, and only the first user a server creates gets the
former for free. Gating the "Delete from server" entry on the admin bit
therefore offered a destructive action that answers 401 to later
administrators, and hid it from plain users who do hold the grant.

Ask the server per item instead, through the new
MediaDeletionPermissionClient capability: BaseItemDto.CanDelete already
folds the global grant, the per-library grant, and item state such as
missing files or an in-progress recording. The probe runs when a menu
opens on a deletable kind, costs ~0.5 KB, carries a whole-request
deadline because the client's own budget covers connect and receive
separately, and fails closed on anything unknown. Plex keeps its
account-level owner/admin gate; it has no per-item permission on the
wire.

close #1749
This commit is contained in:
edde746
2026-08-02 07:08:14 +02:00
parent bc0d14a749
commit 2a7e5f4f9c
7 changed files with 615 additions and 10 deletions
Download Patch File Download Diff File Expand all files Collapse all files
test/services/jellyfin_delete_permission_test.dart
+190
View File
@@ -0,0 +1,190 @@
import 'dart:async';
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/exceptions/media_server_exceptions.dart';
import 'package:plezy/media/media_backend.dart';
import 'package:plezy/media/media_item.dart';
import 'package:plezy/media/media_kind.dart';
import 'package:plezy/media/media_server_client.dart';
import 'package:plezy/services/jellyfin_client.dart';
import 'package:plezy/utils/media_server_timeouts.dart';
import '../test_helpers/backend_client_fixtures.dart';
import '../test_helpers/http_fixtures.dart';
import '../test_helpers/media_items.dart';
/// `CanDelete` is what the server itself checks before honouring
/// `DELETE /Items/{id}` (`BaseItem.CanDelete(user)`), so it folds the global
/// `EnableContentDeletion` grant, the per-library grant, and item state. The
/// client must report it verbatim and never invent an answer.
void main() {
final item = testMediaItem(
id: 'movie-1',
backend: MediaBackend.jellyfin,
kind: MediaKind.movie,
title: 'Movie',
serverId: 'srv-1',
);
({JellyfinClient client, List<Uri> requests}) clientAnswering(
Future<http.Response> Function(http.Request request) handler,
) {
final requests = <Uri>[];
final client = JellyfinClient.forTesting(
connection: testJellyfinConnection(),
httpClient: MockClient((request) async {
requests.add(request.url);
return handler(request);
}),
);
addTearDown(client.close);
return (client: client, requests: requests);
}
group('JellyfinClient.fetchDeletePermission', () {
test('asks the list endpoint for CanDelete only, scoped to the item and user', () async {
final fake = clientAnswering(
(_) async => jsonResponse({
'Items': [
{'Id': 'movie-1', 'CanDelete': true},
],
}),
);
await (fake.client as MediaDeletionPermissionClient).fetchDeletePermission(item);
final uri = fake.requests.single;
// `/Items?ids=` and not `/Users/{id}/Items/{id}`: the single-item route
// ignores `Fields` and ships the whole dto.
expect(uri.path, '/Items');
expect(uri.queryParameters['ids'], 'movie-1');
expect(uri.queryParameters['userId'], 'user-1');
expect(uri.queryParameters['Fields'], 'CanDelete');
expect(uri.queryParameters['EnableImages'], 'false');
expect(uri.queryParameters['EnableUserData'], 'false');
});
test('reports the server answer for a permitted and a rejected item', () async {
final permitted = clientAnswering(
(_) async => jsonResponse({
'Items': [
{'Id': 'movie-1', 'CanDelete': true},
],
}),
);
final rejected = clientAnswering(
(_) async => jsonResponse({
'Items': [
{'Id': 'movie-1', 'CanDelete': false},
],
}),
);
expect(await (permitted.client as MediaDeletionPermissionClient).fetchDeletePermission(item), isTrue);
expect(await (rejected.client as MediaDeletionPermissionClient).fetchDeletePermission(item), isFalse);
});
test('treats an item the user cannot see as not deletable', () async {
final fake = clientAnswering((_) async => jsonResponse({'Items': <Object>[]}));
expect(await (fake.client as MediaDeletionPermissionClient).fetchDeletePermission(item), isFalse);
});
test('returns null when the server omits CanDelete', () async {
// Old servers, or a future one that stops honouring the field: unknown is
// not "allowed", and the caller fails closed on null.
final fake = clientAnswering(
(_) async => jsonResponse({
'Items': [
{'Id': 'movie-1'},
],
}),
);
expect(await (fake.client as MediaDeletionPermissionClient).fetchDeletePermission(item), isNull);
});
test('throws on an error response instead of reporting a permission', () async {
final fake = clientAnswering((_) async => http.Response('nope', 401));
await expectLater(
(fake.client as MediaDeletionPermissionClient).fetchDeletePermission(item),
throwsA(isA<MediaServerHttpException>().having((e) => e.statusCode, 'statusCode', 401)),
);
});
test('gives up on a server that never answers', () {
fakeAsync((async) {
final client = JellyfinClient.forTesting(
connection: testJellyfinConnection(),
httpClient: MockClient((_) => Completer<http.Response>().future),
);
addTearDown(client.close);
final failure = _failureOf(client, item);
async.elapse(MediaServerTimeouts.jellyfinDeletePermission - const Duration(milliseconds: 1));
expect(failure(), isNull);
async.elapse(const Duration(milliseconds: 2));
expect(failure(), _isProbeTimeout);
});
});
test('caps a slow connect followed by a stalled body at the whole-probe deadline', () {
// The regression this guards: the client applies its per-request budget
// to the connect and the receive phase separately, so a server that
// answers late and then stops sending would hold the context menu for
// roughly twice the budget without the caller-side deadline.
fakeAsync((async) {
final body = StreamController<List<int>>();
addTearDown(body.close);
final client = JellyfinClient.forTesting(
connection: testJellyfinConnection(),
httpClient: MockClient.streaming((_, _) async {
await Future<void>.delayed(MediaServerTimeouts.jellyfinDeletePermission - const Duration(seconds: 1));
return http.StreamedResponse(body.stream, 200, headers: const {'content-type': 'application/json'});
}),
);
addTearDown(client.close);
final failure = _failureOf(client, item);
async.elapse(MediaServerTimeouts.jellyfinDeletePermission - const Duration(milliseconds: 1));
expect(failure(), isNull, reason: 'the response arrived in time; only the body is stalled');
async.elapse(const Duration(milliseconds: 2));
expect(failure(), _isProbeTimeout);
expect(async.elapsed, lessThan(MediaServerTimeouts.jellyfinDeletePermission * 2));
});
});
});
}
/// Starts the probe and hands back a getter for whatever it failed with, so a
/// [fakeAsync] body can advance the clock and inspect the outcome without
/// awaiting inside the zone.
Object? Function() _failureOf(JellyfinClient client, MediaItem item) {
Object? failure;
(client as MediaDeletionPermissionClient)
.fetchDeletePermission(item)
.then<void>(
(_) {},
onError: (Object e) {
failure = e;
},
);
return () => failure;
}
/// Both stalls must surface as the canonical media-server failure, the same
/// shape `MediaServerHttpClient` raises for its own per-phase expiries, so
/// callers need one catch and not two.
final _isProbeTimeout = isA<MediaServerHttpException>().having(
(e) => e.type,
'type',
MediaServerHttpErrorType.connectionTimeout,
);
test/widgets/media_context_menu_test.dart
+253
View File
@@ -27,6 +27,7 @@ import 'package:plezy/profiles/profile.dart';
import 'package:plezy/profiles/active_profile_provider.dart';
import 'package:plezy/providers/download_provider.dart';
import 'package:plezy/providers/multi_server_provider.dart';
import 'package:plezy/providers/offline_mode_provider.dart';
import 'package:plezy/providers/playback_state_provider.dart';
import 'package:plezy/screens/music/album_detail_screen.dart';
import 'package:plezy/screens/music/artist_detail_screen.dart';
@@ -40,12 +41,14 @@ import 'package:plezy/services/plex_api_cache.dart';
import 'package:plezy/services/settings_service.dart';
import 'package:plezy/theme/mono_theme.dart';
import 'package:plezy/utils/media_server_http_client.dart';
import 'package:plezy/utils/media_server_timeouts.dart';
import 'package:plezy/utils/platform_detector.dart';
import 'package:plezy/widgets/media_context_menu.dart';
import 'package:provider/provider.dart';
import '../test_helpers/backend_client_fixtures.dart';
import '../test_helpers/media_items.dart';
import '../test_helpers/multi_server_fixtures.dart';
import '../test_helpers/http_fixtures.dart';
import '../test_helpers/prefs.dart';
import '../test_helpers/profile_stack.dart';
import '../test_helpers/stub_music_playback_service.dart';
@@ -86,6 +89,173 @@ void main() {
});
});
group('isMediaDeletionAllowed', () {
test('Jellyfin follows the server answer, not the admin bit', () {
// Jellyfin's own check (`BaseItem.IsAuthorizedToDelete`) never consults
// `IsAdministrator` for library items, and only the first user created on
// a server gets `EnableContentDeletion` for free.
expect(
isMediaDeletionAllowed(
itemBackend: MediaBackend.jellyfin,
resolvedItemPermission: false,
isAdminActionAllowed: true,
),
isFalse,
);
expect(
isMediaDeletionAllowed(
itemBackend: MediaBackend.jellyfin,
resolvedItemPermission: true,
isAdminActionAllowed: false,
),
isTrue,
);
});
test('Jellyfin fails closed when the permission is unknown', () {
expect(
isMediaDeletionAllowed(
itemBackend: MediaBackend.jellyfin,
resolvedItemPermission: null,
isAdminActionAllowed: true,
),
isFalse,
);
});
test('Plex keeps its account-level gate', () {
expect(
isMediaDeletionAllowed(
itemBackend: MediaBackend.plex,
resolvedItemPermission: null,
isAdminActionAllowed: true,
),
isTrue,
);
expect(
isMediaDeletionAllowed(
itemBackend: MediaBackend.plex,
resolvedItemPermission: null,
isAdminActionAllowed: false,
),
isFalse,
);
});
test('an item with no backend marker is never deletable', () {
expect(
isMediaDeletionAllowed(itemBackend: null, resolvedItemPermission: true, isAdminActionAllowed: true),
isFalse,
);
});
});
group('MediaContextMenu delete gate', () {
testWidgets('hides delete for an administrator the server refuses (issue #1749)', (tester) async {
final menuKey = await _pumpJellyfinMovieMenu(
tester,
isAdministrator: true,
handler: (_) async => _canDeleteResponse('movie-1', false),
);
await _openMenu(tester, menuKey);
expect(find.text(t.mediaMenu.deleteFromServer), findsNothing);
});
testWidgets('shows delete for a non-admin the server permits', (tester) async {
final requests = <Uri>[];
final menuKey = await _pumpJellyfinMovieMenu(
tester,
isAdministrator: false,
requests: requests,
handler: (_) async => _canDeleteResponse('movie-1', true),
);
await _openMenu(tester, menuKey);
expect(find.text(t.mediaMenu.deleteFromServer), findsOneWidget);
final probes = requests.where((uri) => uri.queryParameters['Fields'] == 'CanDelete').toList();
expect(probes, hasLength(1));
expect(probes.single.queryParameters['ids'], 'movie-1');
});
testWidgets('shows delete for a library-granted user inside the grant', (tester) async {
final menuKey = await _pumpJellyfinMovieMenu(
tester,
isAdministrator: false,
itemId: 'movie-in-grant',
handler: _libraryGrantedToOneMovie,
);
await _openMenu(tester, menuKey);
expect(find.text(t.mediaMenu.deleteFromServer), findsOneWidget);
});
testWidgets('hides delete for a library-granted user outside the grant', (tester) async {
final menuKey = await _pumpJellyfinMovieMenu(
tester,
isAdministrator: false,
itemId: 'movie-outside-grant',
handler: _libraryGrantedToOneMovie,
);
await _openMenu(tester, menuKey);
expect(find.text(t.mediaMenu.deleteFromServer), findsNothing);
});
testWidgets('hides delete when the permission probe fails', (tester) async {
final menuKey = await _pumpJellyfinMovieMenu(
tester,
isAdministrator: true,
handler: (_) async => http.Response('boom', 500),
);
await _openMenu(tester, menuKey);
expect(find.text(t.mediaMenu.deleteFromServer), findsNothing);
expect(tester.takeException(), isNull);
// The menu itself still opened; only the destructive entry is missing.
expect(find.text(t.mediaMenu.fileInfo), findsOneWidget);
});
testWidgets('hides delete without probing while the item server is offline', (tester) async {
final requests = <Uri>[];
final menuKey = await _pumpJellyfinMovieMenu(
tester,
isAdministrator: true,
serverOnline: false,
requests: requests,
handler: (_) async => _canDeleteResponse('movie-1', true),
);
await _openMenu(tester, menuKey);
expect(find.text(t.mediaMenu.deleteFromServer), findsNothing);
expect(requests.where((uri) => uri.queryParameters['Fields'] == 'CanDelete'), isEmpty);
});
testWidgets('still opens the menu, without delete, when the probe hangs', (tester) async {
final menuKey = await _pumpJellyfinMovieMenu(
tester,
isAdministrator: true,
handler: (_) => Completer<http.Response>().future,
);
menuKey.currentState!.showContextMenu(tester.element(find.text('delete target')));
await tester.pump();
expect(find.text(t.mediaMenu.fileInfo), findsNothing, reason: 'the menu waits for the permission answer');
await tester.pump(MediaServerTimeouts.jellyfinDeletePermission + const Duration(milliseconds: 1));
await tester.pumpAndSettle();
expect(find.text(t.mediaMenu.fileInfo), findsOneWidget);
expect(find.text(t.mediaMenu.deleteFromServer), findsNothing);
});
});
group('supportsMetadataEdit', () {
test('allows Jellyfin video metadata edit through capability gate', () {
final client = JellyfinClient.forTesting(
@@ -612,6 +782,89 @@ Future<GlobalKey<MediaContextMenuState>> _pumpPlexMovieMenu(
return menuKey;
}
/// Jellyfin answer for the per-item delete-permission probe.
http.Response _canDeleteResponse(String id, bool canDelete) => jsonResponse({
'Items': [
{'Id': id, 'CanDelete': canDelete},
],
});
/// A user holding `EnableContentDeletionFromFolders` for one library only: the
/// server answers per item, which is the only way that grant reaches a client.
Future<http.Response> _libraryGrantedToOneMovie(http.Request request) async {
final id = request.url.queryParameters['ids'] ?? '';
return _canDeleteResponse(id, id == 'movie-in-grant');
}
Future<GlobalKey<MediaContextMenuState>> _pumpJellyfinMovieMenu(
WidgetTester tester, {
required bool isAdministrator,
required Future<http.Response> Function(http.Request request) handler,
String itemId = 'movie-1',
bool serverOnline = true,
List<Uri>? requests,
}) async {
LocaleSettings.setLocaleSync(AppLocale.en);
TvDetectionService.debugSetAppleTVOverride(true);
addTearDown(() => TvDetectionService.debugSetAppleTVOverride(null));
final client = JellyfinClient.forTesting(
connection: testJellyfinConnection(isAdministrator: isAdministrator),
httpClient: MockClient((request) async {
requests?.add(request.url);
return handler(request);
}),
);
final manager = MultiServerManager()..debugRegisterJellyfinClientForTesting(client, online: serverOnline);
final multiServerProvider = testMultiServerProvider(manager);
final offlineMode = OfflineModeProvider(manager);
final stack = await ProfileStack.create(withStorage: false);
addTearDown(() async {
await stack.dispose();
offlineMode.dispose();
multiServerProvider.dispose();
manager.dispose();
});
final menuKey = GlobalKey<MediaContextMenuState>();
final item = testMediaItem(
id: itemId,
backend: MediaBackend.jellyfin,
kind: MediaKind.movie,
title: 'Movie',
serverId: 'srv-1',
);
await tester.pumpWidget(
TranslationProvider(
child: MultiProvider(
providers: [
ChangeNotifierProvider<MultiServerProvider>.value(value: multiServerProvider),
ChangeNotifierProvider<ActiveProfileProvider>.value(value: stack.active),
ChangeNotifierProvider<OfflineModeProvider>.value(value: offlineMode),
],
child: MaterialApp(
theme: monoTheme(dark: true),
home: Scaffold(
body: Center(
child: MediaContextMenu(
key: menuKey,
item: item,
child: const SizedBox(width: 120, height: 80, child: Text('delete target')),
),
),
),
),
),
),
);
return menuKey;
}
Future<void> _openMenu(WidgetTester tester, GlobalKey<MediaContextMenuState> menuKey) async {
menuKey.currentState!.showContextMenu(tester.element(find.text('delete target')));
await tester.pumpAndSettle();
}
Future<void> _openPlaylistPicker(WidgetTester tester, GlobalKey<MediaContextMenuState> menuKey) async {
menuKey.currentState!.showContextMenu(tester.element(find.text('picker target')));
await tester.pumpAndSettle();