Skip to content

Harden SignalR stateful reconnect. - #67409

Merged
cincuranet merged 11 commits into
dotnet:mainfrom
cincuranet:signalr-reconnect
Jul 22, 2026
Merged

Harden SignalR stateful reconnect.#67409
cincuranet merged 11 commits into
dotnet:mainfrom
cincuranet:signalr-reconnect

Conversation

@cincuranet

@cincuranet cincuranet commented Jun 25, 2026

Copy link
Copy Markdown
Contributor

Fixes #67311. Takes into account recent #67111.

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR hardens SignalR connection rebind behavior by rejecting requests that attempt to act on an existing connection using a different NameIdentifier, aligning stateful reconnect/long-polling behavior with the security goal in #67311 and updating tests/logging accordingly.

Changes:

  • Add/adjust tests to validate 403 rejection on user-identifier mismatch for stateful reconnect, long polling poll, send, and delete.
  • Replace the prior “log only” behavior with a dedicated warning log event for rejected mismatches.
  • Update client functional coverage to expect a rejected stateful reconnect instead of a warning-only path.

Reviewed changes

Copilot reviewed 5 out of 5 changed files in this pull request and generated 5 comments.

Show a summary per file
File Description
src/SignalR/common/Http.Connections/test/HttpConnectionDispatcherTests.cs Adds new unit tests covering 403 rejection + connection preservation for mismatched users across reconnect/poll/send/delete.
src/SignalR/common/Http.Connections/test/HttpConnectionDispatcherTests.AuthenticationRefresh.cs Updates an auth-refresh-related test to reflect the new rejection behavior when auth refresh is disabled.
src/SignalR/common/Http.Connections/src/Internal/HttpConnectionDispatcher.Log.cs Removes the old “username changed” log and adds a new warning log for rejected mismatches.
src/SignalR/common/Http.Connections/src/Internal/HttpConnectionDispatcher.cs Implements the rejection helper and wires it into reconnect/poll/send/delete code paths.
src/SignalR/clients/csharp/Client/test/FunctionalTests/HubConnectionTests.cs Updates the functional test to validate rejection (via log) when the reconnect authenticates as a different user.

Comment thread src/SignalR/common/Http.Connections/src/Internal/HttpConnectionDispatcher.cs Outdated
Comment thread src/SignalR/common/Http.Connections/src/Internal/HttpConnectionDispatcher.cs Outdated
Comment thread src/SignalR/common/Http.Connections/src/Internal/HttpConnectionDispatcher.Log.cs Outdated
Comment thread src/SignalR/clients/csharp/Client/test/FunctionalTests/HubConnectionTests.cs Outdated
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
@cincuranet

Copy link
Copy Markdown
Contributor Author

@BrennanConroy Can you look at the Copilot comments above, please. I intentionally have the condition with EnableAuthenticationRefresh, but if you agree I can remove it and also gate the username change with EnableAuthenticationRefresh enabled.

Comment thread src/SignalR/common/Http.Connections/src/Internal/HttpConnectionDispatcher.cs Outdated
@cincuranet
cincuranet requested a review from BrennanConroy July 16, 2026 09:51
@cincuranet

Copy link
Copy Markdown
Contributor Author

@BrennanConroy This is ready for final review.

Comment thread src/SignalR/common/Http.Connections/src/Internal/HttpConnectionDispatcher.cs Outdated
@cincuranet

Copy link
Copy Markdown
Contributor Author

/ba-g Failures are unrelated.

@cincuranet
cincuranet merged commit 5a7aa99 into dotnet:main Jul 22, 2026
24 of 26 checks passed
@cincuranet
cincuranet deleted the signalr-reconnect branch July 22, 2026 11:21
@dotnet-milestone-bot dotnet-milestone-bot Bot added this to the 11.0-rc1 milestone Jul 23, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Harden SignalR stateful reconnect with different NameIdentifier by rejecting it instead of only logging it

6 participants