Let a Logout Token the sink failed on be delivered again - #566
Merged
Merged
Conversation
Validating a Logout Token reserves its jti in the replay cache, and that happens before the notification sink runs. When the sink returned a description, threw, or the request was cancelled, the reservation stood, so the provider's retransmission was refused as a replay and the session was never ended. IReplayCache gains ReleaseAsync, and ReplayCacheBase an abstract RemoveAsync that DistributedReplayCache implements over IDistributedCache.RemoveAsync. The member is required: a default that did nothing would compile and keep this behavior in every store the library does not ship. The server's ConfiguredReplayCache forwards the release. BackChannelLogoutHandler gives the reservation back whenever the sink did not end the sessions. The release is not cancellable, since a cancelled request is one of the cases it answers, and a failed release is logged under a new event id instead of replacing the sink's outcome. The replay cache is optional on the handler, as it is on PushDeliveryHandler, for a host whose validator reserves nothing. PushDeliveryHandler keeps recording after its sink, and its remarks no longer say the cache cannot release. Ref #565
A replay refused at validation, a notification carrying no jti, and a release on ReplayCacheBase each get a test row, and the retry theory asserts a throw where it expects one instead of catching whatever the first request raised. ConfiguredReplayCache records a release at Debug under its own event id, so a debug log no longer shows one identifier reserved twice with nothing between. JwtAssertionAuthenticatorBase no longer says a reservation cannot be given back. ILogoutNotificationSink says a refusal returns the token to the replay guard, and DistributedReplayCache says what a release does to the race it already has between instances. The handler's log message says the sink did not report success rather than that nothing was done. Spellings in the new text are American. Ref #565
) After a release, only the next presentation of the token passes as fresh: it writes the entry back, and later ones are refused again. The DistributedReplayCache remark said the token passed until it expired. The release row in ConfiguredReplayCacheTests releases an identifier the message template does not contain, so it fails when the decorator logs a different identifier. Ref #565
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Closes #565.
A back-channel logout receiver reserves the Logout Token's
jtiwhile validating it, before the notification sink runs. If the sink returned a description, threw, or the request was cancelled, that reservation stayed in place. The provider's retransmission was then refused as a replay, and the session never ended.What changes:
IReplayCachegetsReleaseAsync.ReplayCacheBasegets an abstractRemoveAsync, whichDistributedReplayCacheimplements withIDistributedCache.RemoveAsync. The member is required, so a host with its own store gets a build error on upgrade and adds one delete. A default that did nothing would compile and keep the bug in every store we don't ship.ConfiguredReplayCachepasses the release on to the store it wraps and logs it at Debug, next to the reservation events it already writes. A host that runs both the server and the receiver shares that cache.ILogoutNotificationSinknow says that a refusal gives the token back, so the provider's retransmission reaches the sink again.DistributedReplayCachenotes that a release makes its existing race between instances wider.BackChannelLogoutHandlerreleases the reservation whenever the sink did not end the sessions. The release runs withCancellationToken.None, because a cancelled request is one of the cases it handles. If the release fails, it is logged under a new event id (ReservationKept) and the provider still gets the sink's outcome.PushDeliveryHandler, so a host that builds the handler itself around a validator that reserves nothing still works.PushDeliveryHandlerstill records only after its sink. Its remarks no longer say the cache cannot release.2.4 is not patched. Hosts on 2.4 keep the old behavior until they move to 3.0.