Block plugin routes for share guests (#16)

* block plugin routes for share guests

Adds a global MVC filter that refuses share-guest accounts on any plugin
controller. Jellyfin's own API stays open, the share tag policy already
bounds it and playback needs it.

Guests are identified by the auth provider marker on the account, so this
covers a leaked token used from curl or a native client, not just the web
client where the CSS lockdown runs.

* add plugin exception list and honest wording

Config page lists installed plugins with a checkbox each, for the ones that
need to answer guests during playback. Default is unticked.

Renames the hidden selectors setting to say it is cosmetic, and stops the
readme implying the web client lockdown confines anything.

* bump to 1.0.4.0

Jellyfin.Plugin.ShareLinks/Api/ShareLinksController.cs

@@ -11,6 +11,7 @@ using Jellyfin.Plugin.ShareLinks.Configuration;
 using Jellyfin.Plugin.ShareLinks.Models;
 using Jellyfin.Plugin.ShareLinks.Services;
 using Jellyfin.Plugin.ShareLinks.Storage;
+using MediaBrowser.Common.Plugins;
 using MediaBrowser.Controller.Entities;
 using MediaBrowser.Controller.Entities.Movies;
 using MediaBrowser.Controller.Entities.TV;
@@ -99,6 +100,19 @@ public sealed class ShareLinkGuestStateDto
     public string? HiddenSelectors { get; set; }
 }
 
+/// <summary>An installed plugin, as offered in the guard's exception list.</summary>
+public sealed class ShareLinkPluginDto
+{
+    /// <summary>Gets or sets the plugin id.</summary>
+    public Guid Id { get; set; }
+
+    /// <summary>Gets or sets the plugin's display name.</summary>
+    public string Name { get; set; } = string.Empty;
+
+    /// <summary>Gets or sets a value indicating whether guests may currently reach it.</summary>
+    public bool AllowedForGuests { get; set; }
+}
+
 /// <summary>ShareLinks API surface.</summary>
 [ApiController]
 [Route("ShareLinks")]
@@ -109,6 +123,7 @@ public sealed class ShareLinksController : ControllerBase
     private readonly ShareLinkCleanupService _cleanupService;
     private readonly ShareLinkRedemptionService _redemptionService;
     private readonly ShareLinkStore _store;
+    private readonly IPluginManager _pluginManager;
     private readonly ILogger<ShareLinksController> _logger;
 
     /// <summary>Initializes a new instance of the <see cref="ShareLinksController"/> class.</summary>
@@ -118,6 +133,7 @@ public sealed class ShareLinksController : ControllerBase
         ShareLinkCleanupService cleanupService,
         ShareLinkRedemptionService redemptionService,
         ShareLinkStore store,
+        IPluginManager pluginManager,
         ILogger<ShareLinksController> logger)
     {
         _libraryManager = libraryManager;
@@ -125,6 +141,7 @@ public sealed class ShareLinksController : ControllerBase
         _cleanupService = cleanupService;
         _redemptionService = redemptionService;
         _store = store;
+        _pluginManager = pluginManager;
         _logger = logger;
     }
 
@@ -284,6 +301,38 @@ public sealed class ShareLinksController : ControllerBase
         return Ok(new { removed });
     }
 
+    /// <summary>
+    /// Lists the installed plugins so the config page can offer them as guard
+    /// exceptions. ShareLinks itself is left out: it is always reachable, since the
+    /// guest's browser fetches the lockdown script and guest state from it.
+    /// </summary>
+    [HttpGet("Admin/Plugins")]
+    [Authorize(AuthenticationSchemes = "CustomAuthentication")]
+    public ActionResult<IEnumerable<ShareLinkPluginDto>> Plugins()
+    {
+        SetNoStoreHeaders();
+        if (!User.IsInRole("Administrator"))
+        {
+            return Forbid();
+        }
+
+        var allowed = Config.GuestAllowedPluginIds ?? Array.Empty<string>();
+        var ownId = Plugin.Instance?.Id;
+
+        var plugins = _pluginManager.Plugins
+            .Where(plugin => !ownId.HasValue || plugin.Id != ownId.Value)
+            .Select(plugin => new ShareLinkPluginDto
+            {
+                Id = plugin.Id,
+                Name = plugin.Name,
+                AllowedForGuests = allowed.Any(value => Guid.TryParse(value, out var parsed) && parsed == plugin.Id)
+            })
+            .OrderBy(plugin => plugin.Name, StringComparer.OrdinalIgnoreCase)
+            .ToArray();
+
+        return Ok(plugins);
+    }
+
     /// <summary>Returns the guest session state for the current authenticated user.</summary>
     [HttpGet("GuestState")]
     [Authorize(AuthenticationSchemes = "CustomAuthentication")]

