Merge pull request #14 from Franciskid/fix/season-share-leaks-whole-series

Stop a shared season handing over the rest of the series

Jellyfin.Plugin.ShareLinks/Services/ItemTagService.cs

@@ -77,10 +77,10 @@ public sealed class ItemTagService
     }
 
     /// <summary>
-    /// Ensures the supplied tag is present on the item and, for a season or folder,
+    /// Ensures the supplied tag is present on the item and, when the item is a folder
-    /// on the item's related tree (parent series for a season; all recursive
+    /// such as a series or a season, on everything underneath it, so a guest can
-    /// children for a folder such as a series or season) so a guest can browse the
+    /// browse down through the shared branch instead of only seeing the single node
-    /// whole shared branch instead of only the single node the link was created on.
+    /// the link was created on.
     /// </summary>
     public async Task<bool> EnsureTagTreeAsync(BaseItem item, string tag, CancellationToken cancellationToken)
     {
@@ -107,8 +107,8 @@ public sealed class ItemTagService
     }
 
     /// <summary>
-    /// Removes the supplied tag from the item and, for a season or folder, from the
+    /// Removes the supplied tag from the item and, when it is a folder, from
-    /// item's related tree (mirrors <see cref="EnsureTagTreeAsync"/>).
+    /// everything underneath it (mirrors <see cref="EnsureTagTreeAsync"/>).
     /// </summary>
     public async Task<bool> RemoveTagTreeAsync(BaseItem item, string tag, CancellationToken cancellationToken)
     {
@@ -117,7 +117,7 @@ public sealed class ItemTagService
             throw new ArgumentNullException(nameof(item));
         }
 
-        var targets = BuildTagTreeTargets(item);
+        var targets = BuildTagRemovalTargets(item);
         var changed = false;
         foreach (var target in targets)
         {
@@ -134,10 +134,38 @@ public sealed class ItemTagService
         return changed;
     }
 
+    /// <summary>
+    /// The items that carry a share's tag: the shared item itself and, when it is a
+    /// folder, everything under it.
+    /// </summary>
+    /// <remarks>
+    /// Never the parents. Jellyfin's <c>GetInheritedTags</c> is an item's own tags
+    /// plus every ancestor's, and the guest's AllowedTags policy is matched against
+    /// that, so tagging a season's parent series would hand the guest every other
+    /// season of that series as well.
+    /// </remarks>
     private static List<BaseItem> BuildTagTreeTargets(BaseItem item)
     {
         var targets = new List<BaseItem> { item };
 
+        if (item is Folder folder)
+        {
+            targets.AddRange(folder.GetRecursiveChildren());
+        }
+
+        return targets;
+    }
+
+    /// <summary>
+    /// Removal deliberately reaches one level further than tagging does: up to a
+    /// season's parent series. Builds before this fix put the share's tag on that
+    /// series, and taking a tag off can only ever remove access, so cleaning those
+    /// up is safe where applying them was not.
+    /// </summary>
+    private static List<BaseItem> BuildTagRemovalTargets(BaseItem item)
+    {
+        var targets = BuildTagTreeTargets(item);
+
         if (item is Season season)
         {
             var series = season.Series ?? season.GetParent() as Series;
@@ -147,11 +175,6 @@ public sealed class ItemTagService
             }
         }
 
-        if (item is Folder folder)
-        {
-            targets.AddRange(folder.GetRecursiveChildren());
-        }
-
         return targets;
     }
 

README.md

@@ -37,12 +37,15 @@ real user or handing over a login that sees everything.
    watch at the same time, ten by default, and the eleventh is asked to try again
    later rather than displacing anyone.
 2. Behind the scenes the plugin tags the shared item with a unique, random tag
-   and records the share. Share a series or a season and the tag is applied to
+   and records the share. Share a series or a season and the tag goes on
-   the whole tree underneath it too - series, seasons and episodes - so the
+   everything underneath it as well, so the guest can browse down through what
-   guest can actually browse from the series page down into a season and an
+   you shared rather than seeing a single locked node. The tag never goes on
-   episode, not just see a single locked node. Lookups only ever go through a
+   anything above it: Jellyfin treats a parent's tags as belonging to all of its
-   keyed HMAC hash of the token, and the link itself is dropped from the record
+   children, so tagging the series that a shared season sits in would hand over
-   once it is revoked or expired.
+   every other season too. Share one season and that is all the guest gets, and
+   the series page is not theirs to open. Lookups only ever go through a keyed
+   HMAC hash of the token, and the link itself is dropped from the record once
+   it is revoked or expired.
 3. Whoever opens the link gets a throwaway guest user created on the spot,
    restricted by that tag to the shared item and its tree, and is signed in
    automatically. They land on the title's page.
@@ -63,7 +66,10 @@ server, not only in the browser:
 - On top of that, the web client is locked down for the guest: the home,
   menu and search buttons are hidden, in-page links (cast, studio, genres) are
   made inert, any attempt to navigate somewhere outside the shared tree snaps back
-  to the shared title.
+  to the shared title. Navigating down within what you shared works normally: a
+  shared series opens into its seasons and episodes, a shared season into its
+  episodes. Going up does not, so a guest sent one season cannot reach the series
+  it belongs to.
 
 Playback works normally, including transcoding and remuxing if you allow it, and
 the player's back button still returns them to the title's page.