From 88b3490d1756236d0c2fc00243420d45d149a5d1 Mon Sep 17 00:00:00 2001 From: Shadowghost Date: Tue, 26 Mar 2024 15:29:48 +0100 Subject: [PATCH 01/17] Add playlist ACL endpoints --- .../Playlists/PlaylistManager.cs | 88 ++++++++----- .../Controllers/PlaylistsController.cs | 122 ++++++++++++++++++ .../Migrations/Routines/FixPlaylistOwner.cs | 2 +- .../Playlists/IPlaylistManager.cs | 35 +++++ MediaBrowser.Controller/Playlists/Playlist.cs | 29 ++--- MediaBrowser.Model/Entities/IHasShares.cs | 6 +- 6 files changed, 235 insertions(+), 47 deletions(-) diff --git a/Emby.Server.Implementations/Playlists/PlaylistManager.cs b/Emby.Server.Implementations/Playlists/PlaylistManager.cs index aea8d6532..6724d54d1 100644 --- a/Emby.Server.Implementations/Playlists/PlaylistManager.cs +++ b/Emby.Server.Implementations/Playlists/PlaylistManager.cs @@ -59,6 +59,11 @@ namespace Emby.Server.Implementations.Playlists _appConfig = appConfig; } + public Playlist GetPlaylist(Guid userId, Guid playlistId) + { + return GetPlaylists(userId).Where(p => p.Id.Equals(playlistId)).FirstOrDefault(); + } + public IEnumerable GetPlaylists(Guid userId) { var user = _userManager.GetUserById(userId); @@ -160,7 +165,7 @@ namespace Emby.Server.Implementations.Playlists } } - private string GetTargetPath(string path) + private static string GetTargetPath(string path) { while (Directory.Exists(path)) { @@ -231,13 +236,8 @@ namespace Emby.Server.Implementations.Playlists // Update the playlist in the repository playlist.LinkedChildren = newLinkedChildren; - await playlist.UpdateToRepositoryAsync(ItemUpdateType.MetadataEdit, CancellationToken.None).ConfigureAwait(false); - // Update the playlist on disk - if (playlist.IsFile) - { - SavePlaylistFile(playlist); - } + await UpdatePlaylist(playlist).ConfigureAwait(false); // Refresh playlist metadata _providerManager.QueueRefresh( @@ -266,12 +266,7 @@ namespace Emby.Server.Implementations.Playlists .Select(i => i.Item1) .ToArray(); - await playlist.UpdateToRepositoryAsync(ItemUpdateType.MetadataEdit, CancellationToken.None).ConfigureAwait(false); - - if (playlist.IsFile) - { - SavePlaylistFile(playlist); - } + await UpdatePlaylist(playlist).ConfigureAwait(false); _providerManager.QueueRefresh( playlist.Id, @@ -313,14 +308,9 @@ namespace Emby.Server.Implementations.Playlists newList.Insert(newIndex, item); } - playlist.LinkedChildren = newList.ToArray(); + playlist.LinkedChildren = [.. newList]; - await playlist.UpdateToRepositoryAsync(ItemUpdateType.MetadataEdit, CancellationToken.None).ConfigureAwait(false); - - if (playlist.IsFile) - { - SavePlaylistFile(playlist); - } + await UpdatePlaylist(playlist).ConfigureAwait(false); } /// @@ -430,8 +420,11 @@ namespace Emby.Server.Implementations.Playlists } else if (extension.Equals(".m3u8", StringComparison.OrdinalIgnoreCase)) { - var playlist = new M3uPlaylist(); - playlist.IsExtended = true; + var playlist = new M3uPlaylist + { + IsExtended = true + }; + foreach (var child in item.GetLinkedChildren()) { var entry = new M3uPlaylistEntry() @@ -481,7 +474,7 @@ namespace Emby.Server.Implementations.Playlists } } - private string NormalizeItemPath(string playlistPath, string itemPath) + private static string NormalizeItemPath(string playlistPath, string itemPath) { return MakeRelativePath(Path.GetDirectoryName(playlistPath), itemPath); } @@ -541,12 +534,7 @@ namespace Emby.Server.Implementations.Playlists { playlist.OwnerUserId = guid; playlist.Shares = rankedShares.Skip(1).ToArray(); - await playlist.UpdateToRepositoryAsync(ItemUpdateType.MetadataEdit, CancellationToken.None).ConfigureAwait(false); - - if (playlist.IsFile) - { - SavePlaylistFile(playlist); - } + await UpdatePlaylist(playlist).ConfigureAwait(false); } else if (!playlist.OpenAccess) { @@ -563,5 +551,47 @@ namespace Emby.Server.Implementations.Playlists } } } + + public async Task ToggleOpenAccess(Guid playlistId, Guid userId) + { + var playlist = GetPlaylist(userId, playlistId); + playlist.OpenAccess = !playlist.OpenAccess; + + await UpdatePlaylist(playlist).ConfigureAwait(false); + } + + public async Task AddToShares(Guid playlistId, Guid userId, Share share) + { + var playlist = GetPlaylist(userId, playlistId); + var shares = playlist.Shares.ToList(); + var existingUserShare = shares.FirstOrDefault(s => s.UserId?.Equals(share.UserId, StringComparison.OrdinalIgnoreCase) ?? false); + if (existingUserShare is not null) + { + shares.Remove(existingUserShare); + } + + shares.Add(share); + playlist.Shares = shares; + await UpdatePlaylist(playlist).ConfigureAwait(false); + } + + public async Task RemoveFromShares(Guid playlistId, Guid userId, Share share) + { + var playlist = GetPlaylist(userId, playlistId); + var shares = playlist.Shares.ToList(); + shares.Remove(share); + playlist.Shares = shares; + await UpdatePlaylist(playlist).ConfigureAwait(false); + } + + private async Task UpdatePlaylist(Playlist playlist) + { + await playlist.UpdateToRepositoryAsync(ItemUpdateType.MetadataEdit, CancellationToken.None).ConfigureAwait(false); + + if (playlist.IsFile) + { + SavePlaylistFile(playlist); + } + } } } diff --git a/Jellyfin.Api/Controllers/PlaylistsController.cs b/Jellyfin.Api/Controllers/PlaylistsController.cs index 0e7c3f155..f0e8227fd 100644 --- a/Jellyfin.Api/Controllers/PlaylistsController.cs +++ b/Jellyfin.Api/Controllers/PlaylistsController.cs @@ -98,6 +98,128 @@ public class PlaylistsController : BaseJellyfinApiController return result; } + /// + /// Get a playlist's shares. + /// + /// The playlist id. + /// + /// A list of objects. + /// + [HttpGet("{playlistId}/Shares")] + [ProducesResponseType(StatusCodes.Status200OK)] + public IReadOnlyList GetPlaylistShares( + [FromRoute, Required] Guid playlistId) + { + var userId = RequestHelpers.GetUserId(User, default); + + var playlist = _playlistManager.GetPlaylist(userId, playlistId); + var isPermitted = playlist.OwnerUserId.Equals(userId) + || playlist.Shares.Any(s => s.CanEdit && (s.UserId?.Equals(userId) ?? false)); + + return isPermitted ? playlist.Shares : new List(); + } + + /// + /// Toggles OpenAccess of a playlist. + /// + /// The playlist id. + /// + /// A that represents the asynchronous operation to toggle OpenAccess of a playlist. + /// The task result contains an indicating success. + /// + [HttpPost("{playlistId}/ToggleOpenAccess")] + [ProducesResponseType(StatusCodes.Status200OK)] + public async Task ToggleopenAccess( + [FromRoute, Required] Guid playlistId) + { + var callingUserId = RequestHelpers.GetUserId(User, default); + + var playlist = _playlistManager.GetPlaylist(callingUserId, playlistId); + var isPermitted = playlist.OwnerUserId.Equals(callingUserId) + || playlist.Shares.Any(s => s.CanEdit && (s.UserId?.Equals(callingUserId) ?? false)); + + if (!isPermitted) + { + return Unauthorized("Unauthorized access"); + } + + await _playlistManager.ToggleOpenAccess(playlistId, callingUserId).ConfigureAwait(false); + + return NoContent(); + } + + /// + /// Adds shares to a playlist's shares. + /// + /// The playlist id. + /// The shares. + /// + /// A that represents the asynchronous operation to add shares to a playlist. + /// The task result contains an indicating success. + /// + [HttpPost("{playlistId}/Shares")] + [ProducesResponseType(StatusCodes.Status200OK)] + public async Task AddUserToPlaylistShares( + [FromRoute, Required] Guid playlistId, + [FromBody(EmptyBodyBehavior = EmptyBodyBehavior.Disallow)] Share[] shares) + { + var callingUserId = RequestHelpers.GetUserId(User, default); + + var playlist = _playlistManager.GetPlaylist(callingUserId, playlistId); + var isPermitted = playlist.OwnerUserId.Equals(callingUserId) + || playlist.Shares.Any(s => s.CanEdit && (s.UserId?.Equals(callingUserId) ?? false)); + + if (!isPermitted) + { + return Unauthorized("Unauthorized access"); + } + + foreach (var share in shares) + { + await _playlistManager.AddToShares(playlistId, callingUserId, share).ConfigureAwait(false); + } + + return NoContent(); + } + + /// + /// Remove a user from a playlist's shares. + /// + /// The playlist id. + /// The user id. + /// + /// A that represents the asynchronous operation to delete a user from a playlist's shares. + /// The task result contains an indicating success. + /// + [HttpDelete("{playlistId}/Shares")] + [ProducesResponseType(StatusCodes.Status200OK)] + public async Task RemoveUserFromPlaylistShares( + [FromRoute, Required] Guid playlistId, + [FromBody] Guid userId) + { + var callingUserId = RequestHelpers.GetUserId(User, default); + + var playlist = _playlistManager.GetPlaylist(callingUserId, playlistId); + var isPermitted = playlist.OwnerUserId.Equals(callingUserId) + || playlist.Shares.Any(s => s.CanEdit && (s.UserId?.Equals(callingUserId) ?? false)); + + if (!isPermitted) + { + return Unauthorized("Unauthorized access"); + } + + var share = playlist.Shares.FirstOrDefault(s => s.UserId?.Equals(userId) ?? false); + + if (share is null) + { + return NotFound(); + } + + await _playlistManager.RemoveFromShares(playlistId, callingUserId, share).ConfigureAwait(false); + + return NoContent(); + } + /// /// Adds items to a playlist. /// diff --git a/Jellyfin.Server/Migrations/Routines/FixPlaylistOwner.cs b/Jellyfin.Server/Migrations/Routines/FixPlaylistOwner.cs index cf3182003..06596c171 100644 --- a/Jellyfin.Server/Migrations/Routines/FixPlaylistOwner.cs +++ b/Jellyfin.Server/Migrations/Routines/FixPlaylistOwner.cs @@ -54,7 +54,7 @@ internal class FixPlaylistOwner : IMigrationRoutine foreach (var playlist in playlists) { var shares = playlist.Shares; - if (shares.Length > 0) + if (shares.Count > 0) { var firstEditShare = shares.First(x => x.CanEdit); if (firstEditShare is not null && Guid.TryParse(firstEditShare.UserId, out var guid)) diff --git a/MediaBrowser.Controller/Playlists/IPlaylistManager.cs b/MediaBrowser.Controller/Playlists/IPlaylistManager.cs index bb68a3b6d..aaca1cc49 100644 --- a/MediaBrowser.Controller/Playlists/IPlaylistManager.cs +++ b/MediaBrowser.Controller/Playlists/IPlaylistManager.cs @@ -4,12 +4,21 @@ using System; using System.Collections.Generic; using System.Threading.Tasks; using MediaBrowser.Controller.Entities; +using MediaBrowser.Model.Entities; using MediaBrowser.Model.Playlists; namespace MediaBrowser.Controller.Playlists { public interface IPlaylistManager { + /// + /// Gets the playlist. + /// + /// The user identifier. + /// The playlist identifier. + /// Playlist. + Playlist GetPlaylist(Guid userId, Guid playlistId); + /// /// Gets the playlists. /// @@ -17,6 +26,32 @@ namespace MediaBrowser.Controller.Playlists /// IEnumerable<Playlist>. IEnumerable GetPlaylists(Guid userId); + /// + /// Toggle OpenAccess policy of the playlist. + /// + /// The playlist identifier. + /// The user identifier. + /// Task. + Task ToggleOpenAccess(Guid playlistId, Guid userId); + + /// + /// Adds a share to the playlist. + /// + /// The playlist identifier. + /// The user identifier. + /// The share. + /// Task. + Task AddToShares(Guid playlistId, Guid userId, Share share); + + /// + /// Rremoves a share from the playlist. + /// + /// The playlist identifier. + /// The user identifier. + /// The share. + /// Task. + Task RemoveFromShares(Guid playlistId, Guid userId, Share share); + /// /// Creates the playlist. /// diff --git a/MediaBrowser.Controller/Playlists/Playlist.cs b/MediaBrowser.Controller/Playlists/Playlist.cs index ca032e7f6..9a08a4ce3 100644 --- a/MediaBrowser.Controller/Playlists/Playlist.cs +++ b/MediaBrowser.Controller/Playlists/Playlist.cs @@ -16,20 +16,19 @@ using MediaBrowser.Controller.Entities; using MediaBrowser.Controller.Entities.Audio; using MediaBrowser.Controller.Providers; using MediaBrowser.Model.Entities; -using MediaBrowser.Model.Querying; namespace MediaBrowser.Controller.Playlists { public class Playlist : Folder, IHasShares { - public static readonly IReadOnlyList SupportedExtensions = new[] - { + public static readonly IReadOnlyList SupportedExtensions = + [ ".m3u", ".m3u8", ".pls", ".wpl", ".zpl" - }; + ]; public Playlist() { @@ -41,7 +40,7 @@ namespace MediaBrowser.Controller.Playlists public bool OpenAccess { get; set; } - public Share[] Shares { get; set; } + public IReadOnlyList Shares { get; set; } [JsonIgnore] public bool IsFile => IsPlaylistFile(Path); @@ -192,9 +191,9 @@ namespace MediaBrowser.Controller.Playlists return LibraryManager.GetItemList(new InternalItemsQuery(user) { Recursive = true, - IncludeItemTypes = new[] { BaseItemKind.Audio }, - GenreIds = new[] { musicGenre.Id }, - OrderBy = new[] { (ItemSortBy.AlbumArtist, SortOrder.Ascending), (ItemSortBy.Album, SortOrder.Ascending), (ItemSortBy.SortName, SortOrder.Ascending) }, + IncludeItemTypes = [BaseItemKind.Audio], + GenreIds = [musicGenre.Id], + OrderBy = [(ItemSortBy.AlbumArtist, SortOrder.Ascending), (ItemSortBy.Album, SortOrder.Ascending), (ItemSortBy.SortName, SortOrder.Ascending)], DtoOptions = options }); } @@ -204,9 +203,9 @@ namespace MediaBrowser.Controller.Playlists return LibraryManager.GetItemList(new InternalItemsQuery(user) { Recursive = true, - IncludeItemTypes = new[] { BaseItemKind.Audio }, - ArtistIds = new[] { musicArtist.Id }, - OrderBy = new[] { (ItemSortBy.AlbumArtist, SortOrder.Ascending), (ItemSortBy.Album, SortOrder.Ascending), (ItemSortBy.SortName, SortOrder.Ascending) }, + IncludeItemTypes = [BaseItemKind.Audio], + ArtistIds = [musicArtist.Id], + OrderBy = [(ItemSortBy.AlbumArtist, SortOrder.Ascending), (ItemSortBy.Album, SortOrder.Ascending), (ItemSortBy.SortName, SortOrder.Ascending)], DtoOptions = options }); } @@ -217,8 +216,8 @@ namespace MediaBrowser.Controller.Playlists { Recursive = true, IsFolder = false, - OrderBy = new[] { (ItemSortBy.SortName, SortOrder.Ascending) }, - MediaTypes = new[] { mediaType }, + OrderBy = [(ItemSortBy.SortName, SortOrder.Ascending)], + MediaTypes = [mediaType], EnableTotalRecordCount = false, DtoOptions = options }; @@ -226,7 +225,7 @@ namespace MediaBrowser.Controller.Playlists return folder.GetItemList(query); } - return new[] { item }; + return [item]; } public override bool IsVisible(User user) @@ -248,7 +247,7 @@ namespace MediaBrowser.Controller.Playlists } var shares = Shares; - if (shares.Length == 0) + if (shares.Count == 0) { return false; } diff --git a/MediaBrowser.Model/Entities/IHasShares.cs b/MediaBrowser.Model/Entities/IHasShares.cs index b34d1a037..31574a3ff 100644 --- a/MediaBrowser.Model/Entities/IHasShares.cs +++ b/MediaBrowser.Model/Entities/IHasShares.cs @@ -1,4 +1,6 @@ -namespace MediaBrowser.Model.Entities; +using System.Collections.Generic; + +namespace MediaBrowser.Model.Entities; /// /// Interface for access to shares. @@ -8,5 +10,5 @@ public interface IHasShares /// /// Gets or sets the shares. /// - Share[] Shares { get; set; } + IReadOnlyList Shares { get; set; } } From f1dc1610a28fdb2dec4241d233503b6e11020546 Mon Sep 17 00:00:00 2001 From: Shadowghost Date: Tue, 26 Mar 2024 16:13:07 +0100 Subject: [PATCH 02/17] Extend playlist creation capabilities --- .../Playlists/PlaylistManager.cs | 10 +++------- Jellyfin.Api/Controllers/PlaylistsController.cs | 4 +++- .../Models/PlaylistDtos/CreatePlaylistDto.cs | 13 ++++++++++++- .../Playlists/PlaylistCreationRequest.cs | 7 ++++++- 4 files changed, 24 insertions(+), 10 deletions(-) diff --git a/Emby.Server.Implementations/Playlists/PlaylistManager.cs b/Emby.Server.Implementations/Playlists/PlaylistManager.cs index 6724d54d1..6b169db79 100644 --- a/Emby.Server.Implementations/Playlists/PlaylistManager.cs +++ b/Emby.Server.Implementations/Playlists/PlaylistManager.cs @@ -85,12 +85,7 @@ namespace Emby.Server.Implementations.Playlists { foreach (var itemId in options.ItemIdList) { - var item = _libraryManager.GetItemById(itemId); - if (item is null) - { - throw new ArgumentException("No item exists with the supplied Id"); - } - + var item = _libraryManager.GetItemById(itemId) ?? throw new ArgumentException("No item exists with the supplied Id"); if (item.MediaType != MediaType.Unknown) { options.MediaType = item.MediaType; @@ -139,7 +134,8 @@ namespace Emby.Server.Implementations.Playlists Name = name, Path = path, OwnerUserId = options.UserId, - Shares = options.Shares ?? Array.Empty() + Shares = options.Shares ?? [], + OpenAccess = options.OpenAccess ?? false }; playlist.SetMediaType(options.MediaType); diff --git a/Jellyfin.Api/Controllers/PlaylistsController.cs b/Jellyfin.Api/Controllers/PlaylistsController.cs index f0e8227fd..bf618e8fd 100644 --- a/Jellyfin.Api/Controllers/PlaylistsController.cs +++ b/Jellyfin.Api/Controllers/PlaylistsController.cs @@ -92,7 +92,9 @@ public class PlaylistsController : BaseJellyfinApiController Name = name ?? createPlaylistRequest?.Name, ItemIdList = ids, UserId = userId.Value, - MediaType = mediaType ?? createPlaylistRequest?.MediaType + MediaType = mediaType ?? createPlaylistRequest?.MediaType, + Shares = createPlaylistRequest?.Shares.ToArray(), + OpenAccess = createPlaylistRequest?.OpenAccess }).ConfigureAwait(false); return result; diff --git a/Jellyfin.Api/Models/PlaylistDtos/CreatePlaylistDto.cs b/Jellyfin.Api/Models/PlaylistDtos/CreatePlaylistDto.cs index bdc488871..a82bff65e 100644 --- a/Jellyfin.Api/Models/PlaylistDtos/CreatePlaylistDto.cs +++ b/Jellyfin.Api/Models/PlaylistDtos/CreatePlaylistDto.cs @@ -3,6 +3,7 @@ using System.Collections.Generic; using System.Text.Json.Serialization; using Jellyfin.Data.Enums; using Jellyfin.Extensions.Json.Converters; +using MediaBrowser.Model.Entities; namespace Jellyfin.Api.Models.PlaylistDtos; @@ -20,7 +21,7 @@ public class CreatePlaylistDto /// Gets or sets item ids to add to the playlist. /// [JsonConverter(typeof(JsonCommaDelimitedArrayConverterFactory))] - public IReadOnlyList Ids { get; set; } = Array.Empty(); + public IReadOnlyList Ids { get; set; } = []; /// /// Gets or sets the user id. @@ -31,4 +32,14 @@ public class CreatePlaylistDto /// Gets or sets the media type. /// public MediaType? MediaType { get; set; } + + /// + /// Gets or sets the shares. + /// + public IReadOnlyList Shares { get; set; } = []; + + /// + /// Gets or sets a value indicating whether open access is enabled. + /// + public bool OpenAccess { get; set; } } diff --git a/MediaBrowser.Model/Playlists/PlaylistCreationRequest.cs b/MediaBrowser.Model/Playlists/PlaylistCreationRequest.cs index 62d496d04..93eccd5c7 100644 --- a/MediaBrowser.Model/Playlists/PlaylistCreationRequest.cs +++ b/MediaBrowser.Model/Playlists/PlaylistCreationRequest.cs @@ -33,5 +33,10 @@ public class PlaylistCreationRequest /// /// Gets or sets the shares. /// - public Share[]? Shares { get; set; } + public IReadOnlyList? Shares { get; set; } + + /// + /// Gets or sets a value indicating whether open access is enabled. + /// + public bool? OpenAccess { get; set; } } From 56c432a8439e1b75c15729bfdb19d41d34e3124d Mon Sep 17 00:00:00 2001 From: Shadowghost Date: Tue, 26 Mar 2024 23:45:14 +0100 Subject: [PATCH 03/17] Apply review suggestions --- .../Library/Resolvers/PlaylistResolver.cs | 9 ++-- .../Playlists/PlaylistManager.cs | 14 ++--- .../Controllers/PlaylistsController.cs | 51 +++++++++---------- .../Models/PlaylistDtos/CreatePlaylistDto.cs | 10 ++-- .../Playlists/IPlaylistManager.cs | 4 +- MediaBrowser.Controller/Playlists/Playlist.cs | 10 ++-- .../Parsers/BaseItemXmlParser.cs | 18 +++---- MediaBrowser.Model/Entities/IHasShares.cs | 4 +- MediaBrowser.Model/Entities/Share.cs | 17 ------- .../Entities/UserPermissions.cs | 19 +++++++ .../Playlists/PlaylistCreationRequest.cs | 10 ++-- 11 files changed, 82 insertions(+), 84 deletions(-) delete mode 100644 MediaBrowser.Model/Entities/Share.cs create mode 100644 MediaBrowser.Model/Entities/UserPermissions.cs diff --git a/Emby.Server.Implementations/Library/Resolvers/PlaylistResolver.cs b/Emby.Server.Implementations/Library/Resolvers/PlaylistResolver.cs index a50435ae6..a03c1214d 100644 --- a/Emby.Server.Implementations/Library/Resolvers/PlaylistResolver.cs +++ b/Emby.Server.Implementations/Library/Resolvers/PlaylistResolver.cs @@ -1,7 +1,5 @@ #nullable disable -#pragma warning disable CS1591 - using System; using System.IO; using System.Linq; @@ -11,7 +9,6 @@ using MediaBrowser.Controller.Library; using MediaBrowser.Controller.Playlists; using MediaBrowser.Controller.Resolvers; using MediaBrowser.LocalMetadata.Savers; -using MediaBrowser.Model.Entities; namespace Emby.Server.Implementations.Library.Resolvers { @@ -20,11 +17,11 @@ namespace Emby.Server.Implementations.Library.Resolvers /// public class PlaylistResolver : GenericFolderResolver { - private CollectionType?[] _musicPlaylistCollectionTypes = - { + private readonly CollectionType?[] _musicPlaylistCollectionTypes = + [ null, CollectionType.music - }; + ]; /// protected override Playlist Resolve(ItemResolveArgs args) diff --git a/Emby.Server.Implementations/Playlists/PlaylistManager.cs b/Emby.Server.Implementations/Playlists/PlaylistManager.cs index 6b169db79..59c96852a 100644 --- a/Emby.Server.Implementations/Playlists/PlaylistManager.cs +++ b/Emby.Server.Implementations/Playlists/PlaylistManager.cs @@ -134,8 +134,8 @@ namespace Emby.Server.Implementations.Playlists Name = name, Path = path, OwnerUserId = options.UserId, - Shares = options.Shares ?? [], - OpenAccess = options.OpenAccess ?? false + Shares = options.Users ?? [], + OpenAccess = options.Public ?? false }; playlist.SetMediaType(options.MediaType); @@ -171,9 +171,9 @@ namespace Emby.Server.Implementations.Playlists return path; } - private List GetPlaylistItems(IEnumerable itemIds, MediaType playlistMediaType, User user, DtoOptions options) + private IReadOnlyList GetPlaylistItems(IEnumerable itemIds, MediaType playlistMediaType, User user, DtoOptions options) { - var items = itemIds.Select(i => _libraryManager.GetItemById(i)).Where(i => i is not null); + var items = itemIds.Select(_libraryManager.GetItemById).Where(i => i is not null); return Playlist.GetPlaylistItems(playlistMediaType, items, user, options); } @@ -556,11 +556,11 @@ namespace Emby.Server.Implementations.Playlists await UpdatePlaylist(playlist).ConfigureAwait(false); } - public async Task AddToShares(Guid playlistId, Guid userId, Share share) + public async Task AddToShares(Guid playlistId, Guid userId, UserPermissions share) { var playlist = GetPlaylist(userId, playlistId); var shares = playlist.Shares.ToList(); - var existingUserShare = shares.FirstOrDefault(s => s.UserId?.Equals(share.UserId, StringComparison.OrdinalIgnoreCase) ?? false); + var existingUserShare = shares.FirstOrDefault(s => s.UserId.Equals(share.UserId, StringComparison.OrdinalIgnoreCase)); if (existingUserShare is not null) { shares.Remove(existingUserShare); @@ -571,7 +571,7 @@ namespace Emby.Server.Implementations.Playlists await UpdatePlaylist(playlist).ConfigureAwait(false); } - public async Task RemoveFromShares(Guid playlistId, Guid userId, Share share) + public async Task RemoveFromShares(Guid playlistId, Guid userId, UserPermissions share) { var playlist = GetPlaylist(userId, playlistId); var shares = playlist.Shares.ToList(); diff --git a/Jellyfin.Api/Controllers/PlaylistsController.cs b/Jellyfin.Api/Controllers/PlaylistsController.cs index bf618e8fd..c38061c7d 100644 --- a/Jellyfin.Api/Controllers/PlaylistsController.cs +++ b/Jellyfin.Api/Controllers/PlaylistsController.cs @@ -93,32 +93,32 @@ public class PlaylistsController : BaseJellyfinApiController ItemIdList = ids, UserId = userId.Value, MediaType = mediaType ?? createPlaylistRequest?.MediaType, - Shares = createPlaylistRequest?.Shares.ToArray(), - OpenAccess = createPlaylistRequest?.OpenAccess + Users = createPlaylistRequest?.Users.ToArray() ?? [], + Public = createPlaylistRequest?.Public }).ConfigureAwait(false); return result; } /// - /// Get a playlist's shares. + /// Get a playlist's users. /// /// The playlist id. /// - /// A list of objects. + /// A list of objects. /// - [HttpGet("{playlistId}/Shares")] + [HttpGet("{playlistId}/User")] [ProducesResponseType(StatusCodes.Status200OK)] - public IReadOnlyList GetPlaylistShares( + public IReadOnlyList GetPlaylistUsers( [FromRoute, Required] Guid playlistId) { var userId = RequestHelpers.GetUserId(User, default); var playlist = _playlistManager.GetPlaylist(userId, playlistId); var isPermitted = playlist.OwnerUserId.Equals(userId) - || playlist.Shares.Any(s => s.CanEdit && (s.UserId?.Equals(userId) ?? false)); + || playlist.Shares.Any(s => s.CanEdit && s.UserId.Equals(userId)); - return isPermitted ? playlist.Shares : new List(); + return isPermitted ? playlist.Shares : []; } /// @@ -131,14 +131,14 @@ public class PlaylistsController : BaseJellyfinApiController /// [HttpPost("{playlistId}/ToggleOpenAccess")] [ProducesResponseType(StatusCodes.Status200OK)] - public async Task ToggleopenAccess( + public async Task ToggleOpenAccess( [FromRoute, Required] Guid playlistId) { var callingUserId = RequestHelpers.GetUserId(User, default); var playlist = _playlistManager.GetPlaylist(callingUserId, playlistId); var isPermitted = playlist.OwnerUserId.Equals(callingUserId) - || playlist.Shares.Any(s => s.CanEdit && (s.UserId?.Equals(callingUserId) ?? false)); + || playlist.Shares.Any(s => s.CanEdit && s.UserId.Equals(callingUserId)); if (!isPermitted) { @@ -151,35 +151,34 @@ public class PlaylistsController : BaseJellyfinApiController } /// - /// Adds shares to a playlist's shares. + /// Upsert a user to a playlist's users. /// /// The playlist id. - /// The shares. + /// The user id. + /// Edit permission. /// - /// A that represents the asynchronous operation to add shares to a playlist. + /// A that represents the asynchronous operation to upsert an user to a playlist. /// The task result contains an indicating success. /// - [HttpPost("{playlistId}/Shares")] + [HttpPost("{playlistId}/User/{userId}")] [ProducesResponseType(StatusCodes.Status200OK)] - public async Task AddUserToPlaylistShares( + public async Task AddUserToPlaylist( [FromRoute, Required] Guid playlistId, - [FromBody(EmptyBodyBehavior = EmptyBodyBehavior.Disallow)] Share[] shares) + [FromRoute, Required] Guid userId, + [FromBody] bool canEdit) { var callingUserId = RequestHelpers.GetUserId(User, default); var playlist = _playlistManager.GetPlaylist(callingUserId, playlistId); var isPermitted = playlist.OwnerUserId.Equals(callingUserId) - || playlist.Shares.Any(s => s.CanEdit && (s.UserId?.Equals(callingUserId) ?? false)); + || playlist.Shares.Any(s => s.CanEdit && s.UserId.Equals(callingUserId)); if (!isPermitted) { return Unauthorized("Unauthorized access"); } - foreach (var share in shares) - { - await _playlistManager.AddToShares(playlistId, callingUserId, share).ConfigureAwait(false); - } + await _playlistManager.AddToShares(playlistId, callingUserId, new UserPermissions(userId.ToString(), canEdit)).ConfigureAwait(false); return NoContent(); } @@ -193,24 +192,24 @@ public class PlaylistsController : BaseJellyfinApiController /// A that represents the asynchronous operation to delete a user from a playlist's shares. /// The task result contains an indicating success. /// - [HttpDelete("{playlistId}/Shares")] + [HttpDelete("{playlistId}/User/{userId}")] [ProducesResponseType(StatusCodes.Status200OK)] - public async Task RemoveUserFromPlaylistShares( + public async Task RemoveUserFromPlaylist( [FromRoute, Required] Guid playlistId, - [FromBody] Guid userId) + [FromRoute, Required] Guid userId) { var callingUserId = RequestHelpers.GetUserId(User, default); var playlist = _playlistManager.GetPlaylist(callingUserId, playlistId); var isPermitted = playlist.OwnerUserId.Equals(callingUserId) - || playlist.Shares.Any(s => s.CanEdit && (s.UserId?.Equals(callingUserId) ?? false)); + || playlist.Shares.Any(s => s.CanEdit && s.UserId.Equals(callingUserId)); if (!isPermitted) { return Unauthorized("Unauthorized access"); } - var share = playlist.Shares.FirstOrDefault(s => s.UserId?.Equals(userId) ?? false); + var share = playlist.Shares.FirstOrDefault(s => s.UserId.Equals(userId)); if (share is null) { diff --git a/Jellyfin.Api/Models/PlaylistDtos/CreatePlaylistDto.cs b/Jellyfin.Api/Models/PlaylistDtos/CreatePlaylistDto.cs index a82bff65e..6eedd2131 100644 --- a/Jellyfin.Api/Models/PlaylistDtos/CreatePlaylistDto.cs +++ b/Jellyfin.Api/Models/PlaylistDtos/CreatePlaylistDto.cs @@ -15,7 +15,7 @@ public class CreatePlaylistDto /// /// Gets or sets the name of the new playlist. /// - public string? Name { get; set; } + public required string Name { get; set; } /// /// Gets or sets item ids to add to the playlist. @@ -34,12 +34,12 @@ public class CreatePlaylistDto public MediaType? MediaType { get; set; } /// - /// Gets or sets the shares. + /// Gets or sets the playlist users. /// - public IReadOnlyList Shares { get; set; } = []; + public IReadOnlyList Users { get; set; } = []; /// - /// Gets or sets a value indicating whether open access is enabled. + /// Gets or sets a value indicating whether the playlist is public. /// - public bool OpenAccess { get; set; } + public bool Public { get; set; } = true; } diff --git a/MediaBrowser.Controller/Playlists/IPlaylistManager.cs b/MediaBrowser.Controller/Playlists/IPlaylistManager.cs index aaca1cc49..238923d29 100644 --- a/MediaBrowser.Controller/Playlists/IPlaylistManager.cs +++ b/MediaBrowser.Controller/Playlists/IPlaylistManager.cs @@ -41,7 +41,7 @@ namespace MediaBrowser.Controller.Playlists /// The user identifier. /// The share. /// Task. - Task AddToShares(Guid playlistId, Guid userId, Share share); + Task AddToShares(Guid playlistId, Guid userId, UserPermissions share); /// /// Rremoves a share from the playlist. @@ -50,7 +50,7 @@ namespace MediaBrowser.Controller.Playlists /// The user identifier. /// The share. /// Task. - Task RemoveFromShares(Guid playlistId, Guid userId, Share share); + Task RemoveFromShares(Guid playlistId, Guid userId, UserPermissions share); /// /// Creates the playlist. diff --git a/MediaBrowser.Controller/Playlists/Playlist.cs b/MediaBrowser.Controller/Playlists/Playlist.cs index 9a08a4ce3..dfd9b8330 100644 --- a/MediaBrowser.Controller/Playlists/Playlist.cs +++ b/MediaBrowser.Controller/Playlists/Playlist.cs @@ -32,7 +32,7 @@ namespace MediaBrowser.Controller.Playlists public Playlist() { - Shares = Array.Empty(); + Shares = []; OpenAccess = false; } @@ -40,7 +40,7 @@ namespace MediaBrowser.Controller.Playlists public bool OpenAccess { get; set; } - public IReadOnlyList Shares { get; set; } + public IReadOnlyList Shares { get; set; } [JsonIgnore] public bool IsFile => IsPlaylistFile(Path); @@ -129,7 +129,7 @@ namespace MediaBrowser.Controller.Playlists protected override List LoadChildren() { // Save a trip to the database - return new List(); + return []; } protected override Task ValidateChildrenInternal(IProgress progress, bool recursive, bool refreshChildMetadata, MetadataRefreshOptions refreshOptions, IDirectoryService directoryService, CancellationToken cancellationToken) @@ -144,7 +144,7 @@ namespace MediaBrowser.Controller.Playlists protected override IEnumerable GetNonCachedChildren(IDirectoryService directoryService) { - return new List(); + return []; } public override IEnumerable GetRecursiveChildren(User user, InternalItemsQuery query) @@ -166,7 +166,7 @@ namespace MediaBrowser.Controller.Playlists return base.GetChildren(user, true, query); } - public static List GetPlaylistItems(MediaType playlistMediaType, IEnumerable inputItems, User user, DtoOptions options) + public static IReadOnlyList GetPlaylistItems(MediaType playlistMediaType, IEnumerable inputItems, User user, DtoOptions options) { if (user is not null) { diff --git a/MediaBrowser.LocalMetadata/Parsers/BaseItemXmlParser.cs b/MediaBrowser.LocalMetadata/Parsers/BaseItemXmlParser.cs index 8a870e0d9..4ee1b2ef6 100644 --- a/MediaBrowser.LocalMetadata/Parsers/BaseItemXmlParser.cs +++ b/MediaBrowser.LocalMetadata/Parsers/BaseItemXmlParser.cs @@ -519,7 +519,7 @@ namespace MediaBrowser.LocalMetadata.Parsers private void FetchFromSharesNode(XmlReader reader, IHasShares item) { - var list = new List(); + var list = new List(); reader.MoveToContent(); reader.Read(); @@ -565,7 +565,7 @@ namespace MediaBrowser.LocalMetadata.Parsers } } - item.Shares = list.ToArray(); + item.Shares = [.. list]; } /// @@ -830,12 +830,12 @@ namespace MediaBrowser.LocalMetadata.Parsers /// /// The xml reader. /// The share. - protected Share? GetShare(XmlReader reader) + protected UserPermissions? GetShare(XmlReader reader) { - var item = new Share(); - reader.MoveToContent(); reader.Read(); + string? userId = null; + var canEdit = false; // Loop through each element while (!reader.EOF && reader.ReadState == ReadState.Interactive) @@ -845,10 +845,10 @@ namespace MediaBrowser.LocalMetadata.Parsers switch (reader.Name) { case "UserId": - item.UserId = reader.ReadNormalizedString(); + userId = reader.ReadNormalizedString(); break; case "CanEdit": - item.CanEdit = string.Equals(reader.ReadElementContentAsString(), "true", StringComparison.OrdinalIgnoreCase); + canEdit = string.Equals(reader.ReadElementContentAsString(), "true", StringComparison.OrdinalIgnoreCase); break; default: reader.Skip(); @@ -862,9 +862,9 @@ namespace MediaBrowser.LocalMetadata.Parsers } // This is valid - if (!string.IsNullOrWhiteSpace(item.UserId)) + if (!string.IsNullOrWhiteSpace(userId)) { - return item; + return new UserPermissions(userId, canEdit); } return null; diff --git a/MediaBrowser.Model/Entities/IHasShares.cs b/MediaBrowser.Model/Entities/IHasShares.cs index 31574a3ff..fb6b6e424 100644 --- a/MediaBrowser.Model/Entities/IHasShares.cs +++ b/MediaBrowser.Model/Entities/IHasShares.cs @@ -1,4 +1,4 @@ -using System.Collections.Generic; +using System.Collections.Generic; namespace MediaBrowser.Model.Entities; @@ -10,5 +10,5 @@ public interface IHasShares /// /// Gets or sets the shares. /// - IReadOnlyList Shares { get; set; } + IReadOnlyList Shares { get; set; } } diff --git a/MediaBrowser.Model/Entities/Share.cs b/MediaBrowser.Model/Entities/Share.cs deleted file mode 100644 index 186aad189..000000000 --- a/MediaBrowser.Model/Entities/Share.cs +++ /dev/null @@ -1,17 +0,0 @@ -namespace MediaBrowser.Model.Entities; - -/// -/// Class to hold data on sharing permissions. -/// -public class Share -{ - /// - /// Gets or sets the user id. - /// - public string? UserId { get; set; } - - /// - /// Gets or sets a value indicating whether the user has edit permissions. - /// - public bool CanEdit { get; set; } -} diff --git a/MediaBrowser.Model/Entities/UserPermissions.cs b/MediaBrowser.Model/Entities/UserPermissions.cs new file mode 100644 index 000000000..271feed11 --- /dev/null +++ b/MediaBrowser.Model/Entities/UserPermissions.cs @@ -0,0 +1,19 @@ +namespace MediaBrowser.Model.Entities; + +/// +/// Class to hold data on user permissions for lists. +/// +/// The user id. +/// Edit permission. +public class UserPermissions(string userId, bool canEdit = false) +{ + /// + /// Gets or sets the user id. + /// + public string UserId { get; set; } = userId; + + /// + /// Gets or sets a value indicating whether the user has edit permissions. + /// + public bool CanEdit { get; set; } = canEdit; +} diff --git a/MediaBrowser.Model/Playlists/PlaylistCreationRequest.cs b/MediaBrowser.Model/Playlists/PlaylistCreationRequest.cs index 93eccd5c7..f1351588f 100644 --- a/MediaBrowser.Model/Playlists/PlaylistCreationRequest.cs +++ b/MediaBrowser.Model/Playlists/PlaylistCreationRequest.cs @@ -18,7 +18,7 @@ public class PlaylistCreationRequest /// /// Gets or sets the list of items. /// - public IReadOnlyList ItemIdList { get; set; } = Array.Empty(); + public IReadOnlyList ItemIdList { get; set; } = []; /// /// Gets or sets the media type. @@ -31,12 +31,12 @@ public class PlaylistCreationRequest public Guid UserId { get; set; } /// - /// Gets or sets the shares. + /// Gets or sets the user permissions. /// - public IReadOnlyList? Shares { get; set; } + public IReadOnlyList Users { get; set; } = []; /// - /// Gets or sets a value indicating whether open access is enabled. + /// Gets or sets a value indicating whether the playlist is public. /// - public bool? OpenAccess { get; set; } + public bool? Public { get; set; } = true; } From 2aaa9f669a0ba6855329ce63ae4e785dc70a5a69 Mon Sep 17 00:00:00 2001 From: Shadowghost Date: Wed, 27 Mar 2024 06:39:14 +0100 Subject: [PATCH 04/17] Apply review suggestions --- .../Controllers/PlaylistsController.cs | 73 ++++++++++++++----- .../Entities/UserPermissions.cs | 19 +++-- 2 files changed, 69 insertions(+), 23 deletions(-) diff --git a/Jellyfin.Api/Controllers/PlaylistsController.cs b/Jellyfin.Api/Controllers/PlaylistsController.cs index c38061c7d..7ca04d7ba 100644 --- a/Jellyfin.Api/Controllers/PlaylistsController.cs +++ b/Jellyfin.Api/Controllers/PlaylistsController.cs @@ -104,39 +104,58 @@ public class PlaylistsController : BaseJellyfinApiController /// Get a playlist's users. /// /// The playlist id. + /// Found shares. + /// Unauthorized access. + /// Playlist not found. /// /// A list of objects. /// [HttpGet("{playlistId}/User")] [ProducesResponseType(StatusCodes.Status200OK)] - public IReadOnlyList GetPlaylistUsers( + [ProducesResponseType(StatusCodes.Status401Unauthorized)] + [ProducesResponseType(StatusCodes.Status404NotFound)] + public ActionResult> GetPlaylistUsers( [FromRoute, Required] Guid playlistId) { - var userId = RequestHelpers.GetUserId(User, default); + var userId = User.GetUserId(); var playlist = _playlistManager.GetPlaylist(userId, playlistId); + if (playlist is null) + { + return NotFound("Playlist not found"); + } + var isPermitted = playlist.OwnerUserId.Equals(userId) || playlist.Shares.Any(s => s.CanEdit && s.UserId.Equals(userId)); - return isPermitted ? playlist.Shares : []; + return isPermitted ? playlist.Shares.ToList() : Unauthorized("Unauthorized Access"); } /// - /// Toggles OpenAccess of a playlist. + /// Toggles public access of a playlist. /// /// The playlist id. + /// Public access toggled. + /// Unauthorized access. + /// Playlist not found. /// - /// A that represents the asynchronous operation to toggle OpenAccess of a playlist. + /// A that represents the asynchronous operation to toggle public access of a playlist. /// The task result contains an indicating success. /// - [HttpPost("{playlistId}/ToggleOpenAccess")] - [ProducesResponseType(StatusCodes.Status200OK)] - public async Task ToggleOpenAccess( + [HttpPost("{playlistId}/TogglePublic")] + [ProducesResponseType(StatusCodes.Status204NoContent)] + [ProducesResponseType(StatusCodes.Status401Unauthorized)] + public async Task TogglePublicAccess( [FromRoute, Required] Guid playlistId) { - var callingUserId = RequestHelpers.GetUserId(User, default); + var callingUserId = User.GetUserId(); var playlist = _playlistManager.GetPlaylist(callingUserId, playlistId); + if (playlist is null) + { + return NotFound("Playlist not found"); + } + var isPermitted = playlist.OwnerUserId.Equals(callingUserId) || playlist.Shares.Any(s => s.CanEdit && s.UserId.Equals(callingUserId)); @@ -151,25 +170,34 @@ public class PlaylistsController : BaseJellyfinApiController } /// - /// Upsert a user to a playlist's users. + /// Modify a user to a playlist's users. /// /// The playlist id. /// The user id. /// Edit permission. + /// User's permissions modified. + /// Unauthorized access. + /// Playlist not found. /// - /// A that represents the asynchronous operation to upsert an user to a playlist. + /// A that represents the asynchronous operation to modify an user's playlist permissions. /// The task result contains an indicating success. /// [HttpPost("{playlistId}/User/{userId}")] - [ProducesResponseType(StatusCodes.Status200OK)] - public async Task AddUserToPlaylist( + [ProducesResponseType(StatusCodes.Status204NoContent)] + [ProducesResponseType(StatusCodes.Status401Unauthorized)] + public async Task ModifyPlaylistUserPermissions( [FromRoute, Required] Guid playlistId, [FromRoute, Required] Guid userId, [FromBody] bool canEdit) { - var callingUserId = RequestHelpers.GetUserId(User, default); + var callingUserId = User.GetUserId(); var playlist = _playlistManager.GetPlaylist(callingUserId, playlistId); + if (playlist is null) + { + return NotFound("Playlist not found"); + } + var isPermitted = playlist.OwnerUserId.Equals(callingUserId) || playlist.Shares.Any(s => s.CanEdit && s.UserId.Equals(callingUserId)); @@ -188,19 +216,29 @@ public class PlaylistsController : BaseJellyfinApiController /// /// The playlist id. /// The user id. + /// User permissions removed from playlist. + /// Unauthorized access. + /// No playlist or user permissions found. /// /// A that represents the asynchronous operation to delete a user from a playlist's shares. /// The task result contains an indicating success. /// [HttpDelete("{playlistId}/User/{userId}")] - [ProducesResponseType(StatusCodes.Status200OK)] + [ProducesResponseType(StatusCodes.Status204NoContent)] + [ProducesResponseType(StatusCodes.Status401Unauthorized)] + [ProducesResponseType(StatusCodes.Status404NotFound)] public async Task RemoveUserFromPlaylist( [FromRoute, Required] Guid playlistId, [FromRoute, Required] Guid userId) { - var callingUserId = RequestHelpers.GetUserId(User, default); + var callingUserId = User.GetUserId(); var playlist = _playlistManager.GetPlaylist(callingUserId, playlistId); + if (playlist is null) + { + return NotFound("Playlist not found"); + } + var isPermitted = playlist.OwnerUserId.Equals(callingUserId) || playlist.Shares.Any(s => s.CanEdit && s.UserId.Equals(callingUserId)); @@ -210,10 +248,9 @@ public class PlaylistsController : BaseJellyfinApiController } var share = playlist.Shares.FirstOrDefault(s => s.UserId.Equals(userId)); - if (share is null) { - return NotFound(); + return NotFound("User permissions not found"); } await _playlistManager.RemoveFromShares(playlistId, callingUserId, share).ConfigureAwait(false); diff --git a/MediaBrowser.Model/Entities/UserPermissions.cs b/MediaBrowser.Model/Entities/UserPermissions.cs index 271feed11..80e2cd32c 100644 --- a/MediaBrowser.Model/Entities/UserPermissions.cs +++ b/MediaBrowser.Model/Entities/UserPermissions.cs @@ -3,17 +3,26 @@ namespace MediaBrowser.Model.Entities; /// /// Class to hold data on user permissions for lists. /// -/// The user id. -/// Edit permission. -public class UserPermissions(string userId, bool canEdit = false) +public class UserPermissions { + /// + /// Initializes a new instance of the class. + /// + /// The user id. + /// Edit permission. + public UserPermissions(string userId, bool canEdit = false) + { + UserId = userId; + CanEdit = canEdit; + } + /// /// Gets or sets the user id. /// - public string UserId { get; set; } = userId; + public string UserId { get; set; } /// /// Gets or sets a value indicating whether the user has edit permissions. /// - public bool CanEdit { get; set; } = canEdit; + public bool CanEdit { get; set; } } From bff37ed13aa9ee0267ee5e1248339c6044fa1b0c Mon Sep 17 00:00:00 2001 From: Shadowghost Date: Mon, 1 Apr 2024 19:59:48 +0200 Subject: [PATCH 05/17] Apply review suggestions --- .../Playlists/PlaylistManager.cs | 10 +++++----- Jellyfin.Api/Controllers/PlaylistsController.cs | 6 +++--- .../Models/PlaylistDtos/CreatePlaylistDto.cs | 2 +- .../Playlists/IPlaylistManager.cs | 4 ++-- MediaBrowser.Controller/Playlists/Playlist.cs | 2 +- .../Parsers/BaseItemXmlParser.cs | 6 +++--- MediaBrowser.Model/Entities/IHasShares.cs | 2 +- ...UserPermissions.cs => PlaylistUserPermissions.cs} | 12 +++++++----- .../Playlists/PlaylistCreationRequest.cs | 2 +- 9 files changed, 24 insertions(+), 22 deletions(-) rename MediaBrowser.Model/Entities/{UserPermissions.cs => PlaylistUserPermissions.cs} (62%) diff --git a/Emby.Server.Implementations/Playlists/PlaylistManager.cs b/Emby.Server.Implementations/Playlists/PlaylistManager.cs index 59c96852a..0e5add635 100644 --- a/Emby.Server.Implementations/Playlists/PlaylistManager.cs +++ b/Emby.Server.Implementations/Playlists/PlaylistManager.cs @@ -526,9 +526,9 @@ namespace Emby.Server.Implementations.Playlists { // Update owner if shared var rankedShares = playlist.Shares.OrderByDescending(x => x.CanEdit).ToArray(); - if (rankedShares.Length > 0 && Guid.TryParse(rankedShares[0].UserId, out var guid)) + if (rankedShares.Length > 0) { - playlist.OwnerUserId = guid; + playlist.OwnerUserId = rankedShares[0].UserId; playlist.Shares = rankedShares.Skip(1).ToArray(); await UpdatePlaylist(playlist).ConfigureAwait(false); } @@ -556,11 +556,11 @@ namespace Emby.Server.Implementations.Playlists await UpdatePlaylist(playlist).ConfigureAwait(false); } - public async Task AddToShares(Guid playlistId, Guid userId, UserPermissions share) + public async Task AddToShares(Guid playlistId, Guid userId, PlaylistUserPermissions share) { var playlist = GetPlaylist(userId, playlistId); var shares = playlist.Shares.ToList(); - var existingUserShare = shares.FirstOrDefault(s => s.UserId.Equals(share.UserId, StringComparison.OrdinalIgnoreCase)); + var existingUserShare = shares.FirstOrDefault(s => s.UserId.Equals(share.UserId)); if (existingUserShare is not null) { shares.Remove(existingUserShare); @@ -571,7 +571,7 @@ namespace Emby.Server.Implementations.Playlists await UpdatePlaylist(playlist).ConfigureAwait(false); } - public async Task RemoveFromShares(Guid playlistId, Guid userId, UserPermissions share) + public async Task RemoveFromShares(Guid playlistId, Guid userId, PlaylistUserPermissions share) { var playlist = GetPlaylist(userId, playlistId); var shares = playlist.Shares.ToList(); diff --git a/Jellyfin.Api/Controllers/PlaylistsController.cs b/Jellyfin.Api/Controllers/PlaylistsController.cs index 7ca04d7ba..862e5235e 100644 --- a/Jellyfin.Api/Controllers/PlaylistsController.cs +++ b/Jellyfin.Api/Controllers/PlaylistsController.cs @@ -108,13 +108,13 @@ public class PlaylistsController : BaseJellyfinApiController /// Unauthorized access. /// Playlist not found. /// - /// A list of objects. + /// A list of objects. /// [HttpGet("{playlistId}/User")] [ProducesResponseType(StatusCodes.Status200OK)] [ProducesResponseType(StatusCodes.Status401Unauthorized)] [ProducesResponseType(StatusCodes.Status404NotFound)] - public ActionResult> GetPlaylistUsers( + public ActionResult> GetPlaylistUsers( [FromRoute, Required] Guid playlistId) { var userId = User.GetUserId(); @@ -206,7 +206,7 @@ public class PlaylistsController : BaseJellyfinApiController return Unauthorized("Unauthorized access"); } - await _playlistManager.AddToShares(playlistId, callingUserId, new UserPermissions(userId.ToString(), canEdit)).ConfigureAwait(false); + await _playlistManager.AddToShares(playlistId, callingUserId, new PlaylistUserPermissions(userId.ToString(), canEdit)).ConfigureAwait(false); return NoContent(); } diff --git a/Jellyfin.Api/Models/PlaylistDtos/CreatePlaylistDto.cs b/Jellyfin.Api/Models/PlaylistDtos/CreatePlaylistDto.cs index 6eedd2131..69694a769 100644 --- a/Jellyfin.Api/Models/PlaylistDtos/CreatePlaylistDto.cs +++ b/Jellyfin.Api/Models/PlaylistDtos/CreatePlaylistDto.cs @@ -36,7 +36,7 @@ public class CreatePlaylistDto /// /// Gets or sets the playlist users. /// - public IReadOnlyList Users { get; set; } = []; + public IReadOnlyList Users { get; set; } = []; /// /// Gets or sets a value indicating whether the playlist is public. diff --git a/MediaBrowser.Controller/Playlists/IPlaylistManager.cs b/MediaBrowser.Controller/Playlists/IPlaylistManager.cs index 238923d29..1750be619 100644 --- a/MediaBrowser.Controller/Playlists/IPlaylistManager.cs +++ b/MediaBrowser.Controller/Playlists/IPlaylistManager.cs @@ -41,7 +41,7 @@ namespace MediaBrowser.Controller.Playlists /// The user identifier. /// The share. /// Task. - Task AddToShares(Guid playlistId, Guid userId, UserPermissions share); + Task AddToShares(Guid playlistId, Guid userId, PlaylistUserPermissions share); /// /// Rremoves a share from the playlist. @@ -50,7 +50,7 @@ namespace MediaBrowser.Controller.Playlists /// The user identifier. /// The share. /// Task. - Task RemoveFromShares(Guid playlistId, Guid userId, UserPermissions share); + Task RemoveFromShares(Guid playlistId, Guid userId, PlaylistUserPermissions share); /// /// Creates the playlist. diff --git a/MediaBrowser.Controller/Playlists/Playlist.cs b/MediaBrowser.Controller/Playlists/Playlist.cs index dfd9b8330..b948d2e18 100644 --- a/MediaBrowser.Controller/Playlists/Playlist.cs +++ b/MediaBrowser.Controller/Playlists/Playlist.cs @@ -40,7 +40,7 @@ namespace MediaBrowser.Controller.Playlists public bool OpenAccess { get; set; } - public IReadOnlyList Shares { get; set; } + public IReadOnlyList Shares { get; set; } [JsonIgnore] public bool IsFile => IsPlaylistFile(Path); diff --git a/MediaBrowser.LocalMetadata/Parsers/BaseItemXmlParser.cs b/MediaBrowser.LocalMetadata/Parsers/BaseItemXmlParser.cs index 4ee1b2ef6..22ae3f12b 100644 --- a/MediaBrowser.LocalMetadata/Parsers/BaseItemXmlParser.cs +++ b/MediaBrowser.LocalMetadata/Parsers/BaseItemXmlParser.cs @@ -519,7 +519,7 @@ namespace MediaBrowser.LocalMetadata.Parsers private void FetchFromSharesNode(XmlReader reader, IHasShares item) { - var list = new List(); + var list = new List(); reader.MoveToContent(); reader.Read(); @@ -830,7 +830,7 @@ namespace MediaBrowser.LocalMetadata.Parsers /// /// The xml reader. /// The share. - protected UserPermissions? GetShare(XmlReader reader) + protected PlaylistUserPermissions? GetShare(XmlReader reader) { reader.MoveToContent(); reader.Read(); @@ -864,7 +864,7 @@ namespace MediaBrowser.LocalMetadata.Parsers // This is valid if (!string.IsNullOrWhiteSpace(userId)) { - return new UserPermissions(userId, canEdit); + return new PlaylistUserPermissions(userId, canEdit); } return null; diff --git a/MediaBrowser.Model/Entities/IHasShares.cs b/MediaBrowser.Model/Entities/IHasShares.cs index fb6b6e424..8c4ba6c42 100644 --- a/MediaBrowser.Model/Entities/IHasShares.cs +++ b/MediaBrowser.Model/Entities/IHasShares.cs @@ -10,5 +10,5 @@ public interface IHasShares /// /// Gets or sets the shares. /// - IReadOnlyList Shares { get; set; } + IReadOnlyList Shares { get; set; } } diff --git a/MediaBrowser.Model/Entities/UserPermissions.cs b/MediaBrowser.Model/Entities/PlaylistUserPermissions.cs similarity index 62% rename from MediaBrowser.Model/Entities/UserPermissions.cs rename to MediaBrowser.Model/Entities/PlaylistUserPermissions.cs index 80e2cd32c..b5f017d2b 100644 --- a/MediaBrowser.Model/Entities/UserPermissions.cs +++ b/MediaBrowser.Model/Entities/PlaylistUserPermissions.cs @@ -1,16 +1,18 @@ +using System; + namespace MediaBrowser.Model.Entities; /// -/// Class to hold data on user permissions for lists. +/// Class to hold data on user permissions for playlists. /// -public class UserPermissions +public class PlaylistUserPermissions { /// - /// Initializes a new instance of the class. + /// Initializes a new instance of the class. /// /// The user id. /// Edit permission. - public UserPermissions(string userId, bool canEdit = false) + public PlaylistUserPermissions(Guid userId, bool canEdit = false) { UserId = userId; CanEdit = canEdit; @@ -19,7 +21,7 @@ public class UserPermissions /// /// Gets or sets the user id. /// - public string UserId { get; set; } + public Guid UserId { get; set; } /// /// Gets or sets a value indicating whether the user has edit permissions. diff --git a/MediaBrowser.Model/Playlists/PlaylistCreationRequest.cs b/MediaBrowser.Model/Playlists/PlaylistCreationRequest.cs index f1351588f..ec54b1afd 100644 --- a/MediaBrowser.Model/Playlists/PlaylistCreationRequest.cs +++ b/MediaBrowser.Model/Playlists/PlaylistCreationRequest.cs @@ -33,7 +33,7 @@ public class PlaylistCreationRequest /// /// Gets or sets the user permissions. /// - public IReadOnlyList Users { get; set; } = []; + public IReadOnlyList Users { get; set; } = []; /// /// Gets or sets a value indicating whether the playlist is public. From c1dbb49315f90bf03445a960eb8eace86f1ea6f2 Mon Sep 17 00:00:00 2001 From: Shadowghost Date: Mon, 1 Apr 2024 20:43:05 +0200 Subject: [PATCH 06/17] Implement update endpoint --- .../Playlists/PlaylistManager.cs | 88 ++++++++++++------- .../Controllers/PlaylistsController.cs | 88 +++++++++++-------- .../Models/PlaylistDtos/UpdatePlaylistDto.cs | 34 +++++++ .../Migrations/Routines/FixPlaylistOwner.cs | 4 +- .../Playlists/IPlaylistManager.cs | 29 +++--- MediaBrowser.Controller/Playlists/Playlist.cs | 2 +- .../Parsers/BaseItemXmlParser.cs | 4 +- .../Savers/BaseXmlSaver.cs | 19 ++-- .../Playlists/PlaylistUpdateRequest.cs | 41 +++++++++ 9 files changed, 209 insertions(+), 100 deletions(-) create mode 100644 Jellyfin.Api/Models/PlaylistDtos/UpdatePlaylistDto.cs create mode 100644 MediaBrowser.Model/Playlists/PlaylistUpdateRequest.cs diff --git a/Emby.Server.Implementations/Playlists/PlaylistManager.cs b/Emby.Server.Implementations/Playlists/PlaylistManager.cs index 0e5add635..517ec68dc 100644 --- a/Emby.Server.Implementations/Playlists/PlaylistManager.cs +++ b/Emby.Server.Implementations/Playlists/PlaylistManager.cs @@ -71,56 +71,56 @@ namespace Emby.Server.Implementations.Playlists return GetPlaylistsFolder(userId).GetChildren(user, true).OfType(); } - public async Task CreatePlaylist(PlaylistCreationRequest options) + public async Task CreatePlaylist(PlaylistCreationRequest request) { - var name = options.Name; + var name = request.Name; var folderName = _fileSystem.GetValidFilename(name); - var parentFolder = GetPlaylistsFolder(options.UserId); + var parentFolder = GetPlaylistsFolder(request.UserId); if (parentFolder is null) { throw new ArgumentException(nameof(parentFolder)); } - if (options.MediaType is null || options.MediaType == MediaType.Unknown) + if (request.MediaType is null || request.MediaType == MediaType.Unknown) { - foreach (var itemId in options.ItemIdList) + foreach (var itemId in request.ItemIdList) { var item = _libraryManager.GetItemById(itemId) ?? throw new ArgumentException("No item exists with the supplied Id"); if (item.MediaType != MediaType.Unknown) { - options.MediaType = item.MediaType; + request.MediaType = item.MediaType; } else if (item is MusicArtist || item is MusicAlbum || item is MusicGenre) { - options.MediaType = MediaType.Audio; + request.MediaType = MediaType.Audio; } else if (item is Genre) { - options.MediaType = MediaType.Video; + request.MediaType = MediaType.Video; } else { if (item is Folder folder) { - options.MediaType = folder.GetRecursiveChildren(i => !i.IsFolder && i.SupportsAddingToPlaylist) + request.MediaType = folder.GetRecursiveChildren(i => !i.IsFolder && i.SupportsAddingToPlaylist) .Select(i => i.MediaType) .FirstOrDefault(i => i != MediaType.Unknown); } } - if (options.MediaType is not null && options.MediaType != MediaType.Unknown) + if (request.MediaType is not null && request.MediaType != MediaType.Unknown) { break; } } } - if (options.MediaType is null || options.MediaType == MediaType.Unknown) + if (request.MediaType is null || request.MediaType == MediaType.Unknown) { - options.MediaType = MediaType.Audio; + request.MediaType = MediaType.Audio; } - var user = _userManager.GetUserById(options.UserId); + var user = _userManager.GetUserById(request.UserId); var path = Path.Combine(parentFolder.Path, folderName); path = GetTargetPath(path); @@ -133,20 +133,20 @@ namespace Emby.Server.Implementations.Playlists { Name = name, Path = path, - OwnerUserId = options.UserId, - Shares = options.Users ?? [], - OpenAccess = options.Public ?? false + OwnerUserId = request.UserId, + Shares = request.Users ?? [], + OpenAccess = request.Public ?? false }; - playlist.SetMediaType(options.MediaType); + playlist.SetMediaType(request.MediaType); parentFolder.AddChild(playlist); await playlist.RefreshMetadata(new MetadataRefreshOptions(new DirectoryService(_fileSystem)) { ForceSave = true }, CancellationToken.None) .ConfigureAwait(false); - if (options.ItemIdList.Count > 0) + if (request.ItemIdList.Count > 0) { - await AddToPlaylistInternal(playlist.Id, options.ItemIdList, user, new DtoOptions(false) + await AddToPlaylistInternal(playlist.Id, request.ItemIdList, user, new DtoOptions(false) { EnableImages = true }).ConfigureAwait(false); @@ -233,7 +233,7 @@ namespace Emby.Server.Implementations.Playlists // Update the playlist in the repository playlist.LinkedChildren = newLinkedChildren; - await UpdatePlaylist(playlist).ConfigureAwait(false); + await UpdatePlaylistInternal(playlist).ConfigureAwait(false); // Refresh playlist metadata _providerManager.QueueRefresh( @@ -262,7 +262,7 @@ namespace Emby.Server.Implementations.Playlists .Select(i => i.Item1) .ToArray(); - await UpdatePlaylist(playlist).ConfigureAwait(false); + await UpdatePlaylistInternal(playlist).ConfigureAwait(false); _providerManager.QueueRefresh( playlist.Id, @@ -306,7 +306,7 @@ namespace Emby.Server.Implementations.Playlists playlist.LinkedChildren = [.. newList]; - await UpdatePlaylist(playlist).ConfigureAwait(false); + await UpdatePlaylistInternal(playlist).ConfigureAwait(false); } /// @@ -530,7 +530,7 @@ namespace Emby.Server.Implementations.Playlists { playlist.OwnerUserId = rankedShares[0].UserId; playlist.Shares = rankedShares.Skip(1).ToArray(); - await UpdatePlaylist(playlist).ConfigureAwait(false); + await UpdatePlaylistInternal(playlist).ConfigureAwait(false); } else if (!playlist.OpenAccess) { @@ -548,12 +548,40 @@ namespace Emby.Server.Implementations.Playlists } } - public async Task ToggleOpenAccess(Guid playlistId, Guid userId) + public async Task UpdatePlaylist(PlaylistUpdateRequest request) { - var playlist = GetPlaylist(userId, playlistId); - playlist.OpenAccess = !playlist.OpenAccess; + var playlist = GetPlaylist(request.UserId, request.Id); - await UpdatePlaylist(playlist).ConfigureAwait(false); + if (request.Ids is not null) + { + playlist.LinkedChildren = []; + await UpdatePlaylistInternal(playlist).ConfigureAwait(false); + + var user = _userManager.GetUserById(request.UserId); + await AddToPlaylistInternal(request.Id, request.Ids, user, new DtoOptions(false) + { + EnableImages = true + }).ConfigureAwait(false); + + playlist = GetPlaylist(request.UserId, request.Id); + } + + if (request.Name is not null) + { + playlist.Name = request.Name; + } + + if (request.Users is not null) + { + playlist.Shares = request.Users; + } + + if (request.Public is not null) + { + playlist.OpenAccess = request.Public.Value; + } + + await UpdatePlaylistInternal(playlist).ConfigureAwait(false); } public async Task AddToShares(Guid playlistId, Guid userId, PlaylistUserPermissions share) @@ -568,7 +596,7 @@ namespace Emby.Server.Implementations.Playlists shares.Add(share); playlist.Shares = shares; - await UpdatePlaylist(playlist).ConfigureAwait(false); + await UpdatePlaylistInternal(playlist).ConfigureAwait(false); } public async Task RemoveFromShares(Guid playlistId, Guid userId, PlaylistUserPermissions share) @@ -577,10 +605,10 @@ namespace Emby.Server.Implementations.Playlists var shares = playlist.Shares.ToList(); shares.Remove(share); playlist.Shares = shares; - await UpdatePlaylist(playlist).ConfigureAwait(false); + await UpdatePlaylistInternal(playlist).ConfigureAwait(false); } - private async Task UpdatePlaylist(Playlist playlist) + private async Task UpdatePlaylistInternal(Playlist playlist) { await playlist.UpdateToRepositoryAsync(ItemUpdateType.MetadataEdit, CancellationToken.None).ConfigureAwait(false); diff --git a/Jellyfin.Api/Controllers/PlaylistsController.cs b/Jellyfin.Api/Controllers/PlaylistsController.cs index 862e5235e..de4c542d9 100644 --- a/Jellyfin.Api/Controllers/PlaylistsController.cs +++ b/Jellyfin.Api/Controllers/PlaylistsController.cs @@ -100,6 +100,54 @@ public class PlaylistsController : BaseJellyfinApiController return result; } + /// + /// Updates a playlist. + /// + /// The playlist id. + /// The id. + /// Playlist updated. + /// Unauthorized access. + /// Playlist not found. + /// + /// A that represents the asynchronous operation to update a playlist. + /// The task result contains an indicating success. + /// + [HttpPost("{playlistId}")] + [ProducesResponseType(StatusCodes.Status200OK)] + [ProducesResponseType(StatusCodes.Status401Unauthorized)] + public async Task UpdatePlaylist( + [FromRoute, Required] Guid playlistId, + [FromBody(EmptyBodyBehavior = EmptyBodyBehavior.Disallow)] UpdatePlaylistDto updatePlaylistRequest) + { + var callingUserId = User.GetUserId(); + + var playlist = _playlistManager.GetPlaylist(callingUserId, playlistId); + if (playlist is null) + { + return NotFound("Playlist not found"); + } + + var isPermitted = playlist.OwnerUserId.Equals(callingUserId) + || playlist.Shares.Any(s => s.CanEdit && s.UserId.Equals(callingUserId)); + + if (!isPermitted) + { + return Unauthorized("Unauthorized access"); + } + + await _playlistManager.UpdatePlaylist(new PlaylistUpdateRequest + { + UserId = callingUserId, + Id = playlistId, + Name = updatePlaylistRequest.Name, + Ids = updatePlaylistRequest.Ids, + Users = updatePlaylistRequest.Users, + Public = updatePlaylistRequest.Public + }).ConfigureAwait(false); + + return NoContent(); + } + /// /// Get a playlist's users. /// @@ -131,44 +179,6 @@ public class PlaylistsController : BaseJellyfinApiController return isPermitted ? playlist.Shares.ToList() : Unauthorized("Unauthorized Access"); } - /// - /// Toggles public access of a playlist. - /// - /// The playlist id. - /// Public access toggled. - /// Unauthorized access. - /// Playlist not found. - /// - /// A that represents the asynchronous operation to toggle public access of a playlist. - /// The task result contains an indicating success. - /// - [HttpPost("{playlistId}/TogglePublic")] - [ProducesResponseType(StatusCodes.Status204NoContent)] - [ProducesResponseType(StatusCodes.Status401Unauthorized)] - public async Task TogglePublicAccess( - [FromRoute, Required] Guid playlistId) - { - var callingUserId = User.GetUserId(); - - var playlist = _playlistManager.GetPlaylist(callingUserId, playlistId); - if (playlist is null) - { - return NotFound("Playlist not found"); - } - - var isPermitted = playlist.OwnerUserId.Equals(callingUserId) - || playlist.Shares.Any(s => s.CanEdit && s.UserId.Equals(callingUserId)); - - if (!isPermitted) - { - return Unauthorized("Unauthorized access"); - } - - await _playlistManager.ToggleOpenAccess(playlistId, callingUserId).ConfigureAwait(false); - - return NoContent(); - } - /// /// Modify a user to a playlist's users. /// @@ -206,7 +216,7 @@ public class PlaylistsController : BaseJellyfinApiController return Unauthorized("Unauthorized access"); } - await _playlistManager.AddToShares(playlistId, callingUserId, new PlaylistUserPermissions(userId.ToString(), canEdit)).ConfigureAwait(false); + await _playlistManager.AddToShares(playlistId, callingUserId, new PlaylistUserPermissions(userId, canEdit)).ConfigureAwait(false); return NoContent(); } diff --git a/Jellyfin.Api/Models/PlaylistDtos/UpdatePlaylistDto.cs b/Jellyfin.Api/Models/PlaylistDtos/UpdatePlaylistDto.cs new file mode 100644 index 000000000..93e544eed --- /dev/null +++ b/Jellyfin.Api/Models/PlaylistDtos/UpdatePlaylistDto.cs @@ -0,0 +1,34 @@ +using System; +using System.Collections.Generic; +using System.Text.Json.Serialization; +using Jellyfin.Extensions.Json.Converters; +using MediaBrowser.Model.Entities; + +namespace Jellyfin.Api.Models.PlaylistDtos; + +/// +/// Updateexisting playlist dto. +/// +public class UpdatePlaylistDto +{ + /// + /// Gets or sets the name of the new playlist. + /// + public string? Name { get; set; } + + /// + /// Gets or sets item ids of the playlist. + /// + [JsonConverter(typeof(JsonCommaDelimitedArrayConverterFactory))] + public IReadOnlyList? Ids { get; set; } + + /// + /// Gets or sets the playlist users. + /// + public IReadOnlyList? Users { get; set; } + + /// + /// Gets or sets a value indicating whether the playlist is public. + /// + public bool? Public { get; set; } +} diff --git a/Jellyfin.Server/Migrations/Routines/FixPlaylistOwner.cs b/Jellyfin.Server/Migrations/Routines/FixPlaylistOwner.cs index 06596c171..3655a610d 100644 --- a/Jellyfin.Server/Migrations/Routines/FixPlaylistOwner.cs +++ b/Jellyfin.Server/Migrations/Routines/FixPlaylistOwner.cs @@ -57,9 +57,9 @@ internal class FixPlaylistOwner : IMigrationRoutine if (shares.Count > 0) { var firstEditShare = shares.First(x => x.CanEdit); - if (firstEditShare is not null && Guid.TryParse(firstEditShare.UserId, out var guid)) + if (firstEditShare is not null) { - playlist.OwnerUserId = guid; + playlist.OwnerUserId = firstEditShare.UserId; playlist.Shares = shares.Where(x => x != firstEditShare).ToArray(); playlist.UpdateToRepositoryAsync(ItemUpdateType.MetadataEdit, CancellationToken.None).GetAwaiter().GetResult(); _playlistManager.SavePlaylistFile(playlist); diff --git a/MediaBrowser.Controller/Playlists/IPlaylistManager.cs b/MediaBrowser.Controller/Playlists/IPlaylistManager.cs index 1750be619..464620427 100644 --- a/MediaBrowser.Controller/Playlists/IPlaylistManager.cs +++ b/MediaBrowser.Controller/Playlists/IPlaylistManager.cs @@ -19,6 +19,20 @@ namespace MediaBrowser.Controller.Playlists /// Playlist. Playlist GetPlaylist(Guid userId, Guid playlistId); + /// + /// Creates the playlist. + /// + /// The . + /// Task<Playlist>. + Task CreatePlaylist(PlaylistCreationRequest request); + + /// + /// Updates a playlist. + /// + /// The . + /// Task. + Task UpdatePlaylist(PlaylistUpdateRequest request); + /// /// Gets the playlists. /// @@ -26,14 +40,6 @@ namespace MediaBrowser.Controller.Playlists /// IEnumerable<Playlist>. IEnumerable GetPlaylists(Guid userId); - /// - /// Toggle OpenAccess policy of the playlist. - /// - /// The playlist identifier. - /// The user identifier. - /// Task. - Task ToggleOpenAccess(Guid playlistId, Guid userId); - /// /// Adds a share to the playlist. /// @@ -52,13 +58,6 @@ namespace MediaBrowser.Controller.Playlists /// Task. Task RemoveFromShares(Guid playlistId, Guid userId, PlaylistUserPermissions share); - /// - /// Creates the playlist. - /// - /// The options. - /// Task<Playlist>. - Task CreatePlaylist(PlaylistCreationRequest options); - /// /// Adds to playlist. /// diff --git a/MediaBrowser.Controller/Playlists/Playlist.cs b/MediaBrowser.Controller/Playlists/Playlist.cs index b948d2e18..747dd9f63 100644 --- a/MediaBrowser.Controller/Playlists/Playlist.cs +++ b/MediaBrowser.Controller/Playlists/Playlist.cs @@ -252,7 +252,7 @@ namespace MediaBrowser.Controller.Playlists return false; } - return shares.Any(share => Guid.TryParse(share.UserId, out var id) && id.Equals(userId)); + return shares.Any(s => s.UserId.Equals(userId)); } public override bool IsVisibleStandalone(User user) diff --git a/MediaBrowser.LocalMetadata/Parsers/BaseItemXmlParser.cs b/MediaBrowser.LocalMetadata/Parsers/BaseItemXmlParser.cs index 22ae3f12b..a7e027d94 100644 --- a/MediaBrowser.LocalMetadata/Parsers/BaseItemXmlParser.cs +++ b/MediaBrowser.LocalMetadata/Parsers/BaseItemXmlParser.cs @@ -862,9 +862,9 @@ namespace MediaBrowser.LocalMetadata.Parsers } // This is valid - if (!string.IsNullOrWhiteSpace(userId)) + if (!string.IsNullOrWhiteSpace(userId) && Guid.TryParse(userId, out var guid)) { - return new PlaylistUserPermissions(userId, canEdit); + return new PlaylistUserPermissions(guid, canEdit); } return null; diff --git a/MediaBrowser.LocalMetadata/Savers/BaseXmlSaver.cs b/MediaBrowser.LocalMetadata/Savers/BaseXmlSaver.cs index 5a7193079..ee0d10bea 100644 --- a/MediaBrowser.LocalMetadata/Savers/BaseXmlSaver.cs +++ b/MediaBrowser.LocalMetadata/Savers/BaseXmlSaver.cs @@ -420,19 +420,16 @@ namespace MediaBrowser.LocalMetadata.Savers foreach (var share in item.Shares) { - if (share.UserId is not null) - { - await writer.WriteStartElementAsync(null, "Share", null).ConfigureAwait(false); + await writer.WriteStartElementAsync(null, "Share", null).ConfigureAwait(false); - await writer.WriteElementStringAsync(null, "UserId", null, share.UserId).ConfigureAwait(false); - await writer.WriteElementStringAsync( - null, - "CanEdit", - null, - share.CanEdit.ToString(CultureInfo.InvariantCulture).ToLowerInvariant()).ConfigureAwait(false); + await writer.WriteElementStringAsync(null, "UserId", null, share.UserId.ToString()).ConfigureAwait(false); + await writer.WriteElementStringAsync( + null, + "CanEdit", + null, + share.CanEdit.ToString(CultureInfo.InvariantCulture).ToLowerInvariant()).ConfigureAwait(false); - await writer.WriteEndElementAsync().ConfigureAwait(false); - } + await writer.WriteEndElementAsync().ConfigureAwait(false); } await writer.WriteEndElementAsync().ConfigureAwait(false); diff --git a/MediaBrowser.Model/Playlists/PlaylistUpdateRequest.cs b/MediaBrowser.Model/Playlists/PlaylistUpdateRequest.cs new file mode 100644 index 000000000..f574e679c --- /dev/null +++ b/MediaBrowser.Model/Playlists/PlaylistUpdateRequest.cs @@ -0,0 +1,41 @@ +using System; +using System.Collections.Generic; +using MediaBrowser.Model.Entities; + +namespace MediaBrowser.Model.Playlists; + +/// +/// A playlist creation request. +/// +public class PlaylistUpdateRequest +{ + /// + /// Gets or sets the id of the playlist. + /// + public Guid Id { get; set; } + + /// + /// Gets or sets the id of the user updating the playlist. + /// + public Guid UserId { get; set; } + + /// + /// Gets or sets the name of the playlist. + /// + public string? Name { get; set; } + + /// + /// Gets or sets item ids to add to the playlist. + /// + public IReadOnlyList? Ids { get; set; } + + /// + /// Gets or sets the playlist users. + /// + public IReadOnlyList? Users { get; set; } + + /// + /// Gets or sets a value indicating whether the playlist is public. + /// + public bool? Public { get; set; } +} From 8cf77424f6c432386fb06017eccfa0e9f880a3ba Mon Sep 17 00:00:00 2001 From: Shadowghost Date: Tue, 2 Apr 2024 08:08:36 +0200 Subject: [PATCH 07/17] Apply review suggestions --- .../Playlists/PlaylistManager.cs | 24 +++++++------- .../Controllers/PlaylistsController.cs | 31 +++++++++++-------- .../Models/PlaylistDtos/UpdatePlaylistDto.cs | 2 +- .../PlaylistDtos/UpdatePlaylistUserDto.cs | 12 +++++++ .../Playlists/IPlaylistManager.cs | 18 +++++------ .../Playlists/PlaylistUpdateRequest.cs | 2 +- .../Playlists/PlaylistUserUpdateRequest.cs | 24 ++++++++++++++ 7 files changed, 77 insertions(+), 36 deletions(-) create mode 100644 Jellyfin.Api/Models/PlaylistDtos/UpdatePlaylistUserDto.cs create mode 100644 MediaBrowser.Model/Playlists/PlaylistUserUpdateRequest.cs diff --git a/Emby.Server.Implementations/Playlists/PlaylistManager.cs b/Emby.Server.Implementations/Playlists/PlaylistManager.cs index 517ec68dc..7a6cf9eff 100644 --- a/Emby.Server.Implementations/Playlists/PlaylistManager.cs +++ b/Emby.Server.Implementations/Playlists/PlaylistManager.cs @@ -22,6 +22,7 @@ using MediaBrowser.Controller.Providers; using MediaBrowser.Model.Entities; using MediaBrowser.Model.IO; using MediaBrowser.Model.Playlists; +using Microsoft.EntityFrameworkCore.Diagnostics; using Microsoft.Extensions.Configuration; using Microsoft.Extensions.Logging; using PlaylistsNET.Content; @@ -59,7 +60,7 @@ namespace Emby.Server.Implementations.Playlists _appConfig = appConfig; } - public Playlist GetPlaylist(Guid userId, Guid playlistId) + public Playlist GetPlaylistForUser(Guid playlistId, Guid userId) { return GetPlaylists(userId).Where(p => p.Id.Equals(playlistId)).FirstOrDefault(); } @@ -178,7 +179,7 @@ namespace Emby.Server.Implementations.Playlists return Playlist.GetPlaylistItems(playlistMediaType, items, user, options); } - public Task AddToPlaylistAsync(Guid playlistId, IReadOnlyCollection itemIds, Guid userId) + public Task AddItemToPlaylistAsync(Guid playlistId, IReadOnlyCollection itemIds, Guid userId) { var user = userId.IsEmpty() ? null : _userManager.GetUserById(userId); @@ -245,7 +246,7 @@ namespace Emby.Server.Implementations.Playlists RefreshPriority.High); } - public async Task RemoveFromPlaylistAsync(string playlistId, IEnumerable entryIds) + public async Task RemoveItemFromPlaylistAsync(string playlistId, IEnumerable entryIds) { if (_libraryManager.GetItemById(playlistId) is not Playlist playlist) { @@ -550,7 +551,7 @@ namespace Emby.Server.Implementations.Playlists public async Task UpdatePlaylist(PlaylistUpdateRequest request) { - var playlist = GetPlaylist(request.UserId, request.Id); + var playlist = GetPlaylistForUser(request.Id, request.UserId); if (request.Ids is not null) { @@ -563,7 +564,7 @@ namespace Emby.Server.Implementations.Playlists EnableImages = true }).ConfigureAwait(false); - playlist = GetPlaylist(request.UserId, request.Id); + playlist = GetPlaylistForUser(request.Id, request.UserId); } if (request.Name is not null) @@ -584,24 +585,25 @@ namespace Emby.Server.Implementations.Playlists await UpdatePlaylistInternal(playlist).ConfigureAwait(false); } - public async Task AddToShares(Guid playlistId, Guid userId, PlaylistUserPermissions share) + public async Task AddUserToShares(PlaylistUserUpdateRequest request) { - var playlist = GetPlaylist(userId, playlistId); + var userId = request.UserId; + var playlist = GetPlaylistForUser(request.Id, userId); var shares = playlist.Shares.ToList(); - var existingUserShare = shares.FirstOrDefault(s => s.UserId.Equals(share.UserId)); + var existingUserShare = shares.FirstOrDefault(s => s.UserId.Equals(userId)); if (existingUserShare is not null) { shares.Remove(existingUserShare); } - shares.Add(share); + shares.Add(new PlaylistUserPermissions(userId, request.CanEdit ?? false)); playlist.Shares = shares; await UpdatePlaylistInternal(playlist).ConfigureAwait(false); } - public async Task RemoveFromShares(Guid playlistId, Guid userId, PlaylistUserPermissions share) + public async Task RemoveUserFromShares(Guid playlistId, Guid userId, PlaylistUserPermissions share) { - var playlist = GetPlaylist(userId, playlistId); + var playlist = GetPlaylistForUser(playlistId, userId); var shares = playlist.Shares.ToList(); shares.Remove(share); playlist.Shares = shares; diff --git a/Jellyfin.Api/Controllers/PlaylistsController.cs b/Jellyfin.Api/Controllers/PlaylistsController.cs index de4c542d9..12186e02e 100644 --- a/Jellyfin.Api/Controllers/PlaylistsController.cs +++ b/Jellyfin.Api/Controllers/PlaylistsController.cs @@ -121,7 +121,7 @@ public class PlaylistsController : BaseJellyfinApiController { var callingUserId = User.GetUserId(); - var playlist = _playlistManager.GetPlaylist(callingUserId, playlistId); + var playlist = _playlistManager.GetPlaylistForUser(playlistId, callingUserId); if (playlist is null) { return NotFound("Playlist not found"); @@ -167,7 +167,7 @@ public class PlaylistsController : BaseJellyfinApiController { var userId = User.GetUserId(); - var playlist = _playlistManager.GetPlaylist(userId, playlistId); + var playlist = _playlistManager.GetPlaylistForUser(playlistId, userId); if (playlist is null) { return NotFound("Playlist not found"); @@ -184,7 +184,7 @@ public class PlaylistsController : BaseJellyfinApiController /// /// The playlist id. /// The user id. - /// Edit permission. + /// The . /// User's permissions modified. /// Unauthorized access. /// Playlist not found. @@ -195,14 +195,14 @@ public class PlaylistsController : BaseJellyfinApiController [HttpPost("{playlistId}/User/{userId}")] [ProducesResponseType(StatusCodes.Status204NoContent)] [ProducesResponseType(StatusCodes.Status401Unauthorized)] - public async Task ModifyPlaylistUserPermissions( + public async Task UpdatePlaylistUser( [FromRoute, Required] Guid playlistId, [FromRoute, Required] Guid userId, - [FromBody] bool canEdit) + [FromBody(EmptyBodyBehavior = EmptyBodyBehavior.Allow)] UpdatePlaylistUserDto updatePlaylistUserRequest) { var callingUserId = User.GetUserId(); - var playlist = _playlistManager.GetPlaylist(callingUserId, playlistId); + var playlist = _playlistManager.GetPlaylistForUser(playlistId, callingUserId); if (playlist is null) { return NotFound("Playlist not found"); @@ -216,7 +216,12 @@ public class PlaylistsController : BaseJellyfinApiController return Unauthorized("Unauthorized access"); } - await _playlistManager.AddToShares(playlistId, callingUserId, new PlaylistUserPermissions(userId, canEdit)).ConfigureAwait(false); + await _playlistManager.AddUserToShares(new PlaylistUserUpdateRequest + { + Id = playlistId, + UserId = userId, + CanEdit = updatePlaylistUserRequest.CanEdit + }).ConfigureAwait(false); return NoContent(); } @@ -243,7 +248,7 @@ public class PlaylistsController : BaseJellyfinApiController { var callingUserId = User.GetUserId(); - var playlist = _playlistManager.GetPlaylist(callingUserId, playlistId); + var playlist = _playlistManager.GetPlaylistForUser(playlistId, callingUserId); if (playlist is null) { return NotFound("Playlist not found"); @@ -263,7 +268,7 @@ public class PlaylistsController : BaseJellyfinApiController return NotFound("User permissions not found"); } - await _playlistManager.RemoveFromShares(playlistId, callingUserId, share).ConfigureAwait(false); + await _playlistManager.RemoveUserFromShares(playlistId, callingUserId, share).ConfigureAwait(false); return NoContent(); } @@ -278,13 +283,13 @@ public class PlaylistsController : BaseJellyfinApiController /// An on success. [HttpPost("{playlistId}/Items")] [ProducesResponseType(StatusCodes.Status204NoContent)] - public async Task AddToPlaylist( + public async Task AddItemToPlaylist( [FromRoute, Required] Guid playlistId, [FromQuery, ModelBinder(typeof(CommaDelimitedArrayModelBinder))] Guid[] ids, [FromQuery] Guid? userId) { userId = RequestHelpers.GetUserId(User, userId); - await _playlistManager.AddToPlaylistAsync(playlistId, ids, userId.Value).ConfigureAwait(false); + await _playlistManager.AddItemToPlaylistAsync(playlistId, ids, userId.Value).ConfigureAwait(false); return NoContent(); } @@ -316,11 +321,11 @@ public class PlaylistsController : BaseJellyfinApiController /// An on success. [HttpDelete("{playlistId}/Items")] [ProducesResponseType(StatusCodes.Status204NoContent)] - public async Task RemoveFromPlaylist( + public async Task RemoveItemFromPlaylist( [FromRoute, Required] string playlistId, [FromQuery, ModelBinder(typeof(CommaDelimitedArrayModelBinder))] string[] entryIds) { - await _playlistManager.RemoveFromPlaylistAsync(playlistId, entryIds).ConfigureAwait(false); + await _playlistManager.RemoveItemFromPlaylistAsync(playlistId, entryIds).ConfigureAwait(false); return NoContent(); } diff --git a/Jellyfin.Api/Models/PlaylistDtos/UpdatePlaylistDto.cs b/Jellyfin.Api/Models/PlaylistDtos/UpdatePlaylistDto.cs index 93e544eed..0e109db3e 100644 --- a/Jellyfin.Api/Models/PlaylistDtos/UpdatePlaylistDto.cs +++ b/Jellyfin.Api/Models/PlaylistDtos/UpdatePlaylistDto.cs @@ -7,7 +7,7 @@ using MediaBrowser.Model.Entities; namespace Jellyfin.Api.Models.PlaylistDtos; /// -/// Updateexisting playlist dto. +/// Update existing playlist dto. Fields set to `null` will not be updated and keep their current values. /// public class UpdatePlaylistDto { diff --git a/Jellyfin.Api/Models/PlaylistDtos/UpdatePlaylistUserDto.cs b/Jellyfin.Api/Models/PlaylistDtos/UpdatePlaylistUserDto.cs new file mode 100644 index 000000000..60467b5e7 --- /dev/null +++ b/Jellyfin.Api/Models/PlaylistDtos/UpdatePlaylistUserDto.cs @@ -0,0 +1,12 @@ +namespace Jellyfin.Api.Models.PlaylistDtos; + +/// +/// Update existing playlist user dto. Fields set to `null` will not be updated and keep their current values. +/// +public class UpdatePlaylistUserDto +{ + /// + /// Gets or sets a value indicating whether the user can edit the playlist. + /// + public bool? CanEdit { get; set; } +} diff --git a/MediaBrowser.Controller/Playlists/IPlaylistManager.cs b/MediaBrowser.Controller/Playlists/IPlaylistManager.cs index 464620427..821b901a0 100644 --- a/MediaBrowser.Controller/Playlists/IPlaylistManager.cs +++ b/MediaBrowser.Controller/Playlists/IPlaylistManager.cs @@ -14,10 +14,10 @@ namespace MediaBrowser.Controller.Playlists /// /// Gets the playlist. /// - /// The user identifier. /// The playlist identifier. + /// The user identifier. /// Playlist. - Playlist GetPlaylist(Guid userId, Guid playlistId); + Playlist GetPlaylistForUser(Guid playlistId, Guid userId); /// /// Creates the playlist. @@ -43,20 +43,18 @@ namespace MediaBrowser.Controller.Playlists /// /// Adds a share to the playlist. /// - /// The playlist identifier. - /// The user identifier. - /// The share. + /// The . /// Task. - Task AddToShares(Guid playlistId, Guid userId, PlaylistUserPermissions share); + Task AddUserToShares(PlaylistUserUpdateRequest request); /// - /// Rremoves a share from the playlist. + /// Removes a share from the playlist. /// /// The playlist identifier. /// The user identifier. /// The share. /// Task. - Task RemoveFromShares(Guid playlistId, Guid userId, PlaylistUserPermissions share); + Task RemoveUserFromShares(Guid playlistId, Guid userId, PlaylistUserPermissions share); /// /// Adds to playlist. @@ -65,7 +63,7 @@ namespace MediaBrowser.Controller.Playlists /// The item ids. /// The user identifier. /// Task. - Task AddToPlaylistAsync(Guid playlistId, IReadOnlyCollection itemIds, Guid userId); + Task AddItemToPlaylistAsync(Guid playlistId, IReadOnlyCollection itemIds, Guid userId); /// /// Removes from playlist. @@ -73,7 +71,7 @@ namespace MediaBrowser.Controller.Playlists /// The playlist identifier. /// The entry ids. /// Task. - Task RemoveFromPlaylistAsync(string playlistId, IEnumerable entryIds); + Task RemoveItemFromPlaylistAsync(string playlistId, IEnumerable entryIds); /// /// Gets the playlists folder. diff --git a/MediaBrowser.Model/Playlists/PlaylistUpdateRequest.cs b/MediaBrowser.Model/Playlists/PlaylistUpdateRequest.cs index f574e679c..db290bbdb 100644 --- a/MediaBrowser.Model/Playlists/PlaylistUpdateRequest.cs +++ b/MediaBrowser.Model/Playlists/PlaylistUpdateRequest.cs @@ -5,7 +5,7 @@ using MediaBrowser.Model.Entities; namespace MediaBrowser.Model.Playlists; /// -/// A playlist creation request. +/// A playlist update request. /// public class PlaylistUpdateRequest { diff --git a/MediaBrowser.Model/Playlists/PlaylistUserUpdateRequest.cs b/MediaBrowser.Model/Playlists/PlaylistUserUpdateRequest.cs new file mode 100644 index 000000000..1840efdf3 --- /dev/null +++ b/MediaBrowser.Model/Playlists/PlaylistUserUpdateRequest.cs @@ -0,0 +1,24 @@ +using System; + +namespace MediaBrowser.Model.Playlists; + +/// +/// A playlist user update request. +/// +public class PlaylistUserUpdateRequest +{ + /// + /// Gets or sets the id of the playlist. + /// + public Guid Id { get; set; } + + /// + /// Gets or sets the id of the updated user. + /// + public Guid UserId { get; set; } + + /// + /// Gets or sets a value indicating whether the user can edit the playlist. + /// + public bool? CanEdit { get; set; } +} From 3e0b201688d6efeca5c65df11425001f73c8c9ec Mon Sep 17 00:00:00 2001 From: Shadowghost Date: Wed, 3 Apr 2024 16:06:20 +0200 Subject: [PATCH 08/17] Enforce permissions --- .../Controllers/PlaylistsController.cs | 105 +++++++++++++++--- .../Playlists/IPlaylistManager.cs | 2 +- 2 files changed, 88 insertions(+), 19 deletions(-) diff --git a/Jellyfin.Api/Controllers/PlaylistsController.cs b/Jellyfin.Api/Controllers/PlaylistsController.cs index 12186e02e..4ced64ae7 100644 --- a/Jellyfin.Api/Controllers/PlaylistsController.cs +++ b/Jellyfin.Api/Controllers/PlaylistsController.cs @@ -106,7 +106,7 @@ public class PlaylistsController : BaseJellyfinApiController /// The playlist id. /// The id. /// Playlist updated. - /// Unauthorized access. + /// Access forbidden. /// Playlist not found. /// /// A that represents the asynchronous operation to update a playlist. @@ -114,10 +114,11 @@ public class PlaylistsController : BaseJellyfinApiController /// [HttpPost("{playlistId}")] [ProducesResponseType(StatusCodes.Status200OK)] - [ProducesResponseType(StatusCodes.Status401Unauthorized)] + [ProducesResponseType(StatusCodes.Status403Forbidden)] + [ProducesResponseType(StatusCodes.Status404NotFound)] public async Task UpdatePlaylist( [FromRoute, Required] Guid playlistId, - [FromBody(EmptyBodyBehavior = EmptyBodyBehavior.Disallow)] UpdatePlaylistDto updatePlaylistRequest) + [FromBody, Required] UpdatePlaylistDto updatePlaylistRequest) { var callingUserId = User.GetUserId(); @@ -132,7 +133,7 @@ public class PlaylistsController : BaseJellyfinApiController if (!isPermitted) { - return Unauthorized("Unauthorized access"); + return Forbid(); } await _playlistManager.UpdatePlaylist(new PlaylistUpdateRequest @@ -153,14 +154,14 @@ public class PlaylistsController : BaseJellyfinApiController /// /// The playlist id. /// Found shares. - /// Unauthorized access. + /// Access forbidden. /// Playlist not found. /// /// A list of objects. /// [HttpGet("{playlistId}/User")] [ProducesResponseType(StatusCodes.Status200OK)] - [ProducesResponseType(StatusCodes.Status401Unauthorized)] + [ProducesResponseType(StatusCodes.Status403Forbidden)] [ProducesResponseType(StatusCodes.Status404NotFound)] public ActionResult> GetPlaylistUsers( [FromRoute, Required] Guid playlistId) @@ -173,10 +174,9 @@ public class PlaylistsController : BaseJellyfinApiController return NotFound("Playlist not found"); } - var isPermitted = playlist.OwnerUserId.Equals(userId) - || playlist.Shares.Any(s => s.CanEdit && s.UserId.Equals(userId)); + var isPermitted = playlist.OwnerUserId.Equals(userId); - return isPermitted ? playlist.Shares.ToList() : Unauthorized("Unauthorized Access"); + return isPermitted ? playlist.Shares.ToList() : Forbid(); } /// @@ -186,7 +186,7 @@ public class PlaylistsController : BaseJellyfinApiController /// The user id. /// The . /// User's permissions modified. - /// Unauthorized access. + /// Access forbidden. /// Playlist not found. /// /// A that represents the asynchronous operation to modify an user's playlist permissions. @@ -194,7 +194,8 @@ public class PlaylistsController : BaseJellyfinApiController /// [HttpPost("{playlistId}/User/{userId}")] [ProducesResponseType(StatusCodes.Status204NoContent)] - [ProducesResponseType(StatusCodes.Status401Unauthorized)] + [ProducesResponseType(StatusCodes.Status403Forbidden)] + [ProducesResponseType(StatusCodes.Status404NotFound)] public async Task UpdatePlaylistUser( [FromRoute, Required] Guid playlistId, [FromRoute, Required] Guid userId, @@ -208,12 +209,11 @@ public class PlaylistsController : BaseJellyfinApiController return NotFound("Playlist not found"); } - var isPermitted = playlist.OwnerUserId.Equals(callingUserId) - || playlist.Shares.Any(s => s.CanEdit && s.UserId.Equals(callingUserId)); + var isPermitted = playlist.OwnerUserId.Equals(callingUserId); if (!isPermitted) { - return Unauthorized("Unauthorized access"); + return Forbid(); } await _playlistManager.AddUserToShares(new PlaylistUserUpdateRequest @@ -240,7 +240,7 @@ public class PlaylistsController : BaseJellyfinApiController /// [HttpDelete("{playlistId}/User/{userId}")] [ProducesResponseType(StatusCodes.Status204NoContent)] - [ProducesResponseType(StatusCodes.Status401Unauthorized)] + [ProducesResponseType(StatusCodes.Status403Forbidden)] [ProducesResponseType(StatusCodes.Status404NotFound)] public async Task RemoveUserFromPlaylist( [FromRoute, Required] Guid playlistId, @@ -259,7 +259,7 @@ public class PlaylistsController : BaseJellyfinApiController if (!isPermitted) { - return Unauthorized("Unauthorized access"); + return Forbid(); } var share = playlist.Shares.FirstOrDefault(s => s.UserId.Equals(userId)); @@ -280,15 +280,33 @@ public class PlaylistsController : BaseJellyfinApiController /// Item id, comma delimited. /// The userId. /// Items added to playlist. + /// Access forbidden. + /// Playlist not found. /// An on success. [HttpPost("{playlistId}/Items")] [ProducesResponseType(StatusCodes.Status204NoContent)] + [ProducesResponseType(StatusCodes.Status403Forbidden)] + [ProducesResponseType(StatusCodes.Status404NotFound)] public async Task AddItemToPlaylist( [FromRoute, Required] Guid playlistId, [FromQuery, ModelBinder(typeof(CommaDelimitedArrayModelBinder))] Guid[] ids, [FromQuery] Guid? userId) { userId = RequestHelpers.GetUserId(User, userId); + var playlist = _playlistManager.GetPlaylistForUser(playlistId, userId.Value); + if (playlist is null) + { + return NotFound("Playlist not found"); + } + + var isPermitted = playlist.OwnerUserId.Equals(userId.Value) + || playlist.Shares.Any(s => s.CanEdit && s.UserId.Equals(userId.Value)); + + if (!isPermitted) + { + return Forbid(); + } + await _playlistManager.AddItemToPlaylistAsync(playlistId, ids, userId.Value).ConfigureAwait(false); return NoContent(); } @@ -300,14 +318,34 @@ public class PlaylistsController : BaseJellyfinApiController /// The item id. /// The new index. /// Item moved to new index. + /// Access forbidden. + /// Playlist not found. /// An on success. [HttpPost("{playlistId}/Items/{itemId}/Move/{newIndex}")] [ProducesResponseType(StatusCodes.Status204NoContent)] + [ProducesResponseType(StatusCodes.Status403Forbidden)] + [ProducesResponseType(StatusCodes.Status404NotFound)] public async Task MoveItem( [FromRoute, Required] string playlistId, [FromRoute, Required] string itemId, [FromRoute, Required] int newIndex) { + var callingUserId = User.GetUserId(); + + var playlist = _playlistManager.GetPlaylistForUser(Guid.Parse(playlistId), callingUserId); + if (playlist is null) + { + return NotFound("Playlist not found"); + } + + var isPermitted = playlist.OwnerUserId.Equals(callingUserId) + || playlist.Shares.Any(s => s.CanEdit && s.UserId.Equals(callingUserId)); + + if (!isPermitted) + { + return Forbid(); + } + await _playlistManager.MoveItemAsync(playlistId, itemId, newIndex).ConfigureAwait(false); return NoContent(); } @@ -318,13 +356,33 @@ public class PlaylistsController : BaseJellyfinApiController /// The playlist id. /// The item ids, comma delimited. /// Items removed. + /// Access forbidden. + /// Playlist not found. /// An on success. [HttpDelete("{playlistId}/Items")] [ProducesResponseType(StatusCodes.Status204NoContent)] + [ProducesResponseType(StatusCodes.Status403Forbidden)] + [ProducesResponseType(StatusCodes.Status404NotFound)] public async Task RemoveItemFromPlaylist( [FromRoute, Required] string playlistId, [FromQuery, ModelBinder(typeof(CommaDelimitedArrayModelBinder))] string[] entryIds) { + var callingUserId = User.GetUserId(); + + var playlist = _playlistManager.GetPlaylistForUser(Guid.Parse(playlistId), callingUserId); + if (playlist is null) + { + return NotFound("Playlist not found"); + } + + var isPermitted = playlist.OwnerUserId.Equals(callingUserId) + || playlist.Shares.Any(s => s.CanEdit && s.UserId.Equals(callingUserId)); + + if (!isPermitted) + { + return Forbid(); + } + await _playlistManager.RemoveItemFromPlaylistAsync(playlistId, entryIds).ConfigureAwait(false); return NoContent(); } @@ -342,10 +400,12 @@ public class PlaylistsController : BaseJellyfinApiController /// Optional. The max number of images to return, per image type. /// Optional. The image types to include in the output. /// Original playlist returned. + /// Access forbidden. /// Playlist not found. /// The original playlist items. [HttpGet("{playlistId}/Items")] [ProducesResponseType(StatusCodes.Status200OK)] + [ProducesResponseType(StatusCodes.Status403Forbidden)] [ProducesResponseType(StatusCodes.Status404NotFound)] public ActionResult> GetPlaylistItems( [FromRoute, Required] Guid playlistId, @@ -359,10 +419,19 @@ public class PlaylistsController : BaseJellyfinApiController [FromQuery, ModelBinder(typeof(CommaDelimitedArrayModelBinder))] ImageType[] enableImageTypes) { userId = RequestHelpers.GetUserId(User, userId); - var playlist = (Playlist)_libraryManager.GetItemById(playlistId); + var playlist = _playlistManager.GetPlaylistForUser(playlistId, userId.Value); if (playlist is null) { - return NotFound(); + return NotFound("Playlist not found"); + } + + var isPermitted = playlist.OpenAccess + || playlist.OwnerUserId.Equals(userId.Value) + || playlist.Shares.Any(s => s.UserId.Equals(userId.Value)); + + if (!isPermitted) + { + return Forbid(); } var user = userId.IsNullOrEmpty() diff --git a/MediaBrowser.Controller/Playlists/IPlaylistManager.cs b/MediaBrowser.Controller/Playlists/IPlaylistManager.cs index 821b901a0..cbe4bd87f 100644 --- a/MediaBrowser.Controller/Playlists/IPlaylistManager.cs +++ b/MediaBrowser.Controller/Playlists/IPlaylistManager.cs @@ -23,7 +23,7 @@ namespace MediaBrowser.Controller.Playlists /// Creates the playlist. /// /// The . - /// Task<Playlist>. + /// The created playlist. Task CreatePlaylist(PlaylistCreationRequest request); /// From 04c5b9d6800d7790d08958b9d6700299ba0c8e74 Mon Sep 17 00:00:00 2001 From: Shadowghost Date: Wed, 3 Apr 2024 16:14:06 +0200 Subject: [PATCH 09/17] Add endpoint to get user permissions --- .../Controllers/PlaylistsController.cs | 34 +++++++++++++++++++ 1 file changed, 34 insertions(+) diff --git a/Jellyfin.Api/Controllers/PlaylistsController.cs b/Jellyfin.Api/Controllers/PlaylistsController.cs index 4ced64ae7..567a27412 100644 --- a/Jellyfin.Api/Controllers/PlaylistsController.cs +++ b/Jellyfin.Api/Controllers/PlaylistsController.cs @@ -179,6 +179,40 @@ public class PlaylistsController : BaseJellyfinApiController return isPermitted ? playlist.Shares.ToList() : Forbid(); } + /// + /// Get a playlist users. + /// + /// The playlist id. + /// The user id. + /// Found shares. + /// Access forbidden. + /// Playlist not found. + /// + /// . + /// + [HttpGet("{playlistId}/User/{userId}")] + [ProducesResponseType(StatusCodes.Status200OK)] + [ProducesResponseType(StatusCodes.Status403Forbidden)] + [ProducesResponseType(StatusCodes.Status404NotFound)] + public ActionResult GetPlaylistUser( + [FromRoute, Required] Guid playlistId, + [FromRoute, Required] Guid userId) + { + var callingUserId = User.GetUserId(); + + var playlist = _playlistManager.GetPlaylistForUser(playlistId, callingUserId); + if (playlist is null) + { + return NotFound("Playlist not found"); + } + + var isPermitted = playlist.OwnerUserId.Equals(userId) + || playlist.Shares.Any(s => s.CanEdit && s.UserId.Equals(callingUserId)) + || userId.Equals(callingUserId); + + return isPermitted ? playlist.Shares.FirstOrDefault(s => s.UserId.Equals(userId)) : Forbid(); + } + /// /// Modify a user to a playlist's users. /// From d72f40fe4159d83440c3362d137901e4c418c4b9 Mon Sep 17 00:00:00 2001 From: Shadowghost Date: Wed, 3 Apr 2024 16:19:13 +0200 Subject: [PATCH 10/17] Return 204 on OpenAccess --- Jellyfin.Api/Controllers/PlaylistsController.cs | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/Jellyfin.Api/Controllers/PlaylistsController.cs b/Jellyfin.Api/Controllers/PlaylistsController.cs index 567a27412..d6239503c 100644 --- a/Jellyfin.Api/Controllers/PlaylistsController.cs +++ b/Jellyfin.Api/Controllers/PlaylistsController.cs @@ -184,7 +184,8 @@ public class PlaylistsController : BaseJellyfinApiController /// /// The playlist id. /// The user id. - /// Found shares. + /// User permission found. + /// No user permission found but open access. /// Access forbidden. /// Playlist not found. /// @@ -192,6 +193,7 @@ public class PlaylistsController : BaseJellyfinApiController /// [HttpGet("{playlistId}/User/{userId}")] [ProducesResponseType(StatusCodes.Status200OK)] + [ProducesResponseType(StatusCodes.Status204NoContent)] [ProducesResponseType(StatusCodes.Status403Forbidden)] [ProducesResponseType(StatusCodes.Status404NotFound)] public ActionResult GetPlaylistUser( @@ -210,7 +212,7 @@ public class PlaylistsController : BaseJellyfinApiController || playlist.Shares.Any(s => s.CanEdit && s.UserId.Equals(callingUserId)) || userId.Equals(callingUserId); - return isPermitted ? playlist.Shares.FirstOrDefault(s => s.UserId.Equals(userId)) : Forbid(); + return isPermitted ? playlist.Shares.FirstOrDefault(s => s.UserId.Equals(userId)) : playlist.OpenAccess ? NoContent() : Forbid(); } /// From 247ec19de4945f4d7ab64d4a8772b72759e3d2b7 Mon Sep 17 00:00:00 2001 From: Shadowghost Date: Wed, 3 Apr 2024 16:23:14 +0200 Subject: [PATCH 11/17] Fixup --- Jellyfin.Api/Controllers/PlaylistsController.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Jellyfin.Api/Controllers/PlaylistsController.cs b/Jellyfin.Api/Controllers/PlaylistsController.cs index d6239503c..54b2261ab 100644 --- a/Jellyfin.Api/Controllers/PlaylistsController.cs +++ b/Jellyfin.Api/Controllers/PlaylistsController.cs @@ -208,7 +208,7 @@ public class PlaylistsController : BaseJellyfinApiController return NotFound("Playlist not found"); } - var isPermitted = playlist.OwnerUserId.Equals(userId) + var isPermitted = playlist.OwnerUserId.Equals(callingUserId) || playlist.Shares.Any(s => s.CanEdit && s.UserId.Equals(callingUserId)) || userId.Equals(callingUserId); From 5396b616bf2d7e387bc20539eeae03e841f60c01 Mon Sep 17 00:00:00 2001 From: Shadowghost Date: Wed, 3 Apr 2024 16:32:25 +0200 Subject: [PATCH 12/17] Fixup --- Jellyfin.Api/Controllers/PlaylistsController.cs | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/Jellyfin.Api/Controllers/PlaylistsController.cs b/Jellyfin.Api/Controllers/PlaylistsController.cs index 54b2261ab..01c1c578d 100644 --- a/Jellyfin.Api/Controllers/PlaylistsController.cs +++ b/Jellyfin.Api/Controllers/PlaylistsController.cs @@ -208,11 +208,12 @@ public class PlaylistsController : BaseJellyfinApiController return NotFound("Playlist not found"); } + var userPermission = playlist.Shares.FirstOrDefault(s => s.UserId.Equals(userId)); var isPermitted = playlist.OwnerUserId.Equals(callingUserId) || playlist.Shares.Any(s => s.CanEdit && s.UserId.Equals(callingUserId)) || userId.Equals(callingUserId); - return isPermitted ? playlist.Shares.FirstOrDefault(s => s.UserId.Equals(userId)) : playlist.OpenAccess ? NoContent() : Forbid(); + return isPermitted ? userPermission is not null ? userPermission : NotFound("User permissions not found") : playlist.OpenAccess ? NoContent() : Forbid(); } /// From 3c7562313b3d0a8bdaaa6623215f2c979150cf5f Mon Sep 17 00:00:00 2001 From: Shadowghost Date: Wed, 3 Apr 2024 16:57:10 +0200 Subject: [PATCH 13/17] Apply review suggestions --- Jellyfin.Api/Controllers/PlaylistsController.cs | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/Jellyfin.Api/Controllers/PlaylistsController.cs b/Jellyfin.Api/Controllers/PlaylistsController.cs index 01c1c578d..d167d996c 100644 --- a/Jellyfin.Api/Controllers/PlaylistsController.cs +++ b/Jellyfin.Api/Controllers/PlaylistsController.cs @@ -159,7 +159,7 @@ public class PlaylistsController : BaseJellyfinApiController /// /// A list of objects. /// - [HttpGet("{playlistId}/User")] + [HttpGet("{playlistId}/Users")] [ProducesResponseType(StatusCodes.Status200OK)] [ProducesResponseType(StatusCodes.Status403Forbidden)] [ProducesResponseType(StatusCodes.Status404NotFound)] @@ -191,7 +191,7 @@ public class PlaylistsController : BaseJellyfinApiController /// /// . /// - [HttpGet("{playlistId}/User/{userId}")] + [HttpGet("{playlistId}/Users/{userId}")] [ProducesResponseType(StatusCodes.Status200OK)] [ProducesResponseType(StatusCodes.Status204NoContent)] [ProducesResponseType(StatusCodes.Status403Forbidden)] @@ -229,7 +229,7 @@ public class PlaylistsController : BaseJellyfinApiController /// A that represents the asynchronous operation to modify an user's playlist permissions. /// The task result contains an indicating success. /// - [HttpPost("{playlistId}/User/{userId}")] + [HttpPost("{playlistId}/Users/{userId}")] [ProducesResponseType(StatusCodes.Status204NoContent)] [ProducesResponseType(StatusCodes.Status403Forbidden)] [ProducesResponseType(StatusCodes.Status404NotFound)] @@ -275,7 +275,7 @@ public class PlaylistsController : BaseJellyfinApiController /// A that represents the asynchronous operation to delete a user from a playlist's shares. /// The task result contains an indicating success. /// - [HttpDelete("{playlistId}/User/{userId}")] + [HttpDelete("{playlistId}/Users/{userId}")] [ProducesResponseType(StatusCodes.Status204NoContent)] [ProducesResponseType(StatusCodes.Status403Forbidden)] [ProducesResponseType(StatusCodes.Status404NotFound)] From 51e2faa448624a334128042cdca1e592ec97240e Mon Sep 17 00:00:00 2001 From: Shadowghost Date: Wed, 3 Apr 2024 20:06:57 +0200 Subject: [PATCH 14/17] Apply review suggestions --- Jellyfin.Api/Controllers/PlaylistsController.cs | 13 +++++++------ 1 file changed, 7 insertions(+), 6 deletions(-) diff --git a/Jellyfin.Api/Controllers/PlaylistsController.cs b/Jellyfin.Api/Controllers/PlaylistsController.cs index d167d996c..ca90d2a6d 100644 --- a/Jellyfin.Api/Controllers/PlaylistsController.cs +++ b/Jellyfin.Api/Controllers/PlaylistsController.cs @@ -113,7 +113,7 @@ public class PlaylistsController : BaseJellyfinApiController /// The task result contains an indicating success. /// [HttpPost("{playlistId}")] - [ProducesResponseType(StatusCodes.Status200OK)] + [ProducesResponseType(StatusCodes.Status204NoContent)] [ProducesResponseType(StatusCodes.Status403Forbidden)] [ProducesResponseType(StatusCodes.Status404NotFound)] public async Task UpdatePlaylist( @@ -185,16 +185,12 @@ public class PlaylistsController : BaseJellyfinApiController /// The playlist id. /// The user id. /// User permission found. - /// No user permission found but open access. - /// Access forbidden. /// Playlist not found. /// /// . /// [HttpGet("{playlistId}/Users/{userId}")] [ProducesResponseType(StatusCodes.Status200OK)] - [ProducesResponseType(StatusCodes.Status204NoContent)] - [ProducesResponseType(StatusCodes.Status403Forbidden)] [ProducesResponseType(StatusCodes.Status404NotFound)] public ActionResult GetPlaylistUser( [FromRoute, Required] Guid playlistId, @@ -213,7 +209,12 @@ public class PlaylistsController : BaseJellyfinApiController || playlist.Shares.Any(s => s.CanEdit && s.UserId.Equals(callingUserId)) || userId.Equals(callingUserId); - return isPermitted ? userPermission is not null ? userPermission : NotFound("User permissions not found") : playlist.OpenAccess ? NoContent() : Forbid(); + if (isPermitted && userPermission is not null) + { + return userPermission; + } + + return NotFound("User permissions not found"); } /// From e3897fe5ddc6013da43557f03b4836e5acfde18c Mon Sep 17 00:00:00 2001 From: Shadowghost Date: Wed, 3 Apr 2024 21:20:30 +0200 Subject: [PATCH 15/17] Apply review suggestions --- .../Controllers/PlaylistsController.cs | 21 ++++++++++++------- .../Models/PlaylistDtos/CreatePlaylistDto.cs | 2 +- .../Models/PlaylistDtos/UpdatePlaylistDto.cs | 2 +- 3 files changed, 16 insertions(+), 9 deletions(-) diff --git a/Jellyfin.Api/Controllers/PlaylistsController.cs b/Jellyfin.Api/Controllers/PlaylistsController.cs index ca90d2a6d..69abe5f7e 100644 --- a/Jellyfin.Api/Controllers/PlaylistsController.cs +++ b/Jellyfin.Api/Controllers/PlaylistsController.cs @@ -94,7 +94,7 @@ public class PlaylistsController : BaseJellyfinApiController UserId = userId.Value, MediaType = mediaType ?? createPlaylistRequest?.MediaType, Users = createPlaylistRequest?.Users.ToArray() ?? [], - Public = createPlaylistRequest?.Public + Public = createPlaylistRequest?.IsPublic }).ConfigureAwait(false); return result; @@ -143,7 +143,7 @@ public class PlaylistsController : BaseJellyfinApiController Name = updatePlaylistRequest.Name, Ids = updatePlaylistRequest.Ids, Users = updatePlaylistRequest.Users, - Public = updatePlaylistRequest.Public + Public = updatePlaylistRequest.IsPublic }).ConfigureAwait(false); return NoContent(); @@ -180,17 +180,19 @@ public class PlaylistsController : BaseJellyfinApiController } /// - /// Get a playlist users. + /// Get a playlist user. /// /// The playlist id. /// The user id. /// User permission found. + /// Access forbidden. /// Playlist not found. /// /// . /// [HttpGet("{playlistId}/Users/{userId}")] [ProducesResponseType(StatusCodes.Status200OK)] + [ProducesResponseType(StatusCodes.Status403Forbidden)] [ProducesResponseType(StatusCodes.Status404NotFound)] public ActionResult GetPlaylistUser( [FromRoute, Required] Guid playlistId, @@ -209,7 +211,12 @@ public class PlaylistsController : BaseJellyfinApiController || playlist.Shares.Any(s => s.CanEdit && s.UserId.Equals(callingUserId)) || userId.Equals(callingUserId); - if (isPermitted && userPermission is not null) + if (!isPermitted) + { + return Forbid(); + } + + if (userPermission is not null) { return userPermission; } @@ -218,7 +225,7 @@ public class PlaylistsController : BaseJellyfinApiController } /// - /// Modify a user to a playlist's users. + /// Modify a user of a playlist's users. /// /// The playlist id. /// The user id. @@ -237,7 +244,7 @@ public class PlaylistsController : BaseJellyfinApiController public async Task UpdatePlaylistUser( [FromRoute, Required] Guid playlistId, [FromRoute, Required] Guid userId, - [FromBody(EmptyBodyBehavior = EmptyBodyBehavior.Allow)] UpdatePlaylistUserDto updatePlaylistUserRequest) + [FromBody(EmptyBodyBehavior = EmptyBodyBehavior.Allow), Required] UpdatePlaylistUserDto updatePlaylistUserRequest) { var callingUserId = User.GetUserId(); @@ -265,7 +272,7 @@ public class PlaylistsController : BaseJellyfinApiController } /// - /// Remove a user from a playlist's shares. + /// Remove a user from a playlist's users. /// /// The playlist id. /// The user id. diff --git a/Jellyfin.Api/Models/PlaylistDtos/CreatePlaylistDto.cs b/Jellyfin.Api/Models/PlaylistDtos/CreatePlaylistDto.cs index 69694a769..3cbdd031a 100644 --- a/Jellyfin.Api/Models/PlaylistDtos/CreatePlaylistDto.cs +++ b/Jellyfin.Api/Models/PlaylistDtos/CreatePlaylistDto.cs @@ -41,5 +41,5 @@ public class CreatePlaylistDto /// /// Gets or sets a value indicating whether the playlist is public. /// - public bool Public { get; set; } = true; + public bool IsPublic { get; set; } = true; } diff --git a/Jellyfin.Api/Models/PlaylistDtos/UpdatePlaylistDto.cs b/Jellyfin.Api/Models/PlaylistDtos/UpdatePlaylistDto.cs index 0e109db3e..80e20995c 100644 --- a/Jellyfin.Api/Models/PlaylistDtos/UpdatePlaylistDto.cs +++ b/Jellyfin.Api/Models/PlaylistDtos/UpdatePlaylistDto.cs @@ -30,5 +30,5 @@ public class UpdatePlaylistDto /// /// Gets or sets a value indicating whether the playlist is public. /// - public bool? Public { get; set; } + public bool? IsPublic { get; set; } } From 9031aae6531f86bdf2857badb94308c8a1e82b47 Mon Sep 17 00:00:00 2001 From: Shadowghost Date: Wed, 3 Apr 2024 21:24:51 +0200 Subject: [PATCH 16/17] Typo --- Jellyfin.Api/Controllers/PlaylistsController.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Jellyfin.Api/Controllers/PlaylistsController.cs b/Jellyfin.Api/Controllers/PlaylistsController.cs index 69abe5f7e..1100f85cf 100644 --- a/Jellyfin.Api/Controllers/PlaylistsController.cs +++ b/Jellyfin.Api/Controllers/PlaylistsController.cs @@ -185,7 +185,7 @@ public class PlaylistsController : BaseJellyfinApiController /// The playlist id. /// The user id. /// User permission found. - /// Access forbidden. + /// Access forbidden. /// Playlist not found. /// /// . From ddda30fe23a04e07401bc870ac33213ff8a34c71 Mon Sep 17 00:00:00 2001 From: Shadowghost Date: Fri, 5 Apr 2024 21:11:09 +0200 Subject: [PATCH 17/17] Only allow owner and admin to delete playlists --- MediaBrowser.Controller/Entities/BaseItem.cs | 2 +- MediaBrowser.Controller/Playlists/Playlist.cs | 5 +++++ 2 files changed, 6 insertions(+), 1 deletion(-) diff --git a/MediaBrowser.Controller/Entities/BaseItem.cs b/MediaBrowser.Controller/Entities/BaseItem.cs index ac9698ec9..5f9840b1b 100644 --- a/MediaBrowser.Controller/Entities/BaseItem.cs +++ b/MediaBrowser.Controller/Entities/BaseItem.cs @@ -833,7 +833,7 @@ namespace MediaBrowser.Controller.Entities return CanDelete() && IsAuthorizedToDelete(user, allCollectionFolders); } - public bool CanDelete(User user) + public virtual bool CanDelete(User user) { var allCollectionFolders = LibraryManager.GetUserRootFolder().Children.OfType().ToList(); diff --git a/MediaBrowser.Controller/Playlists/Playlist.cs b/MediaBrowser.Controller/Playlists/Playlist.cs index 747dd9f63..34b34e578 100644 --- a/MediaBrowser.Controller/Playlists/Playlist.cs +++ b/MediaBrowser.Controller/Playlists/Playlist.cs @@ -255,6 +255,11 @@ namespace MediaBrowser.Controller.Playlists return shares.Any(s => s.UserId.Equals(userId)); } + public override bool CanDelete(User user) + { + return user.HasPermission(PermissionKind.IsAdministrator) || user.Id.Equals(OwnerUserId); + } + public override bool IsVisibleStandalone(User user) { if (!IsSharedItem)