From aabb9d81df9a70c38f517cbcb1b954be8bff0488 Mon Sep 17 00:00:00 2001 From: Charles Vien Date: Sat, 20 Jun 2026 15:49:12 -0700 Subject: [PATCH 1/2] fail fast to logout on invalid refresh token --- packages/core/src/oauth/oauth.test.ts | 30 ++++++++++++++++++++++++++- packages/core/src/oauth/oauth.ts | 25 ++++++++++++++++++++-- 2 files changed, 52 insertions(+), 3 deletions(-) diff --git a/packages/core/src/oauth/oauth.test.ts b/packages/core/src/oauth/oauth.test.ts index a742523fa4..7081267a32 100644 --- a/packages/core/src/oauth/oauth.test.ts +++ b/packages/core/src/oauth/oauth.test.ts @@ -122,6 +122,34 @@ describe("OAuthService.refreshToken", () => { expect(result.errorCode).toBe("auth_error"); }); + it("maps a 400 invalid_grant to an auth_error", async () => { + const { service } = createDeps(); + fetchMock.mockResolvedValue(jsonResponse({ error: "invalid_grant" }, 400)); + + const result = await service.refreshToken("rt", "us"); + + expect(result.success).toBe(false); + expect(result.errorCode).toBe("auth_error"); + }); + + it("maps a 400 invalid_client to an unknown_error, not a logout", async () => { + const { service } = createDeps(); + fetchMock.mockResolvedValue(jsonResponse({ error: "invalid_client" }, 400)); + + const result = await service.refreshToken("rt", "us"); + + expect(result.errorCode).toBe("unknown_error"); + }); + + it("maps a 400 with no parseable body to an unknown_error", async () => { + const { service } = createDeps(); + fetchMock.mockResolvedValue(new Response("", { status: 400 })); + + const result = await service.refreshToken("rt", "us"); + + expect(result.errorCode).toBe("unknown_error"); + }); + it("maps 5xx to a server_error", async () => { const { service } = createDeps(); fetchMock.mockResolvedValue(jsonResponse({}, 503)); @@ -133,7 +161,7 @@ describe("OAuthService.refreshToken", () => { it("maps other 4xx to an unknown_error", async () => { const { service } = createDeps(); - fetchMock.mockResolvedValue(jsonResponse({}, 400)); + fetchMock.mockResolvedValue(jsonResponse({}, 404)); const result = await service.refreshToken("rt", "us"); diff --git a/packages/core/src/oauth/oauth.ts b/packages/core/src/oauth/oauth.ts index a6ea202d24..91875148d8 100644 --- a/packages/core/src/oauth/oauth.ts +++ b/packages/core/src/oauth/oauth.ts @@ -216,8 +216,20 @@ export class OAuthService { }); if (!response.ok) { - // 401/403 are auth errors - the token is invalid - const isAuthError = response.status === 401 || response.status === 403; + // 401/403 are always auth failures. A 400 is only a dead refresh token + // when the OAuth error is invalid_grant/invalid_token; other 400s like + // invalid_client or invalid_request are config bugs and must not log the + // user out, or they would be unable to log back in with the same broken + // config. + const oauthError = + response.status === 400 + ? await this.readOAuthErrorCode(response) + : null; + const isAuthError = + response.status === 401 || + response.status === 403 || + oauthError === "invalid_grant" || + oauthError === "invalid_token"; // 5xx are server errors - should be retried const isServerError = response.status >= 500; this.log.warn( @@ -249,6 +261,15 @@ export class OAuthService { } } + private async readOAuthErrorCode(response: Response): Promise { + try { + const body = (await response.json()) as { error?: unknown }; + return typeof body.error === "string" ? body.error : null; + } catch { + return null; + } + } + /** * Cancel any pending OAuth flow. */ From f6a8bfacb7ed054378249f23df19bc4ae0f0e388 Mon Sep 17 00:00:00 2001 From: Charles Vien Date: Sat, 20 Jun 2026 22:34:49 -0700 Subject: [PATCH 2/2] extract oauth error parser and cover 400 cases --- packages/core/src/oauth/oauth.test.ts | 45 +++++++++++++++++++-------- packages/core/src/oauth/oauth.ts | 28 ++++++++--------- 2 files changed, 45 insertions(+), 28 deletions(-) diff --git a/packages/core/src/oauth/oauth.test.ts b/packages/core/src/oauth/oauth.test.ts index 7081267a32..bf680e42f3 100644 --- a/packages/core/src/oauth/oauth.test.ts +++ b/packages/core/src/oauth/oauth.test.ts @@ -122,31 +122,50 @@ describe("OAuthService.refreshToken", () => { expect(result.errorCode).toBe("auth_error"); }); - it("maps a 400 invalid_grant to an auth_error", async () => { + it.each([ + { + name: "invalid_grant", + body: { error: "invalid_grant" }, + expected: "auth_error", + }, + { + name: "invalid_token", + body: { error: "invalid_token" }, + expected: "auth_error", + }, + { + name: "invalid_client", + body: { error: "invalid_client" }, + expected: "unknown_error", + }, + { + name: "invalid_request", + body: { error: "invalid_request" }, + expected: "unknown_error", + }, + { + name: "a non-string error field", + body: { error: 42 }, + expected: "unknown_error", + }, + { name: "no error field", body: {}, expected: "unknown_error" }, + ])("maps a 400 $name to a $expected", async ({ body, expected }) => { const { service } = createDeps(); - fetchMock.mockResolvedValue(jsonResponse({ error: "invalid_grant" }, 400)); + fetchMock.mockResolvedValue(jsonResponse(body, 400)); const result = await service.refreshToken("rt", "us"); expect(result.success).toBe(false); - expect(result.errorCode).toBe("auth_error"); + expect(result.errorCode).toBe(expected); }); - it("maps a 400 invalid_client to an unknown_error, not a logout", async () => { - const { service } = createDeps(); - fetchMock.mockResolvedValue(jsonResponse({ error: "invalid_client" }, 400)); - - const result = await service.refreshToken("rt", "us"); - - expect(result.errorCode).toBe("unknown_error"); - }); - - it("maps a 400 with no parseable body to an unknown_error", async () => { + it("maps a 400 with an unparseable body to an unknown_error", async () => { const { service } = createDeps(); fetchMock.mockResolvedValue(new Response("", { status: 400 })); const result = await service.refreshToken("rt", "us"); + expect(result.success).toBe(false); expect(result.errorCode).toBe("unknown_error"); }); diff --git a/packages/core/src/oauth/oauth.ts b/packages/core/src/oauth/oauth.ts index 91875148d8..5149630174 100644 --- a/packages/core/src/oauth/oauth.ts +++ b/packages/core/src/oauth/oauth.ts @@ -61,6 +61,15 @@ interface PendingOAuthFlow { abortController?: AbortController; } +async function parseOAuthErrorCode(response: Response): Promise { + try { + const body = (await response.json()) as { error?: unknown }; + return typeof body.error === "string" ? body.error : null; + } catch { + return null; + } +} + @injectable() export class OAuthService { private pendingFlow: PendingOAuthFlow | null = null; @@ -221,15 +230,13 @@ export class OAuthService { // invalid_client or invalid_request are config bugs and must not log the // user out, or they would be unable to log back in with the same broken // config. - const oauthError = - response.status === 400 - ? await this.readOAuthErrorCode(response) - : null; + const oauthErrorCode = + response.status === 400 ? await parseOAuthErrorCode(response) : null; const isAuthError = response.status === 401 || response.status === 403 || - oauthError === "invalid_grant" || - oauthError === "invalid_token"; + oauthErrorCode === "invalid_grant" || + oauthErrorCode === "invalid_token"; // 5xx are server errors - should be retried const isServerError = response.status >= 500; this.log.warn( @@ -261,15 +268,6 @@ export class OAuthService { } } - private async readOAuthErrorCode(response: Response): Promise { - try { - const body = (await response.json()) as { error?: unknown }; - return typeof body.error === "string" ? body.error : null; - } catch { - return null; - } - } - /** * Cancel any pending OAuth flow. */