Jellyfin.Plugin.ShareLinks/Configuration/PluginConfiguration.cs

@@ -1,3 +1,4 @@
+using System;
 using MediaBrowser.Model.Plugins;
 
 namespace Jellyfin.Plugin.ShareLinks.Configuration;
@@ -52,6 +53,26 @@ public class PluginConfiguration : BasePluginConfiguration
     /// Gets or sets a comma-separated list of CSS selectors that are hidden from guest
     /// sessions in the web client. Used to suppress other plugins' injected UI (search
     /// bars, floating buttons) so a guest only sees the shared title. Empty by default.
+    ///
+    /// This is cosmetic only. It runs in the browser, so it tidies the guest's view but
+    /// enforces nothing: the access boundary is the share tag policy plus
+    /// <see cref="GuestPluginApiGuardEnabled"/>.
     /// </summary>
     public string GuestHiddenSelectors { get; set; } = string.Empty;
+
+    /// <summary>
+    /// Gets or sets a value indicating whether share guests are refused access to other
+    /// plugins' API endpoints. Jellyfin's own API stays reachable, since the share tag
+    /// policy already bounds it and playback depends on it. On by default: a guest holds
+    /// a real access token, so without this any installed plugin answers them directly.
+    /// </summary>
+    public bool GuestPluginApiGuardEnabled { get; set; } = true;
+
+    /// <summary>
+    /// Gets or sets the plugin ids that guests may reach despite the guard. Empty by
+    /// default, so a newly installed plugin is refused without anyone having to
+    /// remember it. Opt a plugin in when it genuinely needs to serve guests: an
+    /// intro-skip plugin, for instance, is called by the client mid-playback.
+    /// </summary>
+    public string[] GuestAllowedPluginIds { get; set; } = Array.Empty<string>();
 }

Jellyfin.Plugin.ShareLinks/Jellyfin.Plugin.ShareLinks.csproj

@@ -6,9 +6,9 @@
     <LangVersion>latest</LangVersion>
     <RootNamespace>Jellyfin.Plugin.ShareLinks</RootNamespace>
     <AssemblyName>Jellyfin.Plugin.ShareLinks</AssemblyName>
-    <Version>1.0.3.0</Version>
-    <AssemblyVersion>1.0.3.0</AssemblyVersion>
-    <FileVersion>1.0.3.0</FileVersion>
+    <Version>1.0.4.0</Version>
+    <AssemblyVersion>1.0.4.0</AssemblyVersion>
+    <FileVersion>1.0.4.0</FileVersion>
     <GenerateAssemblyInfo>true</GenerateAssemblyInfo>
     <TreatWarningsAsErrors>false</TreatWarningsAsErrors>
     <ImplicitUsings>disable</ImplicitUsings>

Jellyfin.Plugin.ShareLinks/PluginServiceRegistrator.cs

@@ -1,10 +1,12 @@
 using Jellyfin.Plugin.ShareLinks.Lifecycle;
+using Jellyfin.Plugin.ShareLinks.Security;
 using Jellyfin.Plugin.ShareLinks.Services;
 using Jellyfin.Plugin.ShareLinks.Storage;
 using Jellyfin.Plugin.ShareLinks.Web;
 using MediaBrowser.Controller;
 using MediaBrowser.Controller.Authentication;
 using MediaBrowser.Controller.Plugins;
+using Microsoft.AspNetCore.Mvc;
 using Microsoft.Extensions.DependencyInjection;
 
 namespace Jellyfin.Plugin.ShareLinks;
