Let a Logout Token the sink failed on be delivered again

src/Abblix.Jwt/ReplayPrevention/DistributedReplayCache.cs

@@ -25,6 +25,11 @@ namespace Abblix.Jwt.ReplayPrevention;
 /// read that way, since RFC 7523 Section 3 lets an authorization server reject a reused one. A
 /// deployment relying on that rejection takes a <see cref="ReplayCacheBase"/> over a store
 /// that decides and writes in one operation.
+/// <para>
+/// A caller that gives a reservation back after its work failed widens the same race: when two
+/// instances both hear "new" for one identifier and one of them releases, the entry the other
+/// one's success rests on is gone, and the next presentation of the token passes as fresh.
+/// </para>
 /// </remarks>
 /// <param name="cache">The distributed cache the host registered; the store is the host's choice.
 /// </param>
@@ -52,4 +57,8 @@ protected override Task<bool> ReserveIfAbsentAsync(
         TimeSpan timeToLive,
         CancellationToken cancellationToken)
         => cache.TryAddAsync(key, timeToLive, cancellationToken);
+
+    /// <inheritdoc />
+    protected override Task RemoveAsync(string key, CancellationToken cancellationToken)
+        => cache.RemoveAsync(key, cancellationToken);
 }

src/Abblix.Jwt/ReplayPrevention/IReplayCache.cs

@@ -47,4 +47,16 @@ Task<bool> TryReserveAsync(
         string identifier,
         DateTimeOffset expiresAt,
         CancellationToken cancellationToken = default);
+
+    /// <summary>
+    /// Gives back a reservation, so the same identifier reads as fresh again.
+    /// </summary>
+    /// <remarks>
+    /// For the caller whose own <see cref="TryReserveAsync"/> answered true and whose work on that
+    /// token then failed: without it a retry of the token is refused as a replay of work that never
+    /// happened. Any other caller that releases an identifier opens it to replay.
+    /// </remarks>
+    /// <param name="identifier">The value that was reserved.</param>
+    /// <param name="cancellationToken">Cancels the cache round trip.</param>
+    Task ReleaseAsync(string identifier, CancellationToken cancellationToken = default);
 }

src/Abblix.Jwt/ReplayPrevention/ReplayCacheBase.cs

@@ -28,10 +28,10 @@ namespace Abblix.Jwt.ReplayPrevention;
 /// and only one of them can be relied on to refuse.
 /// </para>
 /// <para>
-/// What a subclass must NOT do is as fixed as what it must: no read before the write, no release,
-/// no retry. Whether the reservation is indivisible is the store's promise, and it is the whole of
-/// what distinguishes a strict cache from <see cref="DistributedReplayCache"/>; a subclass that
-/// read first would hand back the very race the shape exists to close.
+/// What a subclass must NOT do is as fixed as what it must: no read before the write, no retry.
+/// Whether the reservation is indivisible is the store's promise, and it is the whole of what
+/// distinguishes a strict cache from <see cref="DistributedReplayCache"/>; a subclass that read
+/// first would hand back the very race the shape exists to close.
 /// </para>
 /// <para>
 /// <b>Redis.</b> One command, and the condition is evaluated by the server inside the write that
@@ -47,6 +47,9 @@ namespace Abblix.Jwt.ReplayPrevention;
 ///     protected override Task<bool> ReserveIfAbsentAsync(
 ///         string key, TimeSpan timeToLive, CancellationToken cancellationToken)
 ///         => _database.StringSetAsync(key, 1, timeToLive, When.NotExists)
+///
+///     protected override Task RemoveAsync(string key, CancellationToken cancellationToken)
+///         => _database.KeyDeleteAsync(key)
 /// }
 /// ]]></code>
 /// <para>
@@ -63,6 +66,8 @@ namespace Abblix.Jwt.ReplayPrevention;
 /// INSERT INTO replay_reservations (reservation_key, expires_at)
 /// VALUES (@key, @expiresAt)
 /// ON CONFLICT (reservation_key) DO NOTHING
+///
+/// DELETE FROM replay_reservations WHERE reservation_key = @key
 /// ]]></code>
 /// <para>
 /// A row count of one is a first sighting, zero is a replay - the same answer Redis gives, from the
@@ -129,6 +134,14 @@ public async Task<bool> TryReserveAsync(
         return await ReserveIfAbsentAsync(_keyPrefix + identifier, timeToLive, cancellationToken);
     }
 
