diff --git a/packages/core/src/oauth/oauth.test.ts b/packages/core/src/oauth/oauth.test.ts index a742523fa4..bf680e42f3 100644 --- a/packages/core/src/oauth/oauth.test.ts +++ b/packages/core/src/oauth/oauth.test.ts @@ -122,6 +122,53 @@ describe("OAuthService.refreshToken", () => { expect(result.errorCode).toBe("auth_error"); }); + 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(body, 400)); + + const result = await service.refreshToken("rt", "us"); + + expect(result.success).toBe(false); + expect(result.errorCode).toBe(expected); + }); + + 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"); + }); + it("maps 5xx to a server_error", async () => { const { service } = createDeps(); fetchMock.mockResolvedValue(jsonResponse({}, 503)); @@ -133,7 +180,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..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; @@ -216,8 +225,18 @@ 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 oauthErrorCode = + response.status === 400 ? await parseOAuthErrorCode(response) : null; + const isAuthError = + response.status === 401 || + response.status === 403 || + oauthErrorCode === "invalid_grant" || + oauthErrorCode === "invalid_token"; // 5xx are server errors - should be retried const isServerError = response.status >= 500; this.log.warn(