harden redemption, expiry limits and token storage

Findings from a pass over the plugin, smallest first:

Redemptions now run one at a time behind a gate. The status checks and the status
write that follows them were not atomic, so two requests arriving together with
the same one-use token could both mint a guest session. The spent-link check also
moved above the tagging step, so hammering an already-used link no longer re-tags
a whole series on every hit.

The configured maximum expiry is actually respected. Both the API and the picker
did Math.max(configured, 720), so setting the ceiling to anything under 30 days
was silently ignored. The picker now also hides the quick-pick durations that sit
above the ceiling.

The share URL, which carries the raw token, is dropped from the record when the
link is revoked or expires. Records are never deleted, so dead tokens were
accumulating in the store forever. Live links keep it so the dashboard can still
copy them, and the README claim that no token is ever written to disk is corrected
to say what the code actually does.

The HMAC key file is created 0600 instead of inheriting the default mask.
Cette révision appartient à :
Franciskid
2026-07-26 16:22:41 +02:00
Parent 36fc574d49
révision 5348b4e78b
6 fichiers modifiés avec 67 ajouts et 15 suppressions
+3 -3
Voir le fichier
@@ -189,10 +189,10 @@ public sealed class ShareLinksController : ControllerBase
return BadRequest(new { error = "Expiry must be positive." }); 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); var item = _libraryManager.GetItemById(itemId);
+6
Voir le fichier
@@ -100,6 +100,12 @@ public sealed class ShareLinkCleanupService : IShareLinkCleanupService
return record; 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>(); var errors = new List<string>();
try try
{ {
+25 -5
Voir le fichier
@@ -24,6 +24,7 @@ public sealed class ShareLinkRedemptionService
private readonly ShareLinkCleanupService _cleanupService; private readonly ShareLinkCleanupService _cleanupService;
private readonly ISessionManager _sessionManager; private readonly ISessionManager _sessionManager;
private readonly ILogger<ShareLinkRedemptionService> _logger; private readonly ILogger<ShareLinkRedemptionService> _logger;
private readonly SemaphoreSlim _redeemGate = new(1, 1);
/// <summary>Initializes a new instance of the <see cref="ShareLinkRedemptionService"/> class.</summary> /// <summary>Initializes a new instance of the <see cref="ShareLinkRedemptionService"/> class.</summary>
public ShareLinkRedemptionService( 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> /// <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) 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); var tokenHash = await _tokenService.HashTokenAsync(rawToken, cancellationToken).ConfigureAwait(false);
if (tokenHash is null) if (tokenHash is null)
@@ -73,6 +91,13 @@ public sealed class ShareLinkRedemptionService
return null; 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)) if (!Guid.TryParse(record.ItemId, out var itemId))
{ {
await HandleFailureAsync(record, "Shared item snapshot is invalid.", cancellationToken).ConfigureAwait(false); await HandleFailureAsync(record, "Shared item snapshot is invalid.", cancellationToken).ConfigureAwait(false);
@@ -92,11 +117,6 @@ public sealed class ShareLinkRedemptionService
record.MetadataTouched = true; record.MetadataTouched = true;
} }
if (record.OneUse && record.Status == ShareLinkStatus.Redeemed)
{
return null;
}
if (string.IsNullOrWhiteSpace(record.DeviceId)) if (string.IsNullOrWhiteSpace(record.DeviceId))
{ {
record.DeviceId = Guid.NewGuid().ToString("N"); record.DeviceId = Guid.NewGuid().ToString("N");
+22
Voir le fichier
@@ -124,6 +124,7 @@ public sealed class ShareTokenService
RandomNumberGenerator.Fill(generated); RandomNumberGenerator.Fill(generated);
Directory.CreateDirectory(Path.GetDirectoryName(_secretPath)!); Directory.CreateDirectory(Path.GetDirectoryName(_secretPath)!);
await File.WriteAllTextAsync(_secretPath, Base64UrlEncode(generated), cancellationToken).ConfigureAwait(false); await File.WriteAllTextAsync(_secretPath, Base64UrlEncode(generated), cancellationToken).ConfigureAwait(false);
RestrictToOwner(_secretPath);
_secretKey = generated; _secretKey = generated;
return _secretKey; return _secretKey;
} }
@@ -133,6 +134,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) private static string ComputeHash(byte[] secret, ReadOnlySpan<byte> tokenBytes)
{ {
using var hmac = new HMACSHA256(secret); using var hmac = new HMACSHA256(secret);
+4 -3
Voir le fichier
@@ -892,14 +892,15 @@
} }
function chooseExpiryHours(config, onChoose) { 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 { return {
label: durationLabel(option.hours), label: durationLabel(option.hours),
hours: option.hours hours: option.hours
}; };
}); });
var maxHours = Math.max(clampPositiveInteger(config && config.MaxExpiryHours, 720), 720);
var nowMs = Date.now(); var nowMs = Date.now();
var minDate = new Date(nowMs + 5 * 60000); var minDate = new Date(nowMs + 5 * 60000);
var maxDate = new Date(nowMs + maxHours * 3600000); var maxDate = new Date(nowMs + maxHours * 3600000);
+7 -4
Voir le fichier
@@ -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 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 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 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, 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 restricted by that tag to the shared item and its tree, and is signed in
automatically. They land on the title's page. 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: up the link. So:
1. raw tokens are never logged 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 4. token validation is a hash comparison
5. guest-user creation and teardown live behind explicit service calls 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 6. the real access boundary is the server-side tag policy; the web-client