Skip to content

Fix SignalR reconnect identity bypass with EnableAuthenticationRefresh - #68008

Open
BrennanConroy wants to merge 1 commit into
mainfrom
brennanconroy-fix-signalr-reconnect-identity-bypass
Open

Fix SignalR reconnect identity bypass with EnableAuthenticationRefresh#68008
BrennanConroy wants to merge 1 commit into
mainfrom
brennanconroy-fix-signalr-reconnect-identity-bypass

Conversation

@BrennanConroy

Copy link
Copy Markdown
Member

Fixes #67409 (comment)

The transport-layer identity reject added in PR #67409 was bypassed when EnableAuthenticationRefresh is enabled: a different authenticated user presenting a stolen connection token could attach a long poll or stateful WebSocket reconnect and be served the original user's stream, because the stale/skip branches assume same-user without verifying identity.

  • Ungate the identity reject in EnsureConnectionStateAsync so it always runs for reconnect-capable connections regardless of EnableAuthenticationRefresh. A legitimate same-user token refresh keeps the same identity key and still flows through the authentication-refresh path.
  • Add an identity check in ProcessRefresh before OnAuthenticationRefresh and UpdateUser, rejecting a different user with 403 'user_changed' via the JSON error contract without tearing down the connection.

The transport-layer identity reject added in PR #67409 was bypassed when
EnableAuthenticationRefresh is enabled: a different authenticated user
presenting a stolen connection token could attach a long poll or stateful
WebSocket reconnect and be served the original user's stream, because the
stale/skip branches assume same-user without verifying identity.

- Ungate the identity reject in EnsureConnectionStateAsync so it always runs
  for reconnect-capable connections regardless of EnableAuthenticationRefresh.
  A legitimate same-user token refresh keeps the same identity key and still
  flows through the authentication-refresh path.
- Add an identity check in ProcessRefresh before OnAuthenticationRefresh and
  UpdateUser, rejecting a different user with 403 'user_changed' via the JSON
  error contract without tearing down the connection.

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Copilot AI review requested due to automatic review settings July 24, 2026 18:45
@BrennanConroy
BrennanConroy requested a review from halter73 as a code owner July 24, 2026 18:45
@BrennanConroy BrennanConroy added the area-signalr Includes: SignalR clients and servers label Jul 24, 2026

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 closes an identity-bypass gap in SignalR reconnect-capable transports when EnableAuthenticationRefresh is enabled, ensuring a stolen connection token can’t be used by a different authenticated user to attach to an existing connection’s stream.

Changes:

  • Add an explicit identity-key comparison on the /refresh endpoint (before OnAuthenticationRefresh / UpdateUser) and return a JSON 403 error (user_changed) without tearing down the existing connection.
  • Ensure the transport-layer “user changed” rejection runs for reconnect-capable requests (Long Polling / stateful WebSocket reconnect) regardless of EnableAuthenticationRefresh.
  • Expand test coverage for /refresh, Long Polling, and stateful reconnect scenarios where a different user attempts to reuse a connection token.
Show a summary per file
File Description
src/SignalR/common/Http.Connections/test/HttpConnectionDispatcherTests.AuthenticationRefresh.cs Adds targeted regression tests ensuring different-user refresh/reconnect attempts are rejected while preserving the original connection.
src/SignalR/common/Http.Connections/src/Internal/HttpConnectionDispatcher.cs Enforces identity mismatch rejection for /refresh and reconnect-capable requests even when authentication refresh is enabled.

Copilot's findings

  • Files reviewed: 2/2 changed files
  • Comments generated: 0

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area-signalr Includes: SignalR clients and servers

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants