Merge pull request #9 from Franciskid/security/harden-redemption-and-expiry

Harden redemption, expiry limits, token storage and guest sign-in

Jellyfin.Plugin.ShareLinks/Api/ShareLinksController.cs

@@ -190,10 +190,10 @@ public sealed class ShareLinksController : ControllerBase
             return BadRequest(new { error = "Expiry must be positive." });
         }
 
-        var effectiveMaxExpiryHours = Math.Max(config.MaxExpiryHours, 720);
+        var maxExpiryHours = config.MaxExpiryHours > 0 ? config.MaxExpiryHours : 720;
-        if (expiryHours > effectiveMaxExpiryHours)
+        if (expiryHours > maxExpiryHours)
         {
-            return BadRequest(new { error = $"Expiry exceeds the configured maximum of {effectiveMaxExpiryHours} hours." });
+            return BadRequest(new { error = $"Expiry exceeds the configured maximum of {maxExpiryHours} hours." });
         }
 
         var item = _libraryManager.GetItemById(itemId);

Jellyfin.Plugin.ShareLinks/PluginServiceRegistrator.cs

@@ -3,6 +3,7 @@ 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.Extensions.DependencyInjection;
 
@@ -24,6 +25,7 @@ public class PluginServiceRegistrator : IPluginServiceRegistrator
         serviceCollection.AddSingleton<ShareTokenService>();
         serviceCollection.AddSingleton<ItemTagService>();
         serviceCollection.AddSingleton<JellyfinGuestUserService>();
+        serviceCollection.AddSingleton<IAuthenticationProvider, GuestAuthenticationProvider>();
         serviceCollection.AddSingleton<ShareLinkCreationService>();
         serviceCollection.AddSingleton<ShareLinkRedemptionService>();
         serviceCollection.AddSingleton<ShareLinkCleanupService>();

Jellyfin.Plugin.ShareLinks/Services/GuestAuthenticationProvider.cs

@@ -0,0 +1,53 @@
+using System.Threading.Tasks;
+using Jellyfin.Database.Implementations.Entities;
+using MediaBrowser.Controller.Authentication;
+using Microsoft.Extensions.Logging;
+
+namespace Jellyfin.Plugin.ShareLinks.Services;
+
+/// <summary>
+/// The authentication provider assigned to ShareLinks guest accounts. It refuses
+/// every interactive sign-in, so a guest account cannot be used on the normal
+/// login page even if its name and password were to leak. Guest sessions are
+/// minted server side through <c>ISessionManager.AuthenticateDirect</c>, which
+/// does not enforce a password and so never reaches a provider at all.
+/// </summary>
+public sealed class GuestAuthenticationProvider : IAuthenticationProvider
+{
+    private readonly ILogger<GuestAuthenticationProvider> _logger;
+
+    /// <summary>Initializes a new instance of the <see cref="GuestAuthenticationProvider"/> class.</summary>
+    public GuestAuthenticationProvider(ILogger<GuestAuthenticationProvider> logger)
+    {
+        _logger = logger;
+    }
+
+    /// <summary>
+    /// Gets the value Jellyfin stores on a user to select this provider. Jellyfin
+    /// matches it against the provider's full type name.
+    /// </summary>
+    public static string ProviderId => typeof(GuestAuthenticationProvider).FullName!;
+
+    /// <inheritdoc />
+    public string Name => "ShareLinks guest accounts (blocks sign-in)";
+
+    /// <inheritdoc />
+    public bool IsEnabled => true;
+
+    /// <inheritdoc />
+    public Task<ProviderAuthenticationResult> Authenticate(string username, string password)
+    {
+        _logger.LogWarning("ShareLinks: refused an interactive sign-in attempt for guest account {UserName}.", username);
+        return Task.FromException<ProviderAuthenticationResult>(
+            new AuthenticationException("ShareLinks guest accounts cannot sign in interactively."));
+    }
+
+    /// <summary>
+    /// Reports the account as having a password so nothing offers it as a
+    /// passwordless login.
+    /// </summary>
+    public bool HasPassword(User user) => true;
+
+    /// <inheritdoc />
+    public Task ChangePassword(User user, string newPassword) => Task.CompletedTask;
+}

Jellyfin.Plugin.ShareLinks/Services/ItemTagService.cs

@@ -1,12 +1,12 @@
 using System;
 using System.Collections.Generic;
 using System.Linq;
-using System.Reflection;
 using System.Threading;
 using System.Threading.Tasks;
 using MediaBrowser.Controller.Entities;
 using MediaBrowser.Controller.Entities.TV;
 using MediaBrowser.Controller.Library;