+    /// <inheritdoc />
+    public Task ReleaseAsync(string identifier, CancellationToken cancellationToken = default)
+    {
+        // Refused for the same reason as a reservation of it: the bare prefix is no token's key.
+        ArgumentException.ThrowIfNullOrEmpty(identifier);
+        return RemoveAsync(_keyPrefix + identifier, cancellationToken);
+    }
+
     /// <summary>
     /// Stores something under <paramref name="key"/> only if nothing is there, and answers whether
     /// it was absent.
@@ -148,4 +161,11 @@ protected abstract Task<bool> ReserveIfAbsentAsync(
         string key,
         TimeSpan timeToLive,
         CancellationToken cancellationToken);
+
+    /// <summary>
+    /// Removes whatever is stored under <paramref name="key"/>; a key that is not there is not an error.
+    /// </summary>
+    /// <param name="key">The identifier with this cache's prefix already composed onto it.</param>
+    /// <param name="cancellationToken">Cancels the store round trip.</param>
+    protected abstract Task RemoveAsync(string key, CancellationToken cancellationToken);
 }

src/Abblix.Oidc.Server/Features/ClientAuthentication/JwtAssertionAuthenticatorBase.cs

@@ -65,9 +65,9 @@ protected SecurityProfileRequirements DefaultProfileRequirements
     /// given - a deployment-wide profile is a floor and a client may only tighten it. So this
     /// narrows and never widens.
     ///
-    /// Placed before the identifier is reserved, because a reservation is spent and cannot be given
-    /// back: a refusal after it would burn the assertion's own identifier on a request this check
-    /// was going to reject.
+    /// Placed before the identifier is reserved, because this path never gives a reservation back:
+    /// a refusal after it would burn the assertion's own identifier on a request this check was
+    /// going to reject.
     /// </remarks>
     /// <param name="timestamps">The assertion's timestamps, already read.</param>
     /// <param name="clientInfo">The client the assertion authenticates.</param>

src/Abblix.Oidc.Server/Features/ReplayPrevention/ConfiguredReplayCache.Logging.cs

@@ -23,4 +23,10 @@ partial class ConfiguredReplayCache
         Level = LogLevel.Debug,
         Message = "Marked jti {JwtId} as used, remembered until {ExpiresAt}")]
     private partial void LogMarkedAsUsed(string JwtId, DateTimeOffset ExpiresAt);
+
+    [LoggerMessage(
+        EventId = LogEvents.Tokens.DistributedJwtReplayCache.Released,
+        Level = LogLevel.Debug,
+        Message = "Released jti {JwtId}, so it reads as fresh again")]
+    private partial void LogReleased(string JwtId);
 }

src/Abblix.Oidc.Server/Features/ReplayPrevention/ConfiguredReplayCache.cs

@@ -17,8 +17,8 @@ namespace Abblix.Oidc.Server.Features.ReplayPrevention;
 
 /// <summary>
 /// The server's replay cache: the storage primitive from Abblix.JWT wearing this deployment's
-/// policy - the configured clock skew on top of every retention window, and the two log events
-/// an operator's runbook keys off.
+/// policy - the configured clock skew on top of every retention window, and the log events an
+/// operator's runbook keys off.
 /// </summary>
 /// <remarks>
 /// The retention is the WIDEST window in which the thing an entry names could still be accepted,
@@ -32,7 +32,7 @@ namespace Abblix.Oidc.Server.Features.ReplayPrevention;
 /// costs an entry held a while longer and cannot be a hole, which is what settles the direction to
 /// err in.
 /// </remarks>
-/// <param name="logger">Records the two replay events.</param>
+/// <param name="logger">Records what happens to each identifier.</param>
 /// <param name="inner">The storage the reservation actually lands in.</param>
 /// <param name="options">Where the clock skew is read from, re-read per call so a live
 /// configuration change takes effect without a restart.</param>
@@ -82,4 +82,11 @@ public async Task<bool> TryReserveAsync(
         LogMarkedAsUsed(identifier, skewed);
         return true;
     }
+
+    /// <inheritdoc />
+    public async Task ReleaseAsync(string identifier, CancellationToken cancellationToken = default)
+    {
+        await inner.ReleaseAsync(identifier, cancellationToken);
+        LogReleased(identifier);
+    }
 }

