Read & use client-provided session GUID for concurrency control #238
Conversation
| public bool IsInvocationPermitted(HubInvocationContext context) | ||
| { | ||
| bool hubRegistered = ConnectionIds.TryGetValue(context.Hub.GetType(), out string? registeredConnectionId); | ||
| bool connectionIdMatches = registeredConnectionId == context.Context.ConnectionId; | ||
|
|
||
| return hubRegistered && connectionIdMatches; | ||
| } | ||
|
|
||
| public bool CanCleanUpConnection(HubLifetimeContext context) | ||
| { | ||
| bool hubRegistered = ConnectionIds.TryGetValue(context.Hub.GetType(), out string? registeredConnectionId); | ||
| bool connectionIdMatches = registeredConnectionId == context.Context.ConnectionId; | ||
|
|
||
| return hubRegistered && connectionIdMatches; | ||
| } |
There was a problem hiding this comment.
Readers will note that the content of this method is mostly moved from ConcurrentConnectionLimiter, except for the fact that the tokenIdMatches check is gone.
As far as I can tell that check was mostly dead / bogus anyway; the signalr communication protocol is such that the JWT is only sent at the start of a connection, after which a protocol switch to webscokets happens, and headers are not sent anymore. Which, in turn, means that the JWT is not being sent over the wire on remote procedure calls, and the previous check would just keep checking the same stored JWT from the initial connection, probably fetching it from a dictionary keyed by connection ID or something. Thus, the connection ID should be more than enough to enforce concurrency at this point.
I'd keep it available until next release at least (as in the one that doesn't happen this week). |
Read description of aforementioned PR for explanation what is going on here.
This change is designed to be backwards-compatible, in the sense that old client will continue to use the
jticialm (with all of the deficiencies which that incurs), and new clients with the aforementioned change will use the new one which is hopefully rid of the deficiencies in question. We can probably shutter the old path in a few months - or I can just delete it now if that is considered acceptable.