+using MediaBrowser.Model.Entities;
 using Microsoft.Extensions.Logging;
 
 namespace Jellyfin.Plugin.ShareLinks.Services;
@@ -157,46 +157,7 @@ public sealed class ItemTagService
 
     private async Task PersistAsync(BaseItem item, CancellationToken cancellationToken)
     {
-        var method = _libraryManager.GetType()
-            .GetMethods(BindingFlags.Instance | BindingFlags.Public)
-            .FirstOrDefault(candidate =>
-            {
-                if (!string.Equals(candidate.Name, "UpdateItemAsync", StringComparison.Ordinal))
-                {
-                    return false;
-                }
-
-                var parameters = candidate.GetParameters();
-                return parameters.Length == 4
-                    && typeof(BaseItem).IsAssignableFrom(parameters[0].ParameterType)
-                    && typeof(BaseItem).IsAssignableFrom(parameters[1].ParameterType)
-                    && parameters[3].ParameterType == typeof(CancellationToken);
-            });
-
-        if (method is null)
-        {
-            throw new MissingMethodException(_libraryManager.GetType().FullName, "UpdateItemAsync");
-        }
-
-        var parametersInfo = method.GetParameters();
-        var updateReason = parametersInfo[2].ParameterType.IsEnum
-            ? Enum.ToObject(parametersInfo[2].ParameterType, 0)
-            : 0;
-
         var parent = item.DisplayParent ?? item;
-        var task = method.Invoke(_libraryManager, new object?[]
+        await _libraryManager.UpdateItemAsync(item, parent, ItemUpdateType.None, cancellationToken).ConfigureAwait(false);
-        {
-            item,
-            parent,
-            updateReason,
-            cancellationToken
-        }) as Task;
-
-        if (task is null)
-        {
-            throw new InvalidOperationException("UpdateItemAsync did not return a task.");
-        }
-
-        await task.ConfigureAwait(false);
     }
 }

Jellyfin.Plugin.ShareLinks/Services/JellyfinGuestUserService.cs

@@ -1,12 +1,9 @@
 using System;
-using System.Collections.Generic;
-using System.Globalization;
-using System.Linq;
-using System.Reflection;
 using System.Security.Cryptography;
 using System.Threading;
 using System.Threading.Tasks;
-using Jellyfin.Plugin.ShareLinks.Configuration;
+using Jellyfin.Data.Enums;
+using Jellyfin.Database.Implementations.Entities;
 using Jellyfin.Plugin.ShareLinks.Models;
 using MediaBrowser.Controller.Library;
 using MediaBrowser.Model.Users;
@@ -43,7 +40,7 @@ public sealed class JellyfinGuestUserService
     }
 
     /// <summary>Ensures the temporary guest user exists and has the correct policy and password.</summary>
-    public async Task<dynamic> EnsureGuestUserAsync(ShareLinkRecord record, string password, CancellationToken cancellationToken)
+    public async Task<User> EnsureGuestUserAsync(ShareLinkRecord record, string password, CancellationToken cancellationToken)
     {
         if (record is null)
         {
@@ -55,6 +52,8 @@ public sealed class JellyfinGuestUserService
             throw new ArgumentException("Password cannot be empty.", nameof(password));
         }
 
+        cancellationToken.ThrowIfCancellationRequested();
+
         var username = record.GuestUserName;
         if (string.IsNullOrWhiteSpace(username))
         {
@@ -62,42 +61,27 @@ public sealed class JellyfinGuestUserService
             record.GuestUserName = username;
         }
 
-        object? user = _userManager.GetUserByName(username);
+        var user = _userManager.GetUserByName(username);
         if (user is null)
         {
-            user = await InvokeUserManagerAsync<object?>(
+            user = await _userManager.CreateUserAsync(username).ConfigureAwait(false);
-                    "create user",
-                    cancellationToken,
-                    new InvocationCandidate("CreateUserAsync", new object?[] { username }),
-                    new InvocationCandidate("CreateUser", new object?[] { username }))
-                .ConfigureAwait(false) ?? _userManager.GetUserByName(username);
-
             if (user is null)
             {
                 throw new InvalidOperationException($"Unable to create temporary guest user '{username}'.");
             }
         }
 
-        var existingUserId = GetUserId(user);
+        // The password must be set before the policy update: UpdatePolicyAsync bumps the
-        if (existingUserId != Guid.Empty)
+        // user's EF concurrency token server side, and ChangePassword with a stale instance
-        {
+        // then throws DbUpdateConcurrencyException. The password is only a fallback - the
-            user = _userManager.GetUserById(existingUserId) ?? user;
+        // policy hands the account to GuestAuthenticationProvider, which refuses every
-        }
+        // interactive sign-in - but it means the account is never reachable with a blank
+        // password either.
+        await _userManager.ChangePassword(user, password).ConfigureAwait(false);
+        await ApplyPolicyAsync(user, record, disabled: false).ConfigureAwait(false);
 
-        // The password must be changed before the policy update: UpdatePolicyAsync bumps the
+        user = _userManager.GetUserById(user.Id) ?? user;
-        // user's EF concurrency token server-side, and ChangePassword with a stale instance
+        _logger.LogInformation("ShareLinks: ensured guest user {UserName} for record {RecordId}.", user.Username, record.Id);
-        // throws DbUpdateConcurrencyException.
-        await ChangePasswordAsync(user, password, cancellationToken).ConfigureAwait(false);
-        await ApplyPolicyAsync(user, record, disabled: false, cancellationToken).ConfigureAwait(false);
-
-        var userId = GetUserId(user);
-        if (userId == Guid.Empty)
-        {
-            throw new InvalidOperationException("ShareLinks: created guest user did not expose a valid Id.");
-        }
-
-        user = _userManager.GetUserById(userId) ?? user;
-        _logger.LogInformation("ShareLinks: ensured guest user {UserName} for record {RecordId}.", GetUserName(user), record.Id);
         return user;
     }
 
@@ -110,13 +94,14 @@ public sealed class JellyfinGuestUserService
             return;
         }
 
+        cancellationToken.ThrowIfCancellationRequested();
         try
         {
-            await ApplyPolicyAsync(user, record, disabled: true, cancellationToken).ConfigureAwait(false);
+            await ApplyPolicyAsync(user, record, disabled: true).ConfigureAwait(false);
         }
         catch (Exception ex)
         {
-            _logger.LogWarning(ex, "ShareLinks: failed to disable guest user {UserName} for record {RecordId}.", GetUserName(user), record.Id);
+            _logger.LogWarning(ex, "ShareLinks: failed to disable guest user {UserName} for record {RecordId}.", user.Username, record.Id);
         }
     }
 
@@ -129,19 +114,20 @@ public sealed class JellyfinGuestUserService
             return;
         }
 
+        cancellationToken.ThrowIfCancellationRequested();
         try
         {
-            await DeleteUserAsync(user, cancellationToken).ConfigureAwait(false);
+            await _userManager.DeleteUserAsync(user.Id).ConfigureAwait(false);
-            _logger.LogInformation("ShareLinks: deleted guest user {UserName} for record {RecordId}.", GetUserName(user), record.Id);
+            _logger.LogInformation("ShareLinks: deleted guest user {UserName} for record {RecordId}.", user.Username, record.Id);
         }
         catch (Exception ex)
         {
-            _logger.LogWarning(ex, "ShareLinks: failed to delete guest user {UserName} for record {RecordId}.", GetUserName(user), record.Id);
+            _logger.LogWarning(ex, "ShareLinks: failed to delete guest user {UserName} for record {RecordId}.", user.Username, record.Id);
             throw;
         }
     }
 
-    private object? FindRecordUser(ShareLinkRecord record)
+    private User? FindRecordUser(ShareLinkRecord record)
     {
         if (record.GuestUserId.HasValue)
         {
@@ -152,358 +138,62 @@ public sealed class JellyfinGuestUserService
             }
         }
 
-        if (!string.IsNullOrWhiteSpace(record.GuestUserName))
+        return string.IsNullOrWhiteSpace(record.GuestUserName)
-        {
+            ? null
-            return _userManager.GetUserByName(record.GuestUserName);
+            : _userManager.GetUserByName(record.GuestUserName);
     }
 
-        return null;
+    private async Task ApplyPolicyAsync(User user, ShareLinkRecord record, bool disabled)
-    }
-
-    private async Task ApplyPolicyAsync(object user, ShareLinkRecord record, bool disabled, CancellationToken cancellationToken)
     {
         var config = Plugin.Instance!.Configuration;
-        var policy = new UserPolicy();
+        var policy = new UserPolicy
-
-        SetPolicyValue(policy, "AuthenticationProviderId", GetUserValue(user, "AuthenticationProviderId"));
-        SetPolicyValue(policy, "PasswordResetProviderId", GetUserValue(user, "PasswordResetProviderId"));
-        SetPolicyValue(policy, "AllowedTags", string.IsNullOrWhiteSpace(record.AllowedTag) ? Array.Empty<string>() : new[] { record.AllowedTag! });
-        SetPolicyValue(policy, "BlockedTags", Array.Empty<string>());
-        SetPolicyValue(policy, "IsAdministrator", false);
-        SetPolicyValue(policy, "IsHidden", true);
-        SetPolicyValue(policy, "IsDisabled", disabled);
-        SetPolicyValue(policy, "EnableCollectionManagement", false);
-        SetPolicyValue(policy, "EnableSubtitleManagement", false);
-        SetPolicyValue(policy, "EnableLyricManagement", false);
-        SetPolicyValue(policy, "EnableUserPreferenceAccess", false);
-        SetPolicyValue(policy, "EnableSharedDeviceControl", false);
-        SetPolicyValue(policy, "EnableRemoteAccess", true);
-        SetPolicyValue(policy, "EnableRemoteControlOfOtherUsers", false);
-        SetPolicyValue(policy, "EnableLiveTvManagement", false);
-        SetPolicyValue(policy, "EnableLiveTvAccess", false);
-        SetPolicyValue(policy, "EnableMediaPlayback", true);
-        SetPolicyValue(policy, "EnableAudioPlaybackTranscoding", config.AllowTranscoding);
-        SetPolicyValue(policy, "EnableVideoPlaybackTranscoding", config.AllowTranscoding);
-        SetPolicyValue(policy, "EnablePlaybackRemuxing", config.AllowRemuxing);
-        SetPolicyValue(policy, "ForceRemoteSourceTranscoding", false);
-        SetPolicyValue(policy, "EnableContentDeletion", false);
-        SetPolicyValue(policy, "EnableContentDeletionFromFolders", Array.Empty<string>());
-        SetPolicyValue(policy, "EnableContentDownloading", false);
-        SetPolicyValue(policy, "EnableSyncTranscoding", false);
-        SetPolicyValue(policy, "EnableMediaConversion", false);
-        SetPolicyValue(policy, "EnableAllChannels", false);
-        SetPolicyValue(policy, "EnabledChannels", Array.Empty<Guid>());
-        SetPolicyValue(policy, "EnableAllDevices", true);
-        SetPolicyValue(policy, "EnabledDevices", Array.Empty<string>());
-        SetPolicyValue(policy, "EnableAllFolders", true);
-        SetPolicyValue(policy, "EnabledFolders", Array.Empty<Guid>());
-        SetPolicyValue(policy, "EnablePublicSharing", false);
-        SetPolicyValue(policy, "LoginAttemptsBeforeLockout", -1);
-        SetPolicyValue(policy, "MaxActiveSessions", 1);
-        SetPolicyValue(policy, "BlockUnratedItems", Array.Empty<Jellyfin.Data.Enums.UnratedItem>());
-
-        await InvokeUserManagerAsync<object?>(
-                "update policy",
-                cancellationToken,
-                new InvocationCandidate("UpdatePolicyAsync", new object?[] { GetUserId(user), policy }),
-                new InvocationCandidate("UpdatePolicyAsync", new object?[] { user, policy }),
-                new InvocationCandidate("UpdatePolicy", new object?[] { GetUserId(user), policy }),
-                new InvocationCandidate("UpdatePolicy", new object?[] { user, policy }))
-            .ConfigureAwait(false);
-    }
-
-    private async Task ChangePasswordAsync(object user, string password, CancellationToken cancellationToken)
         {
-        await InvokeUserManagerAsync<object?>(
+            // Hand the account to a provider that refuses interactive sign-in. If the
-                "change password",
+            // plugin is ever disabled the id stops resolving and Jellyfin falls back to
-                cancellationToken,
+            // its own InvalidAuthProvider, which also refuses, so this fails closed.
-                new InvocationCandidate("ChangePasswordAsync", new object?[] { user, password }),
+            AuthenticationProviderId = GuestAuthenticationProvider.ProviderId,
-                new InvocationCandidate("ChangePasswordAsync", new object?[] { GetUserId(user), password }),
+            PasswordResetProviderId = user.PasswordResetProviderId,
-                new InvocationCandidate("ChangePasswordAsync", new object?[] { user, string.Empty, password }),
+            AllowedTags = string.IsNullOrWhiteSpace(record.AllowedTag)
-                new InvocationCandidate("ChangePasswordAsync", new object?[] { GetUserId(user), string.Empty, password }),
+                ? Array.Empty<string>()
-                new InvocationCandidate("ChangePassword", new object?[] { user, password }),
+                : new[] { record.AllowedTag! },
-                new InvocationCandidate("ChangePassword", new object?[] { GetUserId(user), password }),
+            BlockedTags = Array.Empty<string>(),
-                new InvocationCandidate("ChangePassword", new object?[] { user, string.Empty, password }),
+            IsAdministrator = false,
-                new InvocationCandidate("ChangePassword", new object?[] { GetUserId(user), string.Empty, password }))
+            IsHidden = true,
-            .ConfigureAwait(false);
+            IsDisabled = disabled,
-    }
+            EnableCollectionManagement = false,
+            EnableSubtitleManagement = false,
+            EnableLyricManagement = false,
+            EnableUserPreferenceAccess = false,
+            EnableSharedDeviceControl = false,
+            EnableRemoteAccess = true,
+            EnableRemoteControlOfOtherUsers = false,
+            EnableLiveTvManagement = false,
+            EnableLiveTvAccess = false,
+            EnableMediaPlayback = true,
+            EnableAudioPlaybackTranscoding = config.AllowTranscoding,
+            EnableVideoPlaybackTranscoding = config.AllowTranscoding,
+            EnablePlaybackRemuxing = config.AllowRemuxing,
+            ForceRemoteSourceTranscoding = false,
+            EnableContentDeletion = false,
+            EnableContentDeletionFromFolders = Array.Empty<string>(),
+            EnableContentDownloading = false,
+            EnableSyncTranscoding = false,
+            EnableMediaConversion = false,
+            EnableAllChannels = false,
+            EnabledChannels = Array.Empty<Guid>(),
+            EnableAllDevices = true,
+            EnabledDevices = Array.Empty<string>(),
+            EnableAllFolders = true,
+            EnabledFolders = Array.Empty<Guid>(),
+            EnablePublicSharing = false,
+            LoginAttemptsBeforeLockout = -1,
+            MaxActiveSessions = 1,
+            BlockUnratedItems = Array.Empty<UnratedItem>()
+        };
 
