diff --git a/Jellyfin.Plugin.ShareLinks/Api/ShareLinksController.cs b/Jellyfin.Plugin.ShareLinks/Api/ShareLinksController.cs index 213ceaf..d62906c 100644 --- a/Jellyfin.Plugin.ShareLinks/Api/ShareLinksController.cs +++ b/Jellyfin.Plugin.ShareLinks/Api/ShareLinksController.cs @@ -189,10 +189,10 @@ public sealed class ShareLinksController : ControllerBase return BadRequest(new { error = "Expiry must be positive." }); } - var effectiveMaxExpiryHours = Math.Max(config.MaxExpiryHours, 720); - if (expiryHours > effectiveMaxExpiryHours) + var maxExpiryHours = config.MaxExpiryHours > 0 ? config.MaxExpiryHours : 720; + 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); diff --git a/Jellyfin.Plugin.ShareLinks/Services/ShareLinkCleanupService.cs b/Jellyfin.Plugin.ShareLinks/Services/ShareLinkCleanupService.cs index 0c9a0ef..b8a587c 100644 --- a/Jellyfin.Plugin.ShareLinks/Services/ShareLinkCleanupService.cs +++ b/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(); try { diff --git a/Jellyfin.Plugin.ShareLinks/Services/ShareLinkRedemptionService.cs b/Jellyfin.Plugin.ShareLinks/Services/ShareLinkRedemptionService.cs index 1b50166..1f60e29 100644 --- a/Jellyfin.Plugin.ShareLinks/Services/ShareLinkRedemptionService.cs +++ b/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 _logger; + private readonly SemaphoreSlim _redeemGate = new(1, 1); /// Initializes a new instance of the class. public ShareLinkRedemptionService( @@ -48,6 +49,23 @@ public sealed class ShareLinkRedemptionService /// Redeems a token and returns the bootstrap HTML, or null if the token is unusable. public async Task 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(); + } + } + + /// Runs a single redemption; callers must hold the redemption gate. + private async Task 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"); diff --git a/Jellyfin.Plugin.ShareLinks/Services/ShareTokenService.cs b/Jellyfin.Plugin.ShareLinks/Services/ShareTokenService.cs index 6b75ec2..d254cfd 100644 --- a/Jellyfin.Plugin.ShareLinks/Services/ShareTokenService.cs +++ b/Jellyfin.Plugin.ShareLinks/Services/ShareTokenService.cs @@ -124,6 +124,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 +134,27 @@ public sealed class ShareTokenService } } + /// + /// Keeps the HMAC key readable by the server account only. Best effort: a + /// no-op on platforms without Unix file modes. + /// + 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 tokenBytes) { using var hmac = new HMACSHA256(secret); diff --git a/Jellyfin.Plugin.ShareLinks/Web/sharelinks.js b/Jellyfin.Plugin.ShareLinks/Web/sharelinks.js index d7c1b6d..3d41da5 100644 --- a/Jellyfin.Plugin.ShareLinks/Web/sharelinks.js +++ b/Jellyfin.Plugin.ShareLinks/Web/sharelinks.js @@ -892,14 +892,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); diff --git a/README.md b/README.md index 463ab4c..3f40bc7 100644 --- a/README.md +++ b/README.md @@ -33,8 +33,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 - you once and never stored, only a keyed HMAC hash of it is kept. + episode, not just see a single locked node. 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. @@ -92,8 +93,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 -3. the token is only returned in the creation response +2. only the token's HMAC hash is used to look a link up +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