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 建立於 2024年6月27日

在 GitHub 查看
 (2 留言) (0 反應) (0 負責人)C# (37,933 star) (10,653 fork)batch import
area-signalrbughelp wanted

描述

Is there an existing issue for this?

  • 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.

貢獻者指南

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 · dotnet/aspnetcore#56494 | Good First Issue