src/Abblix.Oidc.Server/LogEvents.cs

@@ -399,6 +399,7 @@ public static class DistributedJwtReplayCache
 
             public const int ReplayDetected = Base + 1;
             public const int MarkedAsUsed = Base + 2;
+            public const int Released = Base + 3;
         }
 
         /// <summary>

src/Abblix.SecurityEvents/BackChannelLogout/BackChannelLogoutHandler.Logging.cs

@@ -16,4 +16,10 @@ partial class BackChannelLogoutHandler
         Level = LogLevel.Warning,
         Message = "Back-channel logout refused: {Error} {Description}")]
     private partial void LogRefused(string error, string description);
+
+    [LoggerMessage(
+        EventId = LogEvents.BackChannelLogout.ReservationKept,
+        Level = LogLevel.Warning,
+        Message = "The sink did not report success for the Logout Token {TokenId} from {Issuer}, and the token stays reserved: a retransmission of it will be refused as a replay until it expires")]
+    private partial void LogReservationKept(Exception exception, string issuer, string tokenId);
 }

src/Abblix.SecurityEvents/BackChannelLogout/BackChannelLogoutHandler.cs

@@ -7,6 +7,8 @@
 
 using System.Net.Http.Headers;
 using System.Net.Mime;
+using Abblix.Jwt.ReplayPrevention;
+using Abblix.SecurityEvents.Delivery;
 using Microsoft.Extensions.Logging;
 
 namespace Abblix.SecurityEvents.BackChannelLogout;
@@ -26,10 +28,14 @@ namespace Abblix.SecurityEvents.BackChannelLogout;
 /// <param name="logger">Records every refusal, which no other party keeps.</param>
 /// <param name="validator">The Logout Token's validation, which is Section 2.6.</param>
 /// <param name="sink">Where the notification lands, which is Section 2.7.</param>
+/// <param name="replayCache">
+/// Where validation reserved the token, and where the reservation is given back when the sink did
+/// not end the sessions; null for a host whose validator reserves nothing here.</param>
 public sealed partial class BackChannelLogoutHandler(
     ILogger<BackChannelLogoutHandler> logger,
     ILogoutTokenValidator validator,
-    ILogoutNotificationSink sink)
+    ILogoutNotificationSink sink,
+    IReplayCache? replayCache = null)
 {
     /// <summary>
     /// The single parameter the request must carry (Section 2.5).
@@ -80,8 +86,47 @@ public async Task<BackChannelLogoutResult> HandleAsync(
             return Refuse(exception.Message);
         }
 
-        var refusal = await sink.ConsumeAsync(notification, cancellationToken);
-        return refusal is null ? BackChannelLogoutResult.Ok : Refuse(refusal);
+        var acted = false;
+        try
+        {
+            var refusal = await sink.ConsumeAsync(notification, cancellationToken);
+            if (refusal is not null)
+                return Refuse(refusal);
+
+            acted = true;
+            return BackChannelLogoutResult.Ok;
+        }
+        finally
+        {
+            // A refusal, a throw and a cancellation all leave the sessions open, and Section 2.5
+            // lets the provider retransmit the same token when it suspects a recoverable failure.
+            if (!acted)
+                await ReleaseAsync(notification);
+        }
+    }
+
+    /// <summary>
+    /// Gives back the reservation validation made for this token.
+    /// </summary>
+    /// <remarks>
+    /// Not cancelable, because a canceled request is one of the failures it answers. A release that
+    /// fails is logged rather than thrown: the provider is owed the outcome of the logout, and the
+    /// entry that stays only expires as it would have without the release.
+    /// </remarks>
+    private async Task ReleaseAsync(LogoutNotification notification)
+    {
+        if (replayCache is null || notification.TokenId is not { } tokenId)
+            return;
+
+        try
+        {
+            await replayCache.ReleaseAsync(
+                ReplayIdentifier.ForToken(notification.Issuer, tokenId), CancellationToken.None);
+        }
+        catch (Exception exception)
+        {
+            LogReservationKept(exception, notification.Issuer, tokenId);
+        }
     }
 
     /// <summary>

src/Abblix.SecurityEvents/BackChannelLogout/ILogoutNotificationSink.cs

