dotnet / dotnet/aspnetcore

SignalR - Browser/TypeScript client - the Server-Sent Event keep alive ping fails to renew the access token if a 401 Unauthorized happens on the ping

Open
#56,494 2 comments 0 reactions 0 assignees View on GitHub
area-signalr bug feature-client-javascript help wanted
Dominant language
C#
Stars
38.4k
Forks
10.9k
Avg merge
2d 6h
Merged PRs (30d)
290

Description

### Is there an existing issue for this?

- [X] I have searched the existing issues

### Describe the bug

In the Browser/TypeScript SignalR client, the Server-Sent Event (SSE) keep alive ping fails to renew the access token (it fails to perform the existing retry logic and call the access token factory again) if a 401 Unauthorised happens on the ping.

This doesn't just affect the SSE ping... Similarly, when Long Polling connections are being reissued after they timeout, this same issue causes the Long Polling to unnecessarily go through a longer lifecycle where is 'completes' / sets this._running to false and closes by calling this.raiseOnClose() - instead of a shorter lifecycle that keeps the _poll function looping in a running state.

When an SSE keep alive ping happens (for example), AccessTokenHttpClient calls `this._innerClient` to send a request.
https://github.com/dotnet/aspnetcore/blob/94259788d58e16ba753900b4bf855a6aee08dcb1/src/SignalR/clients/ts/signalr/src/AccessTokenHttpClient.ts#L28

Then FetchHttpClient throws an HttpError if the response is not Ok.
https://github.com/dotnet/aspnetcore/blob/250b489a34b6c01509ed24b18b47b082d8ba4744/src/SignalR/clients/ts/signalr/src/FetchHttpClient.ts#L125

And there be the problem, because that HttpError is not caught anywhere, so the existing retry logic in AccessTokenHttpClient is prevented from inspecting the response.statusCode for a 401 Unauthorized and performing a retry to get a new access token.
https://github.com/dotnet/aspnetcore/blob/94259788d58e16ba753900b4bf855a6aee08dcb1/src/SignalR/clients/ts/signalr/src/AccessTokenHttpClient.ts#L30

```
const response = await this._innerClient.send(request); // FetchHttpClient THROWS IN HERE

// SO THIS NEVER EXECUTES:
if (allowRetry && response.statusCode === 401 && this._accessTokenFactory) {
this._accessToken = await this._accessTokenFactory();
this._setAuthorizationHeader(request);
return await this._innerClient.send(request);
}
```

Given that it is existing retry logic that is not working as intended, I think this is a bug.

### Expected Behavior

The existing retry code should execute as apparently intended and call the access token factory.

### Steps To Reproduce

1. Set up a hub connection (using the TypeScript client in a browser which has fetch available) with the option `transport: HttpTransportType.ServerSentEvents,`
2. Have a bearer access token with a lifetime shorter than the keep alive ping (or ensure that the ping will return a 401 Unauthorized)
3. Let it run and wait for the keep alive ping to happen
4. When the ping happens and a 401 is returned, the retry logic in AccessTokenHttpClient.ts does not execute because an HttpError is thrown
https://github.com/dotnet/aspnetcore/blob/94259788d58e16ba753900b4bf855a6aee08dcb1/src/SignalR/clients/ts/signalr/src/AccessTokenHttpClient.ts#L30

### Exceptions (if any)

HttpError thrown by FetchHttpClient

https://github.com/dotnet/aspnetcore/blob/250b489a34b6c01509ed24b18b47b082d8ba4744/src/SignalR/clients/ts/signalr/src/FetchHttpClient.ts#L125

### .NET Version

8.0.302

### Anything else?

"@microsoft/signalr": "^8.0.0",

I see https://github.com/dotnet/aspnetcore/issues/5297 is somewhat related - not sure if that will change the logic or the intended approach.

Contributor guide

Open the contributing guide

Assessment

This issue has not been assessed yet.

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.