-    private async Task DeleteUserAsync(object user, CancellationToken cancellationToken)
+        await _userManager.UpdatePolicyAsync(user.Id, policy).ConfigureAwait(false);
-    {
-        await InvokeUserManagerAsync<object?>(
-                "delete user",
-                cancellationToken,
-                new InvocationCandidate("DeleteUserAsync", new object?[] { GetUserId(user) }),
-                new InvocationCandidate("DeleteUserAsync", new object?[] { user }),
-                new InvocationCandidate("DeleteUser", new object?[] { GetUserId(user) }),
-                new InvocationCandidate("DeleteUser", new object?[] { user }))
-            .ConfigureAwait(false);
     }
 
-    private async Task<T?> InvokeUserManagerAsync<T>(
-        string operationName,
-        CancellationToken cancellationToken,
-        params InvocationCandidate[] candidates)
-    {
-        var managerType = _userManager.GetType();
-        var triedVariants = new List<string>();
-
-        foreach (var candidate in candidates)
-        {
-            var methods = managerType.GetMethods(BindingFlags.Instance | BindingFlags.Public)
-                .Where(method => string.Equals(method.Name, candidate.MethodName, StringComparison.Ordinal));
-
-            var matchedMethod = false;
-            foreach (var method in methods)
-            {
-                if (!TryBindArguments(method, candidate.Arguments, cancellationToken, out var invocationArguments))
-                {
-                    continue;
-                }
-
-                matchedMethod = true;
-                var invocation = method.Invoke(_userManager, invocationArguments);
-                if (invocation is Task task)
-                {
-                    await task.WaitAsync(cancellationToken).ConfigureAwait(false);
-
-                    var resultProperty = invocation.GetType().GetProperty("Result", BindingFlags.Instance | BindingFlags.Public);
-                    if (resultProperty is null)
-                    {
-                        return default;
-                    }
-
-                    var result = resultProperty.GetValue(invocation);
-                    if (result is null)
-                    {
-                        return default;
-                    }
-
-                    if (result is T typedResult)
-                    {
-                        return typedResult;
-                    }
-
-                    throw new InvalidOperationException(
-                        $"ShareLinks: {managerType.FullName}.{candidate.MethodName} returned incompatible result type {result.GetType().FullName} for {operationName}.");
-                }
-
-                if (invocation is T directResult)
-                {
-                    return directResult;
-                }
-
-                if (invocation is null)
-                {
-                    return default;
-                }
-
-                throw new InvalidOperationException(
-                    $"ShareLinks: {managerType.FullName}.{candidate.MethodName} returned incompatible result type {invocation.GetType().FullName} for {operationName}.");
-            }
-
-            if (!matchedMethod)
-            {
-                triedVariants.Add($"{candidate.MethodName}({DescribeArguments(candidate.Arguments)})");
-            }
-        }
-
-        _logger.LogWarning(
-            "ShareLinks: {UserManagerType} does not expose a compatible {Operation} variant. Tried {Variants}.",
-            managerType.FullName,
-            operationName,
-            string.Join("; ", triedVariants));
-        throw new MissingMethodException(managerType.FullName, operationName);
-    }
-
-    private static string DescribeArguments(object?[] arguments)
-    {
-        return string.Join(", ", arguments.Select(argument => argument?.GetType().Name ?? "null"));
-    }
-
-    private static bool TryBindArguments(MethodInfo method, object?[] suppliedArguments, CancellationToken cancellationToken, out object?[] invocationArguments)
-    {
-        var parameters = method.GetParameters();
-        if (suppliedArguments.Length > parameters.Length)
-        {
-            invocationArguments = Array.Empty<object?>();
-            return false;
-        }
-
-        invocationArguments = new object?[parameters.Length];
-        for (var index = 0; index < suppliedArguments.Length; index++)
-        {
-            if (!TryConvertValue(parameters[index].ParameterType, suppliedArguments[index], out var convertedArgument))
-            {
-                invocationArguments = Array.Empty<object?>();
-                return false;
-            }
-
-            invocationArguments[index] = convertedArgument;
-        }
-
-        for (var index = suppliedArguments.Length; index < parameters.Length; index++)
-        {
-            var parameter = parameters[index];
-            if (parameter.ParameterType == typeof(CancellationToken))
-            {
-                invocationArguments[index] = cancellationToken;
-                continue;
-            }
-
-            if (parameter.IsOptional)
-            {
-                invocationArguments[index] = GetOptionalParameterValue(parameter);
-                continue;
-            }
-
-            invocationArguments = Array.Empty<object?>();
-            return false;
-        }
-
-        return true;
-    }
-
-    private static object? GetOptionalParameterValue(ParameterInfo parameter)
-    {
-        var defaultValue = parameter.DefaultValue;
-        if (defaultValue is not null && defaultValue != DBNull.Value && defaultValue != Type.Missing)
-        {
-            return defaultValue;
-        }
-
-        return parameter.ParameterType.IsValueType
-            ? Activator.CreateInstance(parameter.ParameterType)
-            : null;
-    }
-
-    private static void SetPolicyValue(UserPolicy policy, string memberName, object? value)
-    {
-        var policyType = policy.GetType();
-
-        var property = policyType.GetProperty(memberName, BindingFlags.Instance | BindingFlags.Public);
-        if (property is not null && property.CanWrite && TryConvertValue(property.PropertyType, value, out var convertedPropertyValue))
-        {
-            property.SetValue(policy, convertedPropertyValue);
-            return;
-        }
-
-        var field = policyType.GetField(memberName, BindingFlags.Instance | BindingFlags.Public);
-        if (field is not null && TryConvertValue(field.FieldType, value, out var convertedFieldValue))
-        {
-            field.SetValue(policy, convertedFieldValue);
-        }
-    }
-
-    private static Guid GetUserId(object user)
-    {
-        var value = GetUserValue(user, "Id");
-        if (value is Guid guid)
-        {
-            return guid;
-        }
-
-        if (value is string text && Guid.TryParse(text, out var parsedGuid))
-        {
-            return parsedGuid;
-        }
-
-        return Guid.Empty;
-    }
-
-    private static string GetUserName(object user)
-    {
-        var value = GetUserValue(user, "Username");
-        if (value is string username && !string.IsNullOrWhiteSpace(username))
-        {
-            return username;
-        }
-
-        value = GetUserValue(user, "Name");
-        return value as string ?? string.Empty;
-    }
-
-    private static object? GetUserValue(object user, string propertyName)
-    {
-        var property = user.GetType().GetProperty(propertyName, BindingFlags.Instance | BindingFlags.Public);
-        return property?.GetValue(user);
-    }
-
-    private static bool TryConvertValue(Type targetType, object? value, out object? converted)
-    {
-        if (targetType.IsByRef)
-        {
-            targetType = targetType.GetElementType() ?? targetType;
-        }
-
-        var nonNullableType = Nullable.GetUnderlyingType(targetType) ?? targetType;
-        if (value is null)
-        {
-            converted = null;
-            return !nonNullableType.IsValueType || Nullable.GetUnderlyingType(targetType) is not null;
-        }
-
-        if (nonNullableType.IsInstanceOfType(value) || targetType.IsAssignableFrom(value.GetType()))
-        {
-            converted = value;
-            return true;
-        }
-
-        if (nonNullableType.IsEnum)
-        {
-            if (value is string text)
-            {
-                converted = Enum.Parse(nonNullableType, text, ignoreCase: true);
-                return true;
-            }
-
-            if (IsNumeric(value))
-            {
-                converted = Enum.ToObject(nonNullableType, value);
-                return true;
-            }
-        }
-
-        if (nonNullableType == typeof(Guid) && value is string guidText && Guid.TryParse(guidText, out var guid))
-        {
-            converted = guid;
-            return true;
-        }
-
-        if (value is IConvertible)
-        {
-            try
-            {
-                converted = Convert.ChangeType(value, nonNullableType, CultureInfo.InvariantCulture);
-                return true;
-            }
-            catch
-            {
-                // The caller will ignore the missing or incompatible policy member.
-            }
-        }
-
-        converted = null;
-        return false;
-    }
-
-    private static bool IsNumeric(object value)
-    {
-        return value is byte
-            or sbyte
-            or short
-            or ushort
-            or int
-            or uint
-            or long
-            or ulong
-            or float
-            or double
-            or decimal;
-    }
-
-    private sealed record InvocationCandidate(string MethodName, object?[] Arguments);
-
     private static string Base64UrlEncode(ReadOnlySpan<byte> bytes)
     {
         return Convert.ToBase64String(bytes)

Jellyfin.Plugin.ShareLinks/Services/ShareLinkCleanupService.cs

@@ -100,6 +100,12 @@ public sealed class ShareLinkCleanupService : IShareLinkCleanupService
             return record;
         }
 
