From 873a1132e89e56e4e3349d43cbf19caa40a63b17 Mon Sep 17 00:00:00 2001 From: Danilo Neto Date: Fri, 19 Sep 2025 11:39:20 -0300 Subject: [PATCH 1/6] fix(SignalR): ensure 401 triggers access token refresh retry in TS client - Handle both HttpError.status and HttpResponse.statusCode - Normalize string "401" to number before comparison - Retry exactly once using _refreshTokenAndRetry - Skip retry when token factory returns empty - Preserve existing behavior for non-401 errors - Add unit tests for 401 response, HttpError, string "401", no-retry cases Fixes #56494 --- .../ts/signalr/src/AccessTokenHttpClient.ts | 61 ++++- .../tests/AccessTokenHttpClient.test.ts | 248 ++++++++++++++++++ 2 files changed, 303 insertions(+), 6 deletions(-) create mode 100644 src/SignalR/clients/ts/signalr/tests/AccessTokenHttpClient.test.ts diff --git a/src/SignalR/clients/ts/signalr/src/AccessTokenHttpClient.ts b/src/SignalR/clients/ts/signalr/src/AccessTokenHttpClient.ts index e28c98862af6..27ffe16a5490 100644 --- a/src/SignalR/clients/ts/signalr/src/AccessTokenHttpClient.ts +++ b/src/SignalR/clients/ts/signalr/src/AccessTokenHttpClient.ts @@ -4,6 +4,22 @@ import { HeaderNames } from "./HeaderNames"; import { HttpClient, HttpRequest, HttpResponse } from "./HttpClient"; +// Internal helpers (not exported) for narrowing and status normalization +function isError(u: unknown): u is Error { + return u instanceof Error; +} +function getStatus(u: unknown): number | undefined { + if (typeof u !== "object" || u === null) { return undefined; } + const rec = u as Record; + const raw = rec["statusCode"] ?? rec["status"]; + if (typeof raw === "number") { return raw; } + if (typeof raw === "string") { + const n = parseInt(raw, 10); + return Number.isNaN(n) ? undefined : n; + } + return undefined; +} + /** @private */ export class AccessTokenHttpClient extends HttpClient { private _innerClient: HttpClient; @@ -25,14 +41,47 @@ export class AccessTokenHttpClient extends HttpClient { this._accessToken = await this._accessTokenFactory(); } this._setAuthorizationHeader(request); - const response = await this._innerClient.send(request); - if (allowRetry && response.statusCode === 401 && this._accessTokenFactory) { - this._accessToken = await this._accessTokenFactory(); - this._setAuthorizationHeader(request); - return await this._innerClient.send(request); + try { + const response = await this._innerClient.send(request); + + if (allowRetry && this._accessTokenFactory && response.statusCode === 401) { + return await this._refreshTokenAndRetry(request, response); + } + return response; + } catch (err: unknown) { + if (!allowRetry || !this._accessTokenFactory) { + throw err; + } + if (!isError(err)) { + throw err; + } + const status = getStatus(err); + if (status === 401) { + return await this._refreshTokenAndRetry(request, err); + } + throw err; } - return response; + } + + private async _refreshTokenAndRetry(request: HttpRequest, original: HttpResponse | Error): Promise { + if (!this._accessTokenFactory) { + if (original instanceof HttpResponse) { + return original; + } + throw original; + } + + const newToken = await this._accessTokenFactory(); + if (!newToken) { + if (original instanceof HttpResponse) { + return original; + } + throw original; + } + this._accessToken = newToken; + this._setAuthorizationHeader(request); + return await this._innerClient.send(request); } private _setAuthorizationHeader(request: HttpRequest) { diff --git a/src/SignalR/clients/ts/signalr/tests/AccessTokenHttpClient.test.ts b/src/SignalR/clients/ts/signalr/tests/AccessTokenHttpClient.test.ts new file mode 100644 index 000000000000..37fa5b01ac86 --- /dev/null +++ b/src/SignalR/clients/ts/signalr/tests/AccessTokenHttpClient.test.ts @@ -0,0 +1,248 @@ +// Licensed to the .NET Foundation under one or more agreements. +// The .NET Foundation licenses this file to you under the MIT license. + +import { AccessTokenHttpClient } from "../src/AccessTokenHttpClient"; +import { HttpError } from "../src/Errors"; +import { HttpRequest, HttpResponse } from "../src/HttpClient"; +import { TestHttpClient } from "./TestHttpClient"; +import { registerUnhandledRejectionHandler } from "./Utils"; +import { VerifyLogger } from "./Common"; +import { LongPollingTransport } from "../src/LongPollingTransport"; +import { TransferFormat } from "../src/ITransport"; + +describe("AccessTokenHttpClient", () => { + beforeAll(() => { + registerUnhandledRejectionHandler(); + }); + + afterAll(() => { + // Optional cleanup could go here. + }); + + it("retries exactly once on 401 HttpError when accessTokenFactory provided", async () => { + let call = 0; + let primed = false; + const inner = new TestHttpClient(); + inner.on(() => { + if (!primed) { + primed = true; // prime request returns 200 and sets initial token + return new HttpResponse(200, "OK", "prime"); + } + call++; + if (call === 1) { + throw new HttpError("Unauthorized", 401); + } + return new HttpResponse(200, "OK", "done"); + }); + + let factoryCalls = 0; + const client = new AccessTokenHttpClient(inner, () => { + factoryCalls++; + return `token${factoryCalls}`; + }); + + // Prime token via public API + await client.get("http://example.com/prime"); + + const response = await client.get("http://example.com/resource"); + expect(response.statusCode).toBe(200); + expect(factoryCalls).toBe(2); // prime + retry refresh + expect(call).toBe(2); // failing attempt + successful retry + }); + + [403, 500].forEach(status => { + it(`does not retry on status ${status} HttpError`, async () => { + let primed = false; + let failingCalls = 0; + const inner = new TestHttpClient(); + inner.on(() => { + if (!primed) { + primed = true; + return new HttpResponse(200, "OK", "prime"); + } + failingCalls++; + throw new HttpError("Error", status); + }); + + let factoryCalls = 0; + const client = new AccessTokenHttpClient(inner, () => { + factoryCalls++; + return `token${factoryCalls}`; + }); + + await client.get("http://example.com/prime"); + try { + await client.get("http://example.com/resource"); + fail("expected to throw"); + } catch (e: any) { + expect(e).toBeInstanceOf(HttpError); + expect(e.statusCode ?? e.status).toBe(status); + } + expect(factoryCalls).toBe(1); + expect(failingCalls).toBe(1); + }); + }); + + it("LongPollingTransport continues running after 401 during poll and refreshes token", async () => { + await VerifyLogger.run(async (logger) => { + let pollIteration = 0; + let primed = false; + const tokens: string[] = []; + const accessTokenFactory = () => { + const t = `tok${tokens.length + 1}`; + tokens.push(t); + return t; + }; + const httpClient = new AccessTokenHttpClient(new TestHttpClient() + .on("GET", (r: HttpRequest) => { + // Prime request separate from polling loop + if (!primed && r.url!.includes("/prime")) { + primed = true; + return new HttpResponse(200, "OK", "prime"); + } + pollIteration++; + if (pollIteration === 1) { // initial connect poll + return new HttpResponse(200, "OK", ""); + } + if (pollIteration === 2) { // trigger 401 -> retry + return new HttpResponse(401); + } + if (pollIteration === 3) { // post-refresh poll + expect(r.headers).toBeDefined(); + expect(r.headers?.Authorization).toBeDefined(); + expect(r.headers?.Authorization).toContain(tokens[tokens.length - 1]); + return new HttpResponse(204); + } + return new HttpResponse(204); + }), accessTokenFactory); + + // Prime token using public API + await httpClient.get("http://example.com/prime"); + + const transport = new LongPollingTransport(httpClient, logger, { withCredentials: true, headers: {}, logMessageContent: false }); + await transport.connect("http://example.com?connectionId=abc", TransferFormat.Text); + await transport.stop(); + + expect(tokens.length).toBe(2); // primed + refreshed + expect(pollIteration).toBeGreaterThanOrEqual(3); + }); + }); + + it("retries once on 401 HttpResponse status (non-throwing path)", async () => { + let primed = false; + let attempts = 0; + let retryAuthHeader: string | undefined; + const inner = new TestHttpClient(); + inner.on((r: HttpRequest) => { + if (!primed && r.url!.includes("/prime")) { + primed = true; + return new HttpResponse(200, "OK", "prime"); + } + attempts++; + if (attempts === 1) { + return new HttpResponse(401); + } + // second attempt after refresh + retryAuthHeader = r.headers?.Authorization; + return new HttpResponse(200, "OK", "after-retry"); + }); + + let factoryCalls = 0; + const client = new AccessTokenHttpClient(inner, () => { + factoryCalls++; + return `token${factoryCalls}`; + }); + + await client.get("http://example.com/prime"); + const resp = await client.get("http://example.com/resource"); + expect(resp.statusCode).toBe(200); + expect(factoryCalls).toBe(2); // prime + refresh + expect(attempts).toBe(2); // original 401 + retry 200 + expect(retryAuthHeader).toContain("token2"); + }); + + it("does not retry when allowRetry is false (initial token acquisition)", async () => { + let sends = 0; + const inner = new TestHttpClient(); + inner.on(() => { + sends++; + return new HttpResponse(401); + }); + + let factoryCalls = 0; + const client = new AccessTokenHttpClient(inner, () => { + factoryCalls++; + return `token${factoryCalls}`; // Explicitly call send with allowRetry=false to ensure no retry is attempted. + }); + + const request: HttpRequest = { method: "GET", url: "http://example.com/resource" }; + const resp = await client.send(request); // send path with existing logic; allowRetry=false triggered by initial token acquisition above + expect(resp.statusCode).toBe(401); + expect(factoryCalls).toBe(1); + expect(sends).toBe(1); + }); + + it("does not retry when refreshed token is empty", async () => { + let primed = false; + let attempts = 0; + const inner = new TestHttpClient(); + inner.on((r: HttpRequest) => { + if (!primed && r.url!.includes("/prime")) { + primed = true; + return new HttpResponse(200, "OK", "prime"); + } + attempts++; + return new HttpResponse(401); // cause retry path + }); + + let factoryCalls = 0; + const client = new AccessTokenHttpClient(inner, () => { + factoryCalls++; + if (factoryCalls === 1) { + return "tok1"; // prime + } + return ""; // refresh returns empty -> should not retry send again + }); + + await client.get("http://example.com/prime"); + const resp = await client.get("http://example.com/resource"); + expect(resp.statusCode).toBe(401); // original response returned + expect(factoryCalls).toBe(2); // prime + attempted refresh + expect(attempts).toBe(1); // no second send + }); + + it("retries once when HttpError.status is string '401'", async () => { + let primed = false; + let attempt = 0; + let retryAuth: string | undefined; + const inner = new TestHttpClient(); + inner.on((r: HttpRequest) => { + if (!primed && r.url!.includes("/prime")) { + primed = true; + return new HttpResponse(200, "OK", "prime"); + } + attempt++; + if (attempt === 1) { + const err: any = new Error("Unauthorized: Status code '401'"); + err.name = "HttpError"; // mimic HttpError shape without statusCode + err.status = "401"; // string status to trigger normalization path + throw err; + } + retryAuth = r.headers?.Authorization; + return new HttpResponse(200, "OK", "ok"); + }); + + let factoryCalls = 0; + const client = new AccessTokenHttpClient(inner, () => { + factoryCalls++; + return `token${factoryCalls}`; + }); + + await client.get("http://example.com/prime"); + const resp = await client.get("http://example.com/resource"); + expect(resp.statusCode).toBe(200); + expect(factoryCalls).toBe(2); // prime + refresh after string status retry + expect(attempt).toBe(2); // original throw + retry + expect(retryAuth).toContain("token2"); + }); +}); From 857315ca79116b1f3eb2dd9d25f428adfa701b5f Mon Sep 17 00:00:00 2001 From: Danilo Neto Date: Fri, 19 Sep 2025 11:54:27 -0300 Subject: [PATCH 2/6] Update src/SignalR/clients/ts/signalr/src/AccessTokenHttpClient.ts Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> --- src/SignalR/clients/ts/signalr/src/AccessTokenHttpClient.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/SignalR/clients/ts/signalr/src/AccessTokenHttpClient.ts b/src/SignalR/clients/ts/signalr/src/AccessTokenHttpClient.ts index 27ffe16a5490..cf11a562ff39 100644 --- a/src/SignalR/clients/ts/signalr/src/AccessTokenHttpClient.ts +++ b/src/SignalR/clients/ts/signalr/src/AccessTokenHttpClient.ts @@ -4,7 +4,7 @@ import { HeaderNames } from "./HeaderNames"; import { HttpClient, HttpRequest, HttpResponse } from "./HttpClient"; -// Internal helpers (not exported) for narrowing and status normalization +// Internal helpers for Error type guarding and status normalization function isError(u: unknown): u is Error { return u instanceof Error; } From f88b82cf94c49aa6f5cbdb1c4a4b59ba736bbedc Mon Sep 17 00:00:00 2001 From: Danilo Neto Date: Fri, 19 Sep 2025 11:54:47 -0300 Subject: [PATCH 3/6] Update src/SignalR/clients/ts/signalr/tests/AccessTokenHttpClient.test.ts Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> --- .../clients/ts/signalr/tests/AccessTokenHttpClient.test.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/SignalR/clients/ts/signalr/tests/AccessTokenHttpClient.test.ts b/src/SignalR/clients/ts/signalr/tests/AccessTokenHttpClient.test.ts index 37fa5b01ac86..3ce28803e2b6 100644 --- a/src/SignalR/clients/ts/signalr/tests/AccessTokenHttpClient.test.ts +++ b/src/SignalR/clients/ts/signalr/tests/AccessTokenHttpClient.test.ts @@ -172,7 +172,7 @@ describe("AccessTokenHttpClient", () => { let factoryCalls = 0; const client = new AccessTokenHttpClient(inner, () => { factoryCalls++; - return `token${factoryCalls}`; // Explicitly call send with allowRetry=false to ensure no retry is attempted. + return `token${factoryCalls}`; // Token factory returns a token string for each call. }); const request: HttpRequest = { method: "GET", url: "http://example.com/resource" }; From 0b490cf2bb9604cf8620ad045b9f015f850f72e4 Mon Sep 17 00:00:00 2001 From: Danilo Neto Date: Fri, 19 Sep 2025 11:57:40 -0300 Subject: [PATCH 4/6] Update src/SignalR/clients/ts/signalr/src/AccessTokenHttpClient.ts Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> --- .../clients/ts/signalr/src/AccessTokenHttpClient.ts | 7 ------- 1 file changed, 7 deletions(-) diff --git a/src/SignalR/clients/ts/signalr/src/AccessTokenHttpClient.ts b/src/SignalR/clients/ts/signalr/src/AccessTokenHttpClient.ts index cf11a562ff39..6d440a7f2d1a 100644 --- a/src/SignalR/clients/ts/signalr/src/AccessTokenHttpClient.ts +++ b/src/SignalR/clients/ts/signalr/src/AccessTokenHttpClient.ts @@ -65,13 +65,6 @@ export class AccessTokenHttpClient extends HttpClient { } private async _refreshTokenAndRetry(request: HttpRequest, original: HttpResponse | Error): Promise { - if (!this._accessTokenFactory) { - if (original instanceof HttpResponse) { - return original; - } - throw original; - } - const newToken = await this._accessTokenFactory(); if (!newToken) { if (original instanceof HttpResponse) { From 372a70a75fc653fdd6b711d0767b85f15688500c Mon Sep 17 00:00:00 2001 From: Danilo Neto Date: Fri, 19 Sep 2025 11:58:21 -0300 Subject: [PATCH 5/6] Update src/SignalR/clients/ts/signalr/tests/AccessTokenHttpClient.test.ts Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> --- .../clients/ts/signalr/tests/AccessTokenHttpClient.test.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/SignalR/clients/ts/signalr/tests/AccessTokenHttpClient.test.ts b/src/SignalR/clients/ts/signalr/tests/AccessTokenHttpClient.test.ts index 3ce28803e2b6..b73352027590 100644 --- a/src/SignalR/clients/ts/signalr/tests/AccessTokenHttpClient.test.ts +++ b/src/SignalR/clients/ts/signalr/tests/AccessTokenHttpClient.test.ts @@ -73,7 +73,7 @@ describe("AccessTokenHttpClient", () => { await client.get("http://example.com/prime"); try { await client.get("http://example.com/resource"); - fail("expected to throw"); + expect.fail("expected to throw"); } catch (e: any) { expect(e).toBeInstanceOf(HttpError); expect(e.statusCode ?? e.status).toBe(status); From 75ca5743daca43aad0102757b2a3056afdacbce5 Mon Sep 17 00:00:00 2001 From: Danilo Neto Date: Fri, 19 Sep 2025 12:08:50 -0300 Subject: [PATCH 6/6] fix(SignalR/TS): 401 retry + no-token path cleans Authorization and resends; shrink bundle fix(SignalR): ensure access token factory is always called and improve error handling in tests --- .../ts/signalr/src/AccessTokenHttpClient.ts | 84 +++++-------------- .../tests/AccessTokenHttpClient.test.ts | 3 +- 2 files changed, 23 insertions(+), 64 deletions(-) diff --git a/src/SignalR/clients/ts/signalr/src/AccessTokenHttpClient.ts b/src/SignalR/clients/ts/signalr/src/AccessTokenHttpClient.ts index 6d440a7f2d1a..1abeb0831cf6 100644 --- a/src/SignalR/clients/ts/signalr/src/AccessTokenHttpClient.ts +++ b/src/SignalR/clients/ts/signalr/src/AccessTokenHttpClient.ts @@ -1,28 +1,12 @@ // Licensed to the .NET Foundation under one or more agreements. // The .NET Foundation licenses this file to you under the MIT license. -import { HeaderNames } from "./HeaderNames"; +// Removed HeaderNames import to reduce bundle size; using literal key. import { HttpClient, HttpRequest, HttpResponse } from "./HttpClient"; -// Internal helpers for Error type guarding and status normalization -function isError(u: unknown): u is Error { - return u instanceof Error; -} -function getStatus(u: unknown): number | undefined { - if (typeof u !== "object" || u === null) { return undefined; } - const rec = u as Record; - const raw = rec["statusCode"] ?? rec["status"]; - if (typeof raw === "number") { return raw; } - if (typeof raw === "string") { - const n = parseInt(raw, 10); - return Number.isNaN(n) ? undefined : n; - } - return undefined; -} - /** @private */ export class AccessTokenHttpClient extends HttpClient { - private _innerClient: HttpClient; + private readonly _innerClient: HttpClient; _accessToken: string | undefined; _accessTokenFactory: (() => string | Promise) | undefined; @@ -34,62 +18,38 @@ export class AccessTokenHttpClient extends HttpClient { } public async send(request: HttpRequest): Promise { - let allowRetry = true; - if (this._accessTokenFactory && (!this._accessToken || (request.url && request.url.indexOf("/negotiate?") > 0))) { - // don't retry if the request is a negotiate or if we just got a potentially new token from the access token factory - allowRetry = false; - this._accessToken = await this._accessTokenFactory(); - } + const needsToken = !!(this._accessTokenFactory && (!this._accessToken || (request.url && request.url.indexOf('/negotiate?') > 0))); + const retry = !needsToken; + if (needsToken) this._accessToken = await this._accessTokenFactory!(); this._setAuthorizationHeader(request); - try { - const response = await this._innerClient.send(request); - - if (allowRetry && this._accessTokenFactory && response.statusCode === 401) { - return await this._refreshTokenAndRetry(request, response); - } - return response; + const r = await this._innerClient.send(request); + return (retry && this._accessTokenFactory && r.statusCode === 401) ? this._refreshTokenAndRetry(request, r) : r; } catch (err: unknown) { - if (!allowRetry || !this._accessTokenFactory) { - throw err; - } - if (!isError(err)) { - throw err; - } - const status = getStatus(err); - if (status === 401) { - return await this._refreshTokenAndRetry(request, err); - } + if (!retry || !this._accessTokenFactory) throw err; + const e = err as any, s = +(e.statusCode ?? e.status); + if (s === 401) return this._refreshTokenAndRetry(request, e); throw err; } } - private async _refreshTokenAndRetry(request: HttpRequest, original: HttpResponse | Error): Promise { - const newToken = await this._accessTokenFactory(); - if (!newToken) { - if (original instanceof HttpResponse) { - return original; - } - throw original; + private async _refreshTokenAndRetry(request: HttpRequest, o: HttpResponse | Error): Promise { + const t = await this._accessTokenFactory!(); + if (!t) { + this._accessToken = undefined; + if (request.headers) delete (request.headers as any).Authorization; + if (request.abortSignal) return this._innerClient.send(request); + if (o instanceof HttpResponse) return o; + return Promise.reject(o); } - this._accessToken = newToken; + this._accessToken = t; this._setAuthorizationHeader(request); - return await this._innerClient.send(request); + return this._innerClient.send(request); } private _setAuthorizationHeader(request: HttpRequest) { - if (!request.headers) { - request.headers = {}; - } - if (this._accessToken) { - request.headers[HeaderNames.Authorization] = `Bearer ${this._accessToken}` - } - // don't remove the header if there isn't an access token factory, the user manually added the header in this case - else if (this._accessTokenFactory) { - if (request.headers[HeaderNames.Authorization]) { - delete request.headers[HeaderNames.Authorization]; - } - } + const h = request.headers || (request.headers = {}); + if (this._accessToken) h.Authorization = 'Bearer ' + this._accessToken; else if (this._accessTokenFactory) delete h.Authorization; } public getCookieString(url: string): string { diff --git a/src/SignalR/clients/ts/signalr/tests/AccessTokenHttpClient.test.ts b/src/SignalR/clients/ts/signalr/tests/AccessTokenHttpClient.test.ts index b73352027590..62f12910a1da 100644 --- a/src/SignalR/clients/ts/signalr/tests/AccessTokenHttpClient.test.ts +++ b/src/SignalR/clients/ts/signalr/tests/AccessTokenHttpClient.test.ts @@ -72,8 +72,7 @@ describe("AccessTokenHttpClient", () => { await client.get("http://example.com/prime"); try { - await client.get("http://example.com/resource"); - expect.fail("expected to throw"); + await expect(client.get("http://example.com/resource")).rejects.toThrow(HttpError); } catch (e: any) { expect(e).toBeInstanceOf(HttpError); expect(e.statusCode ?? e.status).toBe(status);