Terminate SignalR connections when the corresponding token expires

[muratg]

Originally asked in: aspnet/SignalR#1155 (comment)

[analogrelay]

Hmm, this is tricky since the token isn't "re-transmitted" on each message. I suppose we could have code that checks the expiration of the token each time a message is received.

[analogrelay]

We should make sure this is relatively easy to add, even if we don't add it ourselves. Having "filter"-style logic for incoming/outgoing HubMessages would be good.

[muratg]

One idea is to use KeepAlive heartbeat to check token expiry. cc @anurse

[analogrelay]

This is something we'll look to make possible, but I don't think we'll actually disconnect the connection ourselves. The new IConnectionHeartbeatFeature should allow the implementer of a Hub to check the token at a regular interval and then terminate the connection if they need to. I'll look at making an example of this in the preview2 milestone.

[davidfowl]

We're not making this first class in 2.1.0 moving to backlog.

[jpmnteiro]

Original question:

I understand that this has been moved to the backlog, but I've ran with @anurse's idea and added some logic to the IConnectionHeartbeatFeature to try and check the token.

The issue, however, is how to access the token from within the callback (or from inside a hub for that matter). AFAICT, the caller only has access to the identity, not the token (one could potentially include the expiration has a claim in the token, but that smells)

Any ideas are appreciated.

Update:

Actually it is possible, as for anyone that stumbles upon this thread with a similar question, there is a way to access the token via Context.Connection.GetHttpContext().GetTokenAsync(""). Make sure you configure your tokens to be saved to the AuthenticationProperties (kudos to this article for pointing me in the right direction).

[kevinchalet]

@muratg @anurse is this what you had in mind?

public override async Task OnConnectedAsync()
{
    var feature = Context.Connection.Features.Get<IConnectionHeartbeatFeature>();
    if (feature == null)
    {
        return;
    }

    var context = Context.Connection.GetHttpContext();
    if (context == null)
    {
        throw new InvalidOperationException("The HTTP context cannot be resolved.");
    }

    // Extract the authentication ticket from the access token.
    // Note: this operation should be cheap as the authentication result
    // was already computed when SignalR invoked the authentication handler
    // and automatically cached by AuthenticationHandler.AuthenticateAsync().
    var result = await context.AuthenticateAsync(OAuthValidationDefaults.AuthenticationScheme);
    if (result.Ticket == null)
    {
        Context.Connection.Abort();

        return;
    }

    feature.OnHeartbeat(state =>
    {
        var (ticket, connection) = ((AuthenticationTicket, HubConnectionContext)) state;

        // Ensure the access token token is still valid.
        // If it's not, abort the connection immediately.
        if (ticket.Properties.ExpiresUtc < DateTimeOffset.UtcNow)
        {
            connection.Abort();
        }
    }, (result.Ticket, Context.Connection));
}

[analogrelay]

Yeah, that looks about right.

[davidfowl]

Nice! That's a great sample for the docs.

[kevinchalet]

Feel free to copy it, but if you adapt it for the JWT handler, double check it works correctly 'cause I don't think the JWT handler populates the AuthenticationProperties like the aspnet-contrib validation handler does.

[kevinchalet]

Also, it might be cool to handle that more gracefully. Aborting the connection without telling the client why the server did that is a bit meh 😅

[analogrelay]

Aborting the connection without telling the client why the server did that is a bit meh

Seems OK to me ;P

[davidfowl]

Logs....

[kevinchalet]

Logs....

Logs are for developers, not for the SignalR client.

When an access token expires, a client is expected to renew it and re-try the request using the new token (in the OIDC world, using a refresh token or by sending a prompt=none authorization request).

If the client isn't told the connection was aborted because the access token expired, it will have to blindly renew the token, even if the connection wasn't aborted for this reason.

[analogrelay]

I do wonder if we should consider a "Close" frame in the protocol to allow the client to have a chance to see why the connection was closed (including an Exception message, such as "Authentication token expired" or even possibly a code). The weird thing is things like Long Polling where the connection does have to stay "open" long enough for the client to poll for the Close message.

[analogrelay]

Of course this could be achieved in an app-specific way in the hub itself by just invoking some method on the client before aborting the connection.

[kevinchalet]

Of course this could be achieved in an app-specific way in the hub itself by just invoking some method on the client before aborting the connection.

It's worth noting the heartbeat callback is not async. If you do that, you'll end up with some nasty blocking IO 😅

[analogrelay]

You could do a fire-and-forget task to send the message and Abort after an Ack of some kind (or a timeout).

[davidfowl]

Unfortunately, this logic will now break with long polling because we did this crazy thing aspnet/SignalR#1644. We should add a test case for this so that we can make it work.

This line:

var result = await context.AuthenticateAsync(OAuthValidationDefaults.AuthenticationScheme);

Will likely explode.

One thing you can do is look for the presence of IConnectionInherentKeepAliveFeature and disable this code if that feature exists. That means that the transport itself supports timing out (i.e. long polling) and doesn't need this heartbeat.

[jpmnteiro]

@davidfowl would you recommend checking for the feature or checking for the type of transport on the OnConnected(...) method?

[davidfowl]

The feature.

[kevinchalet]

Unfortunately, this logic will now break with long polling because we did this crazy thing #1644.

Crazy is really the right word. It will make user code relying on request services quite hard to debug. If the real context is not available, I'm not sure injecting a fake one is really a sane idea.

That's super annoying in this specific case, because we only need the HTTP context very early in OnConnectedAsync(), which is supposed to be always available as this method is invoked for the first time when the first Long Polling request is still processed.

[davidfowl]

Crazy is really the right word. It will make user code relying on request services quite hard to debug. If the real context is not available, I'm not sure injecting a fake one is really a sane idea.

We'll find out when people try out preview2.

That's super annoying in this specific case, because we only need the HTTP context very early in OnConnectedAsync(), which is supposed to be always available as this method is invoked for the first time when the first Long Polling request is still processed.

No, it's not available. Thats a bad assumption. Still I agree what we're doing is slightly crazy now (and I personally made the change) but we're waiting to get more feedback before changing it.

[kevinchalet]

No, it's not available. Thats a bad assumption.

Care to explain why?

[davidfowl]

Care to explain why?

Your hub is not executed on a request thread, everything is asynchronous and nothing runs inline. The request comes in and bytes are written to a queue and the request ends. Later on that data is picked up and parsed into a hub message and that pipeline runs. The two things are completely decoupled

[kevinchalet]

And that's specific to Long Polling or applicable to all transports?

[davidfowl]

FWIW, I'm not sure this kind of check is really a good idea.

I don't disagree, we might want to only do it if it's a debug build. These exist because it's hard to debug tests with timeouts generally.

/cc @anurse

[analogrelay]

Yeah, it's one of those "it sucks if it's there and it sucks that it's there" things. I think limiting it to DEBUG builds is an OK option. The reason I'm not 100% sold on that is that if you are debugging an application (using a Release build) it can be very annoying to have the connection time out, so having a way for users to turn it on this behavior would be good. We could maybe use a quirking flag for that?

[kevinchalet]

Why not a configurable option, independent of the build configuration/debugging environment?

services.AddSignalR(options =>
{
    options.DisableAutomaticConnectionRemoval = true;
});

If you think it's a too dangerous option, an obscure AppContext switch might be a good option.

[analogrelay]

I think it's weird to put something like this on a public options surface. It's really only something you want while actively debugging the app.

[analogrelay]

We should also consider the impact on Server-Sent Events and Long Polling connections. See aspnet/SignalR#2553 (comment)

[weedkiller]

How can we terminate a SignarlR2 Client Connection, and I dont mean a timeout.

[alexb5dh]

Any up-to-date workarounds at the moment?

I'm thinking about implementing custom TimedPrincipal that will throw on all methods after token expires and assigning it to HttpContext.User. Exception handler in that case will force a client to disconnect.

Global handling is limited until #5353 is done but that can be fixed by temporarily adding try/catch in all methods involved.

Would be great if anyone could propose any simpler idea.

[davidfowl]

The core idea here is solid #5283 (comment). The only change would be to stash the AuthenticationResult in HttpContext.Items after the authentication middleware runs then write the same logic in OnConnectedAsync but instead of calling AuthenticateAsync just grab the AuthenticationResult and use it to implement the same logic (aborts the connection after it times out).

[netbidev]

Is there any news about this feature?

[warappa]

Using the workaround in .NET 5

The provided workaround above (#5283 (comment)) doesn't work anymore in .NET 5, as Abort now sets allowReconnect to false which tells the client to not reconnect anymore (see #25693).
You need to use HubCallerContext instead of HubConnectionContext, as HubConnectionContext isn't exposed anymore.

It's now required to directly abort the underlying connection to enable a client-reconnect.
Use the code of the above workaround and replace the OnHeartbeat code with this:

feature.OnHeartbeat(state =>
{
    var (ticket, context) = ((AuthenticationTicket, HubCallerContext))state;
                
    // Ensure the access token token is still valid.
    // If it's not, abort the connection immediately.
    if (ticket.Properties.ExpiresUtc < DateTimeOffset.UtcNow)
    {
         context.GetHttpContext().Abort(); // abort hard - not gracefully (context.Abort()) - to ensure reconnect
    }
}, (result.Ticket, Context));

PS: ExpiresUtc
I also needed to manually set tokenValidatedContext.Properties.ExpiresUtc in OnTokenValidated in Startup.cs.

services.AddAuthentication("CustomAuthenticationScheme")
.AddCustomAuthentication(configure =>
{
    ...
    configure.Events.OnTokenValidated = async context =>
    {
        var token = context.SecurityToken;
        context.Properties.ExpiresUtc = token.ValidTo;
    };
    ...
});

[alinflorin]

Using the workaround in .NET 5

The provided workaround above (#5283 (comment)) doesn't work anymore in .NET 5, as Abort now sets allowReconnect to false which tells the client to not reconnect anymore (see #25693).
You need to use HubCallerContext instead of HubConnectionContext, as HubConnectionContext isn't exposed anymore.

It's now required to directly abort the underlying connection to enable a client-reconnect.
Use the code of the above workaround and replace the OnHeartbeat code with this:

feature.OnHeartbeat(state =>
{
    var (ticket, context) = ((AuthenticationTicket, HubCallerContext))state;
                
    // Ensure the access token token is still valid.
    // If it's not, abort the connection immediately.
    if (ticket.Properties.ExpiresUtc < DateTimeOffset.UtcNow)
    {
         context.GetHttpContext().Abort(); // abort hard - not gracefully (context.Abort()) - to ensure reconnect
    }
}, (result.Ticket, Context));

PS: ExpiresUtc
I also needed to manually set tokenValidatedContext.Properties.ExpiresUtc in OnTokenValidated in Startup.cs.

services.AddAuthentication("CustomAuthenticationScheme")
.AddCustomAuthentication(configure =>
{
    ...
    configure.Events.OnTokenValidated = async context =>
    {
        var token = context.SecurityToken;
        context.Properties.ExpiresUtc = token.ValidTo;
    };
    ...
});

Hello,

Trying this out right now, it seems that the OnHeartbeat function is executed every second or so (not sure why). This is a bit too often I'd say, especially if there are lots of connected users. Is there a workaround? I would say executing this every time the client sends a "ping" - 15s by default I think - would be perfect. Is that possible?

[davidfowl]

@BrennanConroy is going to be looking at this with #5297