+        // The share URL carries the raw token, and it is kept on the record only so
+        // the dashboard can offer "copy" while the link is still usable. Once the
+        // link is torn down the token is dead weight, so drop it rather than leave
+        // it sitting in the store for good.
+        record.ShareUrl = null;
+
         var errors = new List<string>();
         try
         {

Jellyfin.Plugin.ShareLinks/Services/ShareLinkRedemptionService.cs

@@ -24,6 +24,7 @@ public sealed class ShareLinkRedemptionService
     private readonly ShareLinkCleanupService _cleanupService;
     private readonly ISessionManager _sessionManager;
     private readonly ILogger<ShareLinkRedemptionService> _logger;
+    private readonly SemaphoreSlim _redeemGate = new(1, 1);
 
     /// <summary>Initializes a new instance of the <see cref="ShareLinkRedemptionService"/> class.</summary>
     public ShareLinkRedemptionService(
@@ -48,6 +49,23 @@ public sealed class ShareLinkRedemptionService
 
     /// <summary>Redeems a token and returns the bootstrap HTML, or null if the token is unusable.</summary>
     public async Task<string?> RedeemAsync(string rawToken, HttpRequest request, CancellationToken cancellationToken)
+    {
+        // One redemption at a time: the status checks below and the status write
+        // that follows them are not atomic, so two requests arriving together with
+        // the same one-use token would otherwise both mint a guest session.
+        await _redeemGate.WaitAsync(cancellationToken).ConfigureAwait(false);
+        try
+        {
+            return await RedeemInternalAsync(rawToken, request, cancellationToken).ConfigureAwait(false);
+        }
+        finally
+        {
+            _redeemGate.Release();
+        }
+    }
+
+    /// <summary>Runs a single redemption; callers must hold the redemption gate.</summary>
+    private async Task<string?> RedeemInternalAsync(string rawToken, HttpRequest request, CancellationToken cancellationToken)
     {
         var tokenHash = await _tokenService.HashTokenAsync(rawToken, cancellationToken).ConfigureAwait(false);
         if (tokenHash is null)
@@ -73,6 +91,13 @@ public sealed class ShareLinkRedemptionService
             return null;
         }
 
+        // Checked before any library write: re-tagging the whole tree on every hit
+        // to an already-spent link would be a pointless metadata write storm.
+        if (record.OneUse && record.Status == ShareLinkStatus.Redeemed)
+        {
+            return null;
+        }
+
         if (!Guid.TryParse(record.ItemId, out var itemId))
         {
             await HandleFailureAsync(record, "Shared item snapshot is invalid.", cancellationToken).ConfigureAwait(false);
@@ -92,11 +117,6 @@ public sealed class ShareLinkRedemptionService
             record.MetadataTouched = true;
         }
 
-        if (record.OneUse && record.Status == ShareLinkStatus.Redeemed)
-        {
-            return null;
-        }
-
         if (string.IsNullOrWhiteSpace(record.DeviceId))
         {
             record.DeviceId = Guid.NewGuid().ToString("N");

Jellyfin.Plugin.ShareLinks/Services/ShareTokenService.cs

@@ -111,6 +111,9 @@ public sealed class ShareTokenService
                     _secretKey = Base64UrlDecode(secretText.Trim());
                     if (_secretKey.Length >= 16)
                     {
+                        // Also applied on load so a key written by an older build
+                        // stops being world readable.
+                        RestrictToOwner(_secretPath);
                         return _secretKey;
                     }
                 }
@@ -124,6 +127,7 @@ public sealed class ShareTokenService
             RandomNumberGenerator.Fill(generated);
             Directory.CreateDirectory(Path.GetDirectoryName(_secretPath)!);
             await File.WriteAllTextAsync(_secretPath, Base64UrlEncode(generated), cancellationToken).ConfigureAwait(false);
+            RestrictToOwner(_secretPath);
             _secretKey = generated;
             return _secretKey;
         }
@@ -133,6 +137,27 @@ public sealed class ShareTokenService
         }
     }
 