@@ -30,6 +30,11 @@ namespace Abblix.SecurityEvents.BackChannelLogout;
 /// "if the logout request was invalid or the logout FAILED" - so a sink that could not end the
 /// sessions says so rather than acknowledging work it did not do.
 /// </para>
+/// <para>
+/// A description, like an exception, gives the token back to the replay guard, so the provider's
+/// retransmission of it reaches this sink again. A sink that refuses the same token every time sees
+/// it on every retransmission until the token expires.
+/// </para>
 /// </remarks>
 public interface ILogoutNotificationSink
 {

src/Abblix.SecurityEvents/Delivery/PushDeliveryHandler.cs

@@ -24,8 +24,7 @@ namespace Abblix.SecurityEvents.Delivery;
 /// a token the sink accepted is written to the replay cache. Nothing on this path READS that cache,
 /// so a redelivery reaches the sink again - RFC 8935 Section 2 lets a transmitter redeliver
 /// regardless of earlier responses, and <see cref="ISecurityEventSink"/> answers for it by
-/// requiring idempotent processing. Why the write cannot come earlier, and why that is the only
-/// correct order available here, is on <c>RecordAsync</c>.
+/// requiring idempotent processing. Why the write cannot come earlier is on <c>RecordAsync</c>.
 /// </para>
 /// <para>
 /// Nothing here knows which profile of SET it carries. RFC 8935 is a delivery specification and
@@ -127,9 +126,7 @@ public async Task<PushDeliveryResult> HandleAsync(
     /// <para>
     /// The consequence is that a repeat reaches the sink again rather than being short-circuited
     /// here. That is what <see cref="ISecurityEventSink"/> already requires of it - "Processing
-    /// must be idempotent" - and it is the only correct short-circuit available while
-    /// <see cref="IReplayCache"/> can reserve but not release: a cache entry cannot be undone when
-    /// the work it stands for failed, so it must not be written until that work has succeeded.
+    /// must be idempotent".
     /// </para>
     /// </remarks>
     private async Task RecordAsync(

src/Abblix.SecurityEvents/LogEvents.cs

@@ -40,5 +40,11 @@ public static class BackChannelLogout
         /// that traveled back to the provider.
         /// </summary>
         public const int RequestRefused = Base + 1;
+
+        /// <summary>
+        /// A Logout Token the sink did not report success for could not be released from the replay
+        /// cache. The message carries its issuer and identifier, and the exception says why.
+        /// </summary>
+        public const int ReservationKept = Base + 2;
     }
 }

tests/Abblix.Jwt.UnitTests/ReplayCacheBaseTests.cs

@@ -44,6 +44,15 @@ protected override Task<bool> ReserveIfAbsentAsync(
             Calls++;
             return Task.FromResult(answer);
         }
+
+        public string? RemovedKey { get; private set; }
+
+        protected override Task RemoveAsync(string key, CancellationToken cancellationToken)
+        {
+            RemovedKey = key;
+            Token = cancellationToken;
+            return Task.CompletedTask;
+        }
     }
 
     private static RecordingCache NewCache(bool answer = true, string prefix = "replay:")
@@ -122,4 +131,34 @@ await Assert.ThrowsAsync<ArgumentException>(
         // under it read as a replay.
         Assert.Equal(0, cache.Calls);
     }
+
+    /// <summary>
+    /// A release removes the key the reservation wrote, so it has to compose the same prefix: without
+    /// it the store deletes nothing and the token stays refused.
+    /// </summary>
+    [Fact]
+    public async Task AReleaseRemovesTheKeyTheReservationWrote()
+    {
+        var cancellationToken = TestContext.Current.CancellationToken;
+        var cache = NewCache(prefix: "rollout-after:");
+
+        using var release = CancellationTokenSource.CreateLinkedTokenSource(cancellationToken);
+
+        await cache.TryReserveAsync("jti-1", Now.AddMinutes(5), cancellationToken);
+        await cache.ReleaseAsync("jti-1", release.Token);
+
+        Assert.Equal(cache.Key, cache.RemovedKey);
+        Assert.Equal(release.Token, cache.Token);
+    }
+
+    [Fact]
+    public async Task AnEmptyIdentifier_IsNotReleased()
+    {
+        var cache = NewCache();
+
+        await Assert.ThrowsAsync<ArgumentException>(
+            () => cache.ReleaseAsync("", TestContext.Current.CancellationToken));
+
+        Assert.Null(cache.RemovedKey);
+    }
 }