diff --git a/e2e/questdb b/e2e/questdb index 9b59a9211..9559b8d49 160000 --- a/e2e/questdb +++ b/e2e/questdb @@ -1 +1 @@ -Subproject commit 9b59a921165af573cedd22bf8b12613de19cb8bd +Subproject commit 9559b8d490c63bf6edad1054f3af9b2231761ad7 diff --git a/src/utils/questdb/client.test.ts b/src/utils/questdb/client.test.ts index a4be22e92..b2479cead 100644 --- a/src/utils/questdb/client.test.ts +++ b/src/utils/questdb/client.test.ts @@ -2,6 +2,8 @@ import "../../test/stubBrowserGlobals" import { afterEach, describe, expect, it, vi } from "vitest" import { Client } from "./client" import { Type } from "./types" +import { ssoAuthState } from "../../modules/OAuth2/ssoAuthState" +import { AuthPayload } from "../../modules/OAuth2/types" const response = (body: Record): Response => ({ @@ -62,3 +64,44 @@ describe("Client queryRaw NOTICE timings", () => { expect(result).not.toHaveProperty("timings") }) }) + +describe("Client token refresh", () => { + afterEach(() => { + ssoAuthState.clearAuthPayload() + }) + + it("does not deadlock when a token refresh fails at the transport level", async () => { + // Given an active SSO session whose token is inside the 30s refresh window + ssoAuthState.setAuthPayload({ + access_token: "stale", + refresh_token: "refresh", + expires_at: new Date(new Date().getTime() + 10_000).toString(), + } as AuthPayload) + + vi.stubGlobal( + "fetch", + vi.fn().mockResolvedValue(response({ notice: "hint applied" })), + ) + const errorSpy = vi.spyOn(console, "error").mockImplementation(() => {}) + + // And a refresh that rejects, e.g. the token endpoint is unreachable or + // answers with a non-JSON body + const client = new Client() + client.refreshTokenMethod = () => Promise.reject(new Error("network down")) + + // When a query runs, it must not hang waiting on a stuck refresh flag: the + // failure is swallowed and the request proceeds with the stale token (the + // server would then answer 401 and drive the normal re-auth flow). + const result = await client.queryRaw("SELECT 1") + + expect(result.type).toBe(Type.NOTICE) + expect(fetch).toHaveBeenCalledTimes(1) + expect(errorSpy).toHaveBeenCalled() + + // And a subsequent query still goes through — the flag was reset + await client.queryRaw("SELECT 1") + expect(fetch).toHaveBeenCalledTimes(2) + + errorSpy.mockRestore() + }) +}) diff --git a/src/utils/questdb/client.ts b/src/utils/questdb/client.ts index 165bcf05a..343e6ef81 100644 --- a/src/utils/questdb/client.ts +++ b/src/utils/questdb/client.ts @@ -61,26 +61,38 @@ export class Client { private refreshAuthToken = async () => { Client.refreshTokenPending = true - await new Promise((resolve) => { - const interval = setInterval(async () => { - if (Client.numOfPendingQueries === 0) { - clearInterval(interval) - const newToken = await this.refreshTokenMethod() - if (newToken.access_token) { - this.setCommonHeaders({ - ...this.commonHeaders, - Authorization: `Bearer ${ - newToken.groups_encoded_in_token - ? newToken.id_token - : newToken.access_token - }`, - }) + try { + // Wait until all in-flight queries have finished before swapping the auth + // header, so we don't change it out from under a pending request. + await new Promise((resolve) => { + const interval = setInterval(() => { + if (Client.numOfPendingQueries === 0) { + clearInterval(interval) + resolve() } - Client.refreshTokenPending = false - return resolve(true) - } - }, 50) - }) + }, 50) + }) + const newToken = await this.refreshTokenMethod() + if (newToken.access_token) { + this.setCommonHeaders({ + ...this.commonHeaders, + Authorization: `Bearer ${ + newToken.groups_encoded_in_token + ? newToken.id_token + : newToken.access_token + }`, + }) + } + } catch (error) { + // A transport-level failure (token endpoint unreachable, non-JSON + // response, etc.) must not leave refreshTokenPending stuck true, which + // would deadlock every subsequent query. We keep the stale token in + // place; the next request will get a 401 and drive the normal re-auth + // flow, matching what happens when there is no refresh token at all. + console.error("Failed to refresh the auth token", error) + } finally { + Client.refreshTokenPending = false + } } static encodeParams = (