+    /// <summary>
+    /// Keeps the HMAC key readable by the server account only. Best effort: a
+    /// no-op on platforms without Unix file modes.
+    /// </summary>
+    private void RestrictToOwner(string path)
+    {
+        if (OperatingSystem.IsWindows())
+        {
+            return;
+        }
+
+        try
+        {
+            File.SetUnixFileMode(path, UnixFileMode.UserRead | UnixFileMode.UserWrite);
+        }
+        catch (Exception ex)
+        {
+            _logger.LogDebug(ex, "ShareLinks: could not restrict permissions on the token secret file.");
+        }
+    }
+
     private static string ComputeHash(byte[] secret, ReadOnlySpan<byte> tokenBytes)
     {
         using var hmac = new HMACSHA256(secret);

Jellyfin.Plugin.ShareLinks/Web/sharelinks.js

@@ -1111,14 +1111,15 @@
     }
 
     function chooseExpiryHours(config, onChoose) {
-        var options = durationOptions.map(function (option) {
+        var maxHours = clampPositiveInteger(config && config.MaxExpiryHours, 720);
+        var options = durationOptions.filter(function (option) {
+            return option.hours <= maxHours;
+        }).map(function (option) {
             return {
                 label: durationLabel(option.hours),
                 hours: option.hours
             };
         });
-
-        var maxHours = Math.max(clampPositiveInteger(config && config.MaxExpiryHours, 720), 720);
         var nowMs = Date.now();
         var minDate = new Date(nowMs + 5 * 60000);
         var maxDate = new Date(nowMs + maxHours * 3600000);

README.md

@@ -35,8 +35,9 @@ real user or handing over a login that sees everything.
    and records the share. Share a series or a season and the tag is applied to
    the whole tree underneath it too - series, seasons and episodes - so the
    guest can actually browse from the series page down into a season and an
-   episode, not just see a single locked node. The raw link token is shown to
+   episode, not just see a single locked node. Lookups only ever go through a
-   you once and never stored, only a keyed HMAC hash of it is kept.
+   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.
@@ -94,8 +95,10 @@ only a keyed HMAC hash of the token plus the metadata needed to audit and clean
 up the link. So:
 
 1. raw tokens are never logged
-2. raw tokens are never written to disk
+2. only the token's HMAC hash is used to look a link up
-3. the token is only returned in the creation response
+3. the finished share URL is kept on the record while the link is live, so the
+   dashboard can re-copy it, and is dropped again the moment the link is revoked
+   or expires
 4. token validation is a hash comparison
 5. guest-user creation and teardown live behind explicit service calls
 6. the real access boundary is the server-side tag policy; the web-client
@@ -106,7 +109,10 @@ on the server, using Jellyfin's own session manager. No password is ever stored
 anywhere, not even encrypted, and no password ever appears in the page sent to
 the guest. The only thing the guest's browser receives is a session token
 scoped to that one guest account, and that token dies the moment the guest
-account is cleaned up.
+account is cleaned up. On top of that, the guest account is assigned an authentication provider that
+refuses every interactive sign-in, so the normal login page cannot be used to get
+into a guest account at all, password or not. If the plugin is disabled Jellyfin
+falls back to its own invalid-provider handling, which refuses too.
 
 ## Configuration