From 1a546034ed560bd9d5ef45f602ddf36336f5f09b Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Tue, 3 Mar 2026 21:43:42 +0000 Subject: [PATCH 1/7] Initial plan From 1290c4a7465c18b84d1866136df0ed0ca2ba9dc6 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Tue, 3 Mar 2026 21:51:45 +0000 Subject: [PATCH 2/7] Fix SignalR JS: reset _accessToken on reconnect to prevent Azure SignalR JWT leaking to app server Co-authored-by: BrennanConroy <7574801+BrennanConroy@users.noreply.github.com> --- .../clients/ts/signalr/src/HttpConnection.ts | 1 + .../ts/signalr/tests/HttpConnection.test.ts | 91 +++++++++++++++++++ 2 files changed, 92 insertions(+) diff --git a/src/SignalR/clients/ts/signalr/src/HttpConnection.ts b/src/SignalR/clients/ts/signalr/src/HttpConnection.ts index f8397f4d65b5..f29ccb52dc46 100644 --- a/src/SignalR/clients/ts/signalr/src/HttpConnection.ts +++ b/src/SignalR/clients/ts/signalr/src/HttpConnection.ts @@ -227,6 +227,7 @@ export class HttpConnection implements IConnection { // as part of negotiating let url = this.baseUrl; this._accessTokenFactory = this._options.accessTokenFactory; + this._httpClient._accessToken = undefined; this._httpClient._accessTokenFactory = this._accessTokenFactory; try { diff --git a/src/SignalR/clients/ts/signalr/tests/HttpConnection.test.ts b/src/SignalR/clients/ts/signalr/tests/HttpConnection.test.ts index 76ff38e3357f..c84028c0ec9d 100644 --- a/src/SignalR/clients/ts/signalr/tests/HttpConnection.test.ts +++ b/src/SignalR/clients/ts/signalr/tests/HttpConnection.test.ts @@ -729,6 +729,97 @@ describe("HttpConnection", () => { }); }); + it("does not send access token from redirect negotiate to original server on reconnect", async () => { + await VerifyLogger.run(async (logger) => { + let negotiateCount = 0; + let firstPoll = true; + const pollingPromiseSource = new PromiseSource(); + const httpClient = new TestHttpClient() + .on("POST", /\/negotiate/, (r) => { + negotiateCount++; + if (negotiateCount === 1) { + // First negotiate to app server - redirect to Azure SignalR with a token + return { url: "https://azure.signalr.net/client/", accessToken: "azureSignalRToken" }; + } + + if (negotiateCount === 2) { + // Second negotiate to Azure SignalR service (redirect target) - succeeds + return { + availableTransports: [{ transport: "LongPolling", transferFormats: ["Text"] }], + connectionId: "0rge0d00-0040-0030-0r00-000q00r00e00", + }; + } + + if (negotiateCount === 3) { + // Third negotiate is a reconnect to the original app server + // It should NOT include the Azure SignalR token + if (r.headers && r.headers.Authorization === "Bearer azureSignalRToken") { + fail("Reconnect negotiate to app server must not include the Azure SignalR access token."); + } + return { url: "https://azure.signalr.net/client/", accessToken: "azureSignalRToken2" }; + } + + // Fourth negotiate to Azure SignalR service on reconnect + return { + availableTransports: [{ transport: "LongPolling", transferFormats: ["Text"] }], + connectionId: "1rge0d00-0040-0030-0r00-000q00r00e01", + }; + }) + .on("GET", async (r) => { + if (firstPoll) { + firstPoll = false; + return ""; + } + await pollingPromiseSource.promise; + return new HttpResponse(204, "No Content", ""); + }) + .on("DELETE", () => new HttpResponse(202)); + + const options: IHttpConnectionOptions = { + ...commonOptions, + accessTokenFactory: () => "appToken", + httpClient, + logger, + transport: HttpTransportType.LongPolling, + } as IHttpConnectionOptions; + + const connection = new HttpConnection("http://tempuri.org", options); + try { + await connection.start(TransferFormat.Text); + + // Verify first connection negotiated through the redirect + expect(negotiateCount).toBe(2); + + pollingPromiseSource.resolve(); + } finally { + await connection.stop(); + } + + // Reconnect - start again + firstPoll = true; + const pollingPromiseSource2 = new PromiseSource(); + httpClient.on("GET", async (r) => { + if (firstPoll) { + firstPoll = false; + return ""; + } + await pollingPromiseSource2.promise; + return new HttpResponse(204, "No Content", ""); + }); + + try { + await connection.start(TransferFormat.Text); + + // Verify reconnect also went through redirect (4 total negotiates) + expect(negotiateCount).toBe(4); + + pollingPromiseSource2.resolve(); + } finally { + await connection.stop(); + } + }); + }); + it("throws error if negotiate response has error", async () => { await VerifyLogger.run(async (logger) => { const httpClient = new TestHttpClient() From 8abd216f5ddddb79a751126cb13c035e2c1c8b53 Mon Sep 17 00:00:00 2001 From: Brennan Conroy Date: Tue, 3 Mar 2026 16:12:07 -0800 Subject: [PATCH 3/7] update --- eng/scripts/CodeCheck.ps1 | 2 +- src/SignalR/clients/ts/signalr/tests/HttpConnection.test.ts | 3 ++- 2 files changed, 3 insertions(+), 2 deletions(-) diff --git a/eng/scripts/CodeCheck.ps1 b/eng/scripts/CodeCheck.ps1 index f3ef5d320f6a..daab0b0a19b9 100644 --- a/eng/scripts/CodeCheck.ps1 +++ b/eng/scripts/CodeCheck.ps1 @@ -217,7 +217,7 @@ try { # Only enforce changelog rule if there are relevant TS changes if ($tsChanges.Count -gt 0 -and -not $changelogChanged) { # Check if the override marker exists in recent commit messages - $hasOverride = git log origin/$targetBranch..HEAD --pretty=%B | Select-String -Pattern $signalrChangelogOverrideMarker -Quiet + $hasOverride = git log origin/$targetBranch..HEAD --pretty=%B | Select-String -Pattern $signalrChangelogOverrideMarker -Quiet -SimpleMatch if (-not $hasOverride) { LogError "Changes were made to 'src/SignalR/clients/ts/', but no update to 'CHANGELOG.md' was found." diff --git a/src/SignalR/clients/ts/signalr/tests/HttpConnection.test.ts b/src/SignalR/clients/ts/signalr/tests/HttpConnection.test.ts index c84028c0ec9d..3787327c86bf 100644 --- a/src/SignalR/clients/ts/signalr/tests/HttpConnection.test.ts +++ b/src/SignalR/clients/ts/signalr/tests/HttpConnection.test.ts @@ -777,7 +777,8 @@ describe("HttpConnection", () => { const options: IHttpConnectionOptions = { ...commonOptions, - accessTokenFactory: () => "appToken", + // explicitly do not provide an access token factory to verify we correctly reset state + // accessTokenFactory: () => null, httpClient, logger, transport: HttpTransportType.LongPolling, From ff57922e13b08be291a5e5ea279a300c725d80a5 Mon Sep 17 00:00:00 2001 From: Brennan Conroy Date: Tue, 3 Mar 2026 16:29:50 -0800 Subject: [PATCH 4/7] update changelog --- src/SignalR/clients/ts/CHANGELOG.md | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/src/SignalR/clients/ts/CHANGELOG.md b/src/SignalR/clients/ts/CHANGELOG.md index 6fe59d38a6e4..7691922b7035 100644 --- a/src/SignalR/clients/ts/CHANGELOG.md +++ b/src/SignalR/clients/ts/CHANGELOG.md @@ -1,5 +1,9 @@ Change log contains changes for both @microsoft/signalr and @microsoft/signalr-protocol-msgpack. +## v11.0.0-preview.3 + +- Don't send Azure SignalR Service access token to app server on reconnect when accessTokenFactory is null. [#65638](https://github.com/dotnet/aspnetcore/pull/65638) + ## v10.0.0-rc.1 - Implemented fix for correctly reporting retries in the SignalR TS client. [#62812](https://github.com/dotnet/aspnetcore/pull/62812) From 493ce773198a6606d3196c41e5252a3e51e11980 Mon Sep 17 00:00:00 2001 From: Brennan Conroy Date: Wed, 20 May 2026 17:52:44 -0700 Subject: [PATCH 5/7] Address PR feedback: refactor token plumbing, update changelog, clean up test Addresses review feedback on #65638: - Pass redirect-scoped access token explicitly through _createTransport/_constructTransport instead of relying on stale mutable state on HttpConnection / AccessTokenHttpClient - Drop the duplicate HttpConnection._accessTokenFactory field; rely on _httpClient - Reword CHANGELOG entry to describe the actual scenario and bump to preview.5 - Replace dead commented-out option in test with a real explanatory comment Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- src/SignalR/clients/ts/CHANGELOG.md | 4 +- .../clients/ts/signalr/src/HttpConnection.ts | 48 +++++++++++-------- .../ts/signalr/tests/HttpConnection.test.ts | 7 ++- 3 files changed, 35 insertions(+), 24 deletions(-) diff --git a/src/SignalR/clients/ts/CHANGELOG.md b/src/SignalR/clients/ts/CHANGELOG.md index 7691922b7035..6e9ca743d870 100644 --- a/src/SignalR/clients/ts/CHANGELOG.md +++ b/src/SignalR/clients/ts/CHANGELOG.md @@ -1,8 +1,8 @@ Change log contains changes for both @microsoft/signalr and @microsoft/signalr-protocol-msgpack. -## v11.0.0-preview.3 +## v11.0.0-preview.5 -- Don't send Azure SignalR Service access token to app server on reconnect when accessTokenFactory is null. [#65638](https://github.com/dotnet/aspnetcore/pull/65638) +- Don't send a redirect-scoped Azure SignalR Service access token obtained from a negotiate redirect to the original app server on subsequent start/reconnect. [#65638](https://github.com/dotnet/aspnetcore/pull/65638) ## v10.0.0-rc.1 diff --git a/src/SignalR/clients/ts/signalr/src/HttpConnection.ts b/src/SignalR/clients/ts/signalr/src/HttpConnection.ts index f29ccb52dc46..5a56000aa19e 100644 --- a/src/SignalR/clients/ts/signalr/src/HttpConnection.ts +++ b/src/SignalR/clients/ts/signalr/src/HttpConnection.ts @@ -57,7 +57,6 @@ export class HttpConnection implements IConnection { private _stopPromise?: Promise; private _stopPromiseResolver: (value?: PromiseLike) => void = () => {}; private _stopError?: Error; - private _accessTokenFactory?: () => string | Promise; private _sendQueue?: TransportSendQueue; public readonly features: any = {}; @@ -223,18 +222,18 @@ export class HttpConnection implements IConnection { } private async _startInternal(transferFormat: TransferFormat): Promise { - // Store the original base url and the access token factory since they may change - // as part of negotiating + // Reset http client auth state for this start attempt. A previous start may have + // cached a redirect-scoped access token; that token must not be reused for the + // initial /negotiate request against the original (app) server. let url = this.baseUrl; - this._accessTokenFactory = this._options.accessTokenFactory; this._httpClient._accessToken = undefined; - this._httpClient._accessTokenFactory = this._accessTokenFactory; + this._httpClient._accessTokenFactory = this._options.accessTokenFactory; try { if (this._options.skipNegotiation) { if (this._options.transport === HttpTransportType.WebSockets) { // No need to add a connection ID in this case - this.transport = this._constructTransport(HttpTransportType.WebSockets); + this.transport = this._constructTransport(HttpTransportType.WebSockets, undefined); // We should just call connect directly in this case. // No fallback or negotiate in this case. await this._startTransport(url, transferFormat); @@ -244,6 +243,10 @@ export class HttpConnection implements IConnection { } else { let negotiateResponse: INegotiateResponse | null = null; let redirects = 0; + // Tracks an access token returned by a negotiate redirect. It is scoped to the + // redirect target (e.g. Azure SignalR Service) and is bound to the transport + // created from this start attempt, never leaking back to the original server. + let redirectAccessToken: string | undefined; do { negotiateResponse = await this._getNegotiationResponse(url); @@ -265,12 +268,11 @@ export class HttpConnection implements IConnection { } if (negotiateResponse.accessToken) { - // Replace the current access token factory with one that uses - // the returned access token - const accessToken = negotiateResponse.accessToken; - this._accessTokenFactory = () => accessToken; - // set the factory to undefined so the AccessTokenHttpClient won't retry with the same token, since we know it won't change until a connection restart - this._httpClient._accessToken = accessToken; + redirectAccessToken = negotiateResponse.accessToken; + // Set the cached token on the http client so the next /negotiate in the + // redirect loop carries it. Clear the factory so AccessTokenHttpClient + // won't retry with the same token - we know it won't change until restart. + this._httpClient._accessToken = redirectAccessToken; this._httpClient._accessTokenFactory = undefined; } @@ -282,7 +284,7 @@ export class HttpConnection implements IConnection { throw new Error("Negotiate redirection limit exceeded."); } - await this._createTransport(url, this._options.transport, negotiateResponse, transferFormat); + await this._createTransport(url, this._options.transport, negotiateResponse, transferFormat, redirectAccessToken); } if (this.transport instanceof LongPollingTransport) { @@ -362,7 +364,7 @@ export class HttpConnection implements IConnection { return url + (url.indexOf("?") === -1 ? "?" : "&") + `id=${connectionToken}`; } - private async _createTransport(url: string, requestedTransport: HttpTransportType | ITransport | undefined, negotiateResponse: INegotiateResponse, requestedTransferFormat: TransferFormat): Promise { + private async _createTransport(url: string, requestedTransport: HttpTransportType | ITransport | undefined, negotiateResponse: INegotiateResponse, requestedTransferFormat: TransferFormat, redirectAccessToken: string | undefined): Promise { let connectUrl = this._createConnectUrl(url, negotiateResponse.connectionToken); if (this._isITransport(requestedTransport)) { this._logger.log(LogLevel.Debug, "Connection was provided an instance of ITransport, using that directly."); @@ -378,7 +380,7 @@ export class HttpConnection implements IConnection { let negotiate: INegotiateResponse | undefined = negotiateResponse; for (const endpoint of transports) { const transportOrError = this._resolveTransportOrError(endpoint, requestedTransport, requestedTransferFormat, - negotiate?.useStatefulReconnect === true); + negotiate?.useStatefulReconnect === true, redirectAccessToken); if (transportOrError instanceof Error) { // Store the error and continue, we don't want to cause a re-negotiate in these cases transportExceptions.push(`${endpoint.transport} failed:`); @@ -417,19 +419,25 @@ export class HttpConnection implements IConnection { return Promise.reject(new Error("None of the transports supported by the client are supported by the server.")); } - private _constructTransport(transport: HttpTransportType): ITransport { + private _constructTransport(transport: HttpTransportType, redirectAccessToken: string | undefined): ITransport { switch (transport) { case HttpTransportType.WebSockets: if (!this._options.WebSocket) { throw new Error("'WebSocket' is not supported in your environment."); } - return new WebSocketTransport(this._httpClient, this._accessTokenFactory, this._logger, this._options.logMessageContent!, + // If the negotiate redirect returned an access token, use a closure that returns it + // (matching the behavior expected by AccessTokenHttpClient). Otherwise, fall through + // to the original options factory. + const wsAccessTokenFactory = redirectAccessToken !== undefined + ? () => redirectAccessToken + : this._options.accessTokenFactory; + return new WebSocketTransport(this._httpClient, wsAccessTokenFactory, this._logger, this._options.logMessageContent!, this._options.WebSocket, this._options.headers || {}); case HttpTransportType.ServerSentEvents: if (!this._options.EventSource) { throw new Error("'EventSource' is not supported in your environment."); } - return new ServerSentEventsTransport(this._httpClient, this._httpClient._accessToken, this._logger, this._options); + return new ServerSentEventsTransport(this._httpClient, redirectAccessToken, this._logger, this._options); case HttpTransportType.LongPolling: return new LongPollingTransport(this._httpClient, this._logger, this._options); default: @@ -466,7 +474,7 @@ export class HttpConnection implements IConnection { } private _resolveTransportOrError(endpoint: IAvailableTransport, requestedTransport: HttpTransportType | undefined, - requestedTransferFormat: TransferFormat, useStatefulReconnect: boolean): ITransport | Error | unknown { + requestedTransferFormat: TransferFormat, useStatefulReconnect: boolean, redirectAccessToken: string | undefined): ITransport | Error | unknown { const transport = HttpTransportType[endpoint.transport]; if (transport === null || transport === undefined) { this._logger.log(LogLevel.Debug, `Skipping transport '${endpoint.transport}' because it is not supported by this client.`); @@ -483,7 +491,7 @@ export class HttpConnection implements IConnection { this._logger.log(LogLevel.Debug, `Selecting transport '${HttpTransportType[transport]}'.`); try { this.features.reconnect = transport === HttpTransportType.WebSockets ? useStatefulReconnect : undefined; - return this._constructTransport(transport); + return this._constructTransport(transport, redirectAccessToken); } catch (ex) { return ex; } diff --git a/src/SignalR/clients/ts/signalr/tests/HttpConnection.test.ts b/src/SignalR/clients/ts/signalr/tests/HttpConnection.test.ts index 3787327c86bf..f436d2a60721 100644 --- a/src/SignalR/clients/ts/signalr/tests/HttpConnection.test.ts +++ b/src/SignalR/clients/ts/signalr/tests/HttpConnection.test.ts @@ -777,8 +777,11 @@ describe("HttpConnection", () => { const options: IHttpConnectionOptions = { ...commonOptions, - // explicitly do not provide an access token factory to verify we correctly reset state - // accessTokenFactory: () => null, + // Intentionally omit accessTokenFactory: without the fix, providing + // `accessTokenFactory: () => null` would mask the bug because the factory + // would overwrite the stale _accessToken on the next negotiate. The bug + // surfaces when no factory is set and the only token in play came from a + // negotiate redirect on a previous start. httpClient, logger, transport: HttpTransportType.LongPolling, From 27643bed658ecea30cdbe0e7d5536e8930d23713 Mon Sep 17 00:00:00 2001 From: Brennan Conroy Date: Thu, 21 May 2026 15:34:44 -0700 Subject: [PATCH 6/7] Fix SSE auth regression: fall back to httpClient cached token The refactor in 493ce77 changed _constructTransport's SSE branch to pass only redirectAccessToken to ServerSentEventsTransport. When there's no negotiate redirect, redirectAccessToken is undefined but the user's accessTokenFactory has already populated _httpClient._accessToken during the initial /negotiate. The original code passed _httpClient._accessToken so SSE got the user's token in the access_token query string. Restore that fallback so the TypeScript functional test 'hubConnection over ServerSentEvents transport can connect to hub with authorization' passes again. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- src/SignalR/clients/ts/signalr/src/HttpConnection.ts | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/src/SignalR/clients/ts/signalr/src/HttpConnection.ts b/src/SignalR/clients/ts/signalr/src/HttpConnection.ts index 5a56000aa19e..21fad302202e 100644 --- a/src/SignalR/clients/ts/signalr/src/HttpConnection.ts +++ b/src/SignalR/clients/ts/signalr/src/HttpConnection.ts @@ -437,7 +437,11 @@ export class HttpConnection implements IConnection { if (!this._options.EventSource) { throw new Error("'EventSource' is not supported in your environment."); } - return new ServerSentEventsTransport(this._httpClient, redirectAccessToken, this._logger, this._options); + // Fall back to the token cached on the http client (populated from + // accessTokenFactory during the initial /negotiate) when no redirect + // token was returned, so SSE continues to authenticate with the user's + // access token in the access_token query string. + return new ServerSentEventsTransport(this._httpClient, redirectAccessToken ?? this._httpClient._accessToken, this._logger, this._options); case HttpTransportType.LongPolling: return new LongPollingTransport(this._httpClient, this._logger, this._options); default: From 6beba6bf74a18227b9f82715d374a2a5c05d132d Mon Sep 17 00:00:00 2001 From: Brennan Conroy Date: Thu, 21 May 2026 21:02:32 -0700 Subject: [PATCH 7/7] Bump changelog to preview.6 and brace WebSockets case block Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- src/SignalR/clients/ts/CHANGELOG.md | 2 +- src/SignalR/clients/ts/signalr/src/HttpConnection.ts | 5 +++-- 2 files changed, 4 insertions(+), 3 deletions(-) diff --git a/src/SignalR/clients/ts/CHANGELOG.md b/src/SignalR/clients/ts/CHANGELOG.md index 6e9ca743d870..bcf87cd6e101 100644 --- a/src/SignalR/clients/ts/CHANGELOG.md +++ b/src/SignalR/clients/ts/CHANGELOG.md @@ -1,6 +1,6 @@ Change log contains changes for both @microsoft/signalr and @microsoft/signalr-protocol-msgpack. -## v11.0.0-preview.5 +## v11.0.0-preview.6 - Don't send a redirect-scoped Azure SignalR Service access token obtained from a negotiate redirect to the original app server on subsequent start/reconnect. [#65638](https://github.com/dotnet/aspnetcore/pull/65638) diff --git a/src/SignalR/clients/ts/signalr/src/HttpConnection.ts b/src/SignalR/clients/ts/signalr/src/HttpConnection.ts index 21fad302202e..6123d86ab95e 100644 --- a/src/SignalR/clients/ts/signalr/src/HttpConnection.ts +++ b/src/SignalR/clients/ts/signalr/src/HttpConnection.ts @@ -421,18 +421,19 @@ export class HttpConnection implements IConnection { private _constructTransport(transport: HttpTransportType, redirectAccessToken: string | undefined): ITransport { switch (transport) { - case HttpTransportType.WebSockets: + case HttpTransportType.WebSockets: { if (!this._options.WebSocket) { throw new Error("'WebSocket' is not supported in your environment."); } // If the negotiate redirect returned an access token, use a closure that returns it // (matching the behavior expected by AccessTokenHttpClient). Otherwise, fall through - // to the original options factory. + // to the original options factory so it can be re-invoked on token refresh. const wsAccessTokenFactory = redirectAccessToken !== undefined ? () => redirectAccessToken : this._options.accessTokenFactory; return new WebSocketTransport(this._httpClient, wsAccessTokenFactory, this._logger, this._options.logMessageContent!, this._options.WebSocket, this._options.headers || {}); + } case HttpTransportType.ServerSentEvents: if (!this._options.EventSource) { throw new Error("'EventSource' is not supported in your environment.");