@@ -20,6 +22,12 @@ public class PluginServiceRegistrator : IPluginServiceRegistrator
     {
         _ = applicationHost;
 
+        // Registered as a global MVC filter so it sees every plugin's controllers,
+        // not just this one's. Options are materialised after all plugin registrators
+        // have run, so adding to the collection here is in time.
+        serviceCollection.AddSingleton<GuestPluginApiGuard>();
+        serviceCollection.Configure<MvcOptions>(options => options.Filters.AddService<GuestPluginApiGuard>());
+
         serviceCollection.AddHostedService<WebInjectionHostedService>();
         serviceCollection.AddSingleton<ShareLinkStore>();
         serviceCollection.AddSingleton<ShareTokenService>();

Jellyfin.Plugin.ShareLinks/Security/GuestPluginApiGuard.cs

@@ -0,0 +1,178 @@
+using System;
+using System.Collections.Generic;
+using System.Linq;
+using System.Reflection;
+using System.Security.Claims;
+using System.Threading.Tasks;
+using Jellyfin.Plugin.ShareLinks.Services;
+using MediaBrowser.Common.Plugins;
+using MediaBrowser.Controller.Library;
+using Microsoft.AspNetCore.Mvc;
+using Microsoft.AspNetCore.Mvc.Controllers;
+using Microsoft.AspNetCore.Mvc.Filters;
+using Microsoft.Extensions.Logging;
+
+namespace Jellyfin.Plugin.ShareLinks.Security;
+
+/// <summary>
+/// Refuses share-guest accounts access to any plugin's API surface.
+///
+/// The web-client lockdown in sharelinks.js can only hide things from a browser
+/// that chooses to run it. A guest holds a real Jellyfin access token, so curl or
+/// a native client sees everything the CSS was hiding. This filter is the part
+/// that actually holds: it runs server side on every MVC action, so the caller's
+/// choice of client is irrelevant.
+///
+/// The rule is structural rather than a curated route list. Jellyfin's own API
+/// lives in one assembly and is already bounded for guests by the share tag
+/// policy, so it is allowed wholesale. Everything else is by definition a
+/// plugin's controller and is refused unless the admin opted that plugin in.
+/// A plugin installed next month is therefore covered on the day it lands.
+/// </summary>
+public sealed class GuestPluginApiGuard : IAsyncActionFilter
+{
+    private const string CoreApiAssemblyName = "Jellyfin.Api";
+
+    private static readonly Assembly OwnAssembly = typeof(GuestPluginApiGuard).Assembly;
+
+    private readonly IUserManager _userManager;
+    private readonly IPluginManager _pluginManager;
+    private readonly ILogger<GuestPluginApiGuard> _logger;
+
+    /// <summary>Initializes a new instance of the <see cref="GuestPluginApiGuard"/> class.</summary>
+    public GuestPluginApiGuard(
+        IUserManager userManager,
+        IPluginManager pluginManager,
+        ILogger<GuestPluginApiGuard> logger)
+    {
+        _userManager = userManager;
+        _pluginManager = pluginManager;
+        _logger = logger;
+    }
+
+    /// <inheritdoc />
+    public async Task OnActionExecutionAsync(ActionExecutingContext context, ActionExecutionDelegate next)
+    {
+        if (context is null)
+        {
+            throw new ArgumentNullException(nameof(context));
+        }
+
+        if (next is null)
+        {
+            throw new ArgumentNullException(nameof(next));
+        }
+
+        if (IsBlocked(context))
+        {
+            context.Result = new StatusCodeResult(403);
+            return;
+        }
+
+        await next().ConfigureAwait(false);
+    }
+
+    /// <summary>Maps a controller assembly back to the plugin that shipped it.</summary>
+    public Guid? FindOwningPluginId(Assembly assembly)
+    {
+        foreach (var plugin in _pluginManager.Plugins)
+        {
+            var instanceAssembly = plugin.Instance?.GetType().Assembly;
+            if (instanceAssembly is not null && instanceAssembly == assembly)
+            {
+                return plugin.Id;
+            }
+        }
+
+        return null;
+    }
+
+    private bool IsBlocked(ActionExecutingContext context)
+    {
+        var config = Plugin.Instance?.Configuration;
+        if (config is null || !config.Enabled || !config.GuestPluginApiGuardEnabled)
+        {
+            return false;
+        }
+
+        // Only controller actions carry an assembly we can reason about. Anything
+        // else (Razor pages, raw endpoints) is left alone rather than guessed at.
+        if (context.ActionDescriptor is not ControllerActionDescriptor descriptor)
+        {
+            return false;
+        }
+
+        var assembly = descriptor.ControllerTypeInfo.Assembly;
+
+        // Jellyfin's own API: the share tag policy is the boundary here, and it is
+        // the same boundary playback depends on. Blocking any of it would break the
+        // guest's ability to watch what they were sent.
+        if (string.Equals(assembly.GetName().Name, CoreApiAssemblyName, StringComparison.Ordinal))
+        {
+            return false;
+        }
+
+        // ShareLinks' own routes must stay reachable: the guest's browser fetches
+        // the lockdown script and its guest state from here. The admin routes on
+        // this controller do their own Administrator check and already refuse a guest.
+        if (assembly == OwnAssembly)
+        {
+            return false;
+        }
+
+        var userId = GetUserId(context.HttpContext.User);
+        if (userId == Guid.Empty)
+        {
+            return false;
+        }
+
+        var user = _userManager.GetUserById(userId);
+        if (user is null)
+        {
+            return false;
+        }
+
+        // The marker is written onto the account by JellyfinGuestUserService, and a
+        // guest cannot clear it: changing a policy needs admin, which they are not.
+        if (!string.Equals(user.AuthenticationProviderId, GuestAuthenticationProvider.ProviderId, StringComparison.Ordinal))
+        {
+            return false;
+        }
+
+        var pluginId = FindOwningPluginId(assembly);
+        if (pluginId.HasValue && IsAllowedPlugin(config.GuestAllowedPluginIds, pluginId.Value))
+        {
+            return false;
+        }
+
+        _logger.LogInformation(
+            "ShareLinks: refused guest {UserName} access to {Controller}.{Action} ({Assembly}).",
+            user.Username,
+            descriptor.ControllerName,
+            descriptor.ActionName,
+            assembly.GetName().Name);
+        return true;
+    }
+
+    private static bool IsAllowedPlugin(IReadOnlyList<string>? allowed, Guid pluginId)
+    {
+        if (allowed is null || allowed.Count == 0)
+        {
+            return false;
+        }
+
+        return allowed.Any(value => Guid.TryParse(value, out var parsed) && parsed == pluginId);
+    }
+
+    private static Guid GetUserId(ClaimsPrincipal? principal)
+    {
+        if (principal is null)
+        {
+            return Guid.Empty;
+        }
+
+        var claim = principal.FindFirst("Jellyfin-UserId")?.Value
+            ?? principal.FindFirst(ClaimTypes.NameIdentifier)?.Value;
+        return Guid.TryParse(claim, out var id) ? id : Guid.Empty;
+    }
+}

Jellyfin.Plugin.ShareLinks/Web/configPage.html

@@ -130,8 +130,8 @@
                         </div>
 
                         <div class="sl-field inputContainer" style="grid-column: 1 / -1;">
-                            <input is="emby-input" type="text" id="GuestHiddenSelectors" label="Guest hidden selectors (CSS, comma-separated)" />
-                            <div class="fieldDescription">Elements hidden from guest sessions, e.g. other plugins' buttons. Empty by default.</div>
+                            <input is="emby-input" type="text" id="GuestHiddenSelectors" label="Cosmetic: hide elements from guests (CSS, comma-separated)" />
+                            <div class="fieldDescription">Tidies the guest's view only. This runs in the browser and blocks nothing, so do not rely on it to keep a guest out of anything. Use the plugin access section below for that.</div>
                         </div>
 
                         <div class="sl-field inputContainer">
@@ -167,6 +167,21 @@
                     </div>
                 </div>
 
+                <div class="sl-section">
+                    <h3 class="sectionTitle">Plugin access for guests</h3>
+                    <div class="checkboxContainer checkboxContainer-withDescription">
+                        <label>
+                            <input is="emby-checkbox" type="checkbox" id="GuestPluginApiGuardEnabled" />
+                            <span>Block other plugins for guests</span>
+                        </label>
+                        <div class="fieldDescription">Refuses guest accounts on other plugins' API endpoints, on the server. A guest holds a real Jellyfin token, so without this any installed plugin answers them directly, whatever the web client shows. Jellyfin's own API stays available: the share tag already limits it to the shared title, and playback needs it.</div>
+                    </div>
+                    <div class="sl-muted" style="margin-top:0.75rem;">Tick a plugin to let guests reach it anyway. Leave everything unticked unless a plugin needs to serve guests during playback, such as an intro skipper.</div>
+                    <div id="GuestAllowedPlugins" style="margin-top:0.5rem;">
+                        <div class="sl-muted">Loading plugins…</div>
+                    </div>
+                </div>
+
                 <div class="sl-section">
                     <div class="sl-inline">
                         <h3 class="sectionTitle">Share links</h3>
@@ -228,11 +243,57 @@
                     page.querySelector('#OneUseDefault').checked = cfg.OneUseDefault !== false;
                     page.querySelector('#MaxConcurrentViewers').value = cfg.MaxConcurrentViewers === undefined ? 10 : cfg.MaxConcurrentViewers;
                     page.querySelector('#GuestModeLockdownEnabled').checked = cfg.GuestModeLockdownEnabled !== false;
+                    page.querySelector('#GuestPluginApiGuardEnabled').checked = cfg.GuestPluginApiGuardEnabled !== false;
                 }).finally(function () {
                     Dashboard.hideLoadingMsg();
                 });
             }
 
+            // The ticked state comes from the server rather than from the config we
+            // just loaded, so a plugin that has since been uninstalled drops out of
+            // the list instead of lingering as a stale checkbox.
+            function loadPlugins() {
+                var host = page.querySelector('#GuestAllowedPlugins');
+                return ApiClient.ajax({
+                    type: 'GET',
+                    url: ApiClient.getUrl('ShareLinks/Admin/Plugins'),
+                    dataType: 'json'
+                }).then(function (list) {
+                    var items = Array.isArray(list) ? list : [];
+                    if (!items.length) {
+                        host.innerHTML = '<div class="sl-muted">No other plugins installed.</div>';
+                        return;
+                    }
+
+                    host.innerHTML = items.map(function (plugin) {
+                        return '<div class="checkboxContainer">'
+                            + '<label>'
+                            + '<input is="emby-checkbox" type="checkbox" class="sl-plugin-allow" data-plugin-id="'
+                            + escapeHtml(plugin.Id) + '"' + (plugin.AllowedForGuests ? ' checked' : '') + ' />'
+                            + '<span>' + escapeHtml(plugin.Name) + '</span>'
+                            + '</label>'
+                            + '</div>';
+                    }).join('');
+                }).catch(function () {
+                    host.innerHTML = '<div class="sl-muted">Could not load the plugin list.</div>';
+                });
+            }
+
+            function collectAllowedPluginIds() {
+                // A failed plugin load leaves no checkboxes to read. Returning the saved
+                // config untouched in that case avoids silently clearing the exceptions.
+                var boxes = page.querySelectorAll('.sl-plugin-allow');
+                if (!boxes.length) {
+                    return null;
+                }
+
+                return Array.prototype.filter.call(boxes, function (box) {
+                    return box.checked;
+                }).map(function (box) {
+                    return box.getAttribute('data-plugin-id');
+                });
+            }
+
             function fmtDate(value) {
                 if (!value) { return 'n/a'; }
                 return new Date(value).toLocaleString();
@@ -410,6 +471,13 @@
                     cfg.OneUseDefault = page.querySelector('#OneUseDefault').checked;
                     cfg.MaxConcurrentViewers = Math.max(parseInt(page.querySelector('#MaxConcurrentViewers').value, 10) || 0, 0);
                     cfg.GuestModeLockdownEnabled = page.querySelector('#GuestModeLockdownEnabled').checked;
+                    cfg.GuestPluginApiGuardEnabled = page.querySelector('#GuestPluginApiGuardEnabled').checked;
+
+                    var allowedPluginIds = collectAllowedPluginIds();
+                    if (allowedPluginIds !== null) {
+                        cfg.GuestAllowedPluginIds = allowedPluginIds;
+                    }
+
                     ApiClient.updatePluginConfiguration(ShareLinksPluginId, cfg).then(function (result) {
                         Dashboard.processPluginConfigurationUpdateResult(result);
                         return loadLinks();
@@ -422,7 +490,7 @@
 
             document.querySelector('#ShareLinksConfigPage').addEventListener('pageshow', function () {
                 page = this;
-                loadConfig().then(loadLinks);
+                loadConfig().then(loadPlugins).then(loadLinks);
             });
 
             document.querySelector('#ShareLinksConfigForm').addEventListener('submit', save);

Jellyfin.Plugin.ShareLinks/meta.json

@@ -1,7 +1,7 @@
 {
   "guid": "68540b76-ee74-436d-85ff-2abc884bbea6",
   "name": "ShareLinks",
-  "version": "1.0.3.0",
+  "version": "1.0.4.0",
   "targetAbi": "10.11.0.0",
   "framework": "net9.0",
   "owner": "Franciskid",