dotnet / dotnet/aspnetcore

AccessTokenFactory improvement options

Open
#56,290 0 comments 0 reactions 0 assignees View on GitHub
area-signalr
Dominant language
C#
Stars
38.4k
Forks
10.9k
Avg merge
2d 5h
Merged PRs (30d)
276

Description

### Is there an existing issue for this?

- [X] I have searched the existing issues

### Is your feature request related to a problem? Please describe the problem.

_No response_

### Describe the solution you'd like

In the docs, it's recommended to refresh the accessToken within the AccessTokenFactory method. However, I have noticed that this method is called with every SignalR connection, regardless of the reason why that connection was required.

If this is an expensive (aka serverside) operation, it doesn't make sense to refresh the accessToken every time. Imagine a client with patchy internet having to reconnect, and that yielding an additional and unnecessary token refresh. Not only from the client's perspective, also in terms of server load, and latency for all clients on first connection.

Instead, the accessToken should be cached until there is specifically a 401 exception, at which point it should be refreshed. At least there should be an option to do this.

Alternatively, an optional boolean property could be passed into the AccessTokenFactory indicating whether the previous token has been found to be in valid.

Either that, or it should be possible to catch 401 exceptions specifically on the HubConnection, which could in turn be configured to trigger a token refresh. Right now, the exception is simply plaintext. Perhaps adding a status code property would be enough.

In general, errors on the HubConnection are not possible to catch. The 'onclose' doesn't fire in case of all errors, so it's not the correct place for that, and there is no equivalent 'onerror'. One way to fix this would be to add an 'onerror' event which includes the status code as part of the error.

### Additional context

_No response_

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.