From fedd05586c5a7ee2eb6b8d16b88b08dfb7e16a89 Mon Sep 17 00:00:00 2001 From: vhinayindia Date: Thu, 15 Oct 2026 17:05:36 +0530 Subject: [PATCH] fix(oauth2): report an unreachable JWKS as an error, not an invalid token (#11612) Co-authored-by: vhinayindia <281186805+vhinayindia@users.noreply.github.com> Co-authored-by: Gustavo Valverde --- .changeset/fix-jwks-network-errors.md | 9 ++ packages/core/src/oauth2/verify.test.ts | 127 ++++++++++++++++++ packages/core/src/oauth2/verify.ts | 46 ++++--- .../src/jwt-verification-errors.test.ts | 49 +++++++ 4 files changed, 215 insertions(+), 16 deletions(-) create mode 100644 .changeset/fix-jwks-network-errors.md diff --git a/.changeset/fix-jwks-network-errors.md b/.changeset/fix-jwks-network-errors.md new file mode 100644 index 0000000000..65dbdf7a3b --- /dev/null +++ b/.changeset/fix-jwks-network-errors.md @@ -0,0 +1,9 @@ +--- +"@better-auth/core": patch +--- + +If your resource server verifies access tokens with `verifyAccessTokenRequest` or `verifyBearerToken` and a `jwksUrl`, including through the MCP plugin, a key set that cannot be reached at all, for example a refused connection, now throws an error. Before, the token was rejected as invalid with 401 "no token payload", which hid the outage. A key set that answers with an HTTP error already threw. + +With `remoteVerify` configured, such a failure no longer falls back to remote introspection. + +The OAuth provider's `/oauth2/introspect`, `/oauth2/userinfo` and `/oauth2/revoke` behave the same way with the JWT plugin's `jwks.remoteUrl`: they return 500 instead of reporting the token as invalid. diff --git a/packages/core/src/oauth2/verify.test.ts b/packages/core/src/oauth2/verify.test.ts index 34c50a2327..c7a5808f0f 100644 --- a/packages/core/src/oauth2/verify.test.ts +++ b/packages/core/src/oauth2/verify.test.ts @@ -10,6 +10,7 @@ import { it, vi, } from "vitest"; +import { logger } from "../env"; import { createInsufficientScopeError, isInsufficientScopeError, @@ -40,6 +41,7 @@ describe("verifyBearerToken", () => { // Tests reuse one JWKS URL with new keys; a cached set would put later // tests' kids inside the refetch cooldown. jwksCache.clear(); + vi.restoreAllMocks(); }); afterAll(() => { @@ -587,6 +589,60 @@ describe("verifyBearerToken", () => { vi.resetModules(); }); + it("should sanitize TypeErrors from a function jwks source", async () => { + vi.resetModules(); + const { logger } = await import("../env"); + const { verifyJwsAccessToken: verify } = await import("./verify"); + const { privateKey, kid } = await createTestJWKS(); + const token = await createSignedToken(privateKey, kid); + const providerError = new TypeError( + "fetch failed for https://internal.example/private-jwks", + ); + const loggerError = vi.spyOn(logger, "error").mockImplementation(() => {}); + const jwksFetch = vi.fn(async () => { + throw providerError; + }); + + const error = await verify(token, { + jwksFetch, + verifyOptions: { issuer, audience }, + }).then( + () => undefined, + (thrown: unknown) => thrown, + ); + + expect(jwksFetch).toHaveBeenCalledOnce(); + expect(error).toBeInstanceOf(Error); + expect(error).toMatchObject({ message: "JWKS fetch failed" }); + expect(error).not.toHaveProperty("cause"); + expect(loggerError).toHaveBeenCalledWith( + "JWKS fetch failed:", + providerError, + ); + expect(mockedFetch).not.toHaveBeenCalled(); + vi.resetModules(); + }); + + it("should preserve non-TypeError failures from a function jwks source", async () => { + vi.resetModules(); + const { verifyJwsAccessToken: verify } = await import("./verify"); + const { privateKey, kid } = await createTestJWKS(); + const token = await createSignedToken(privateKey, kid); + const providerError = new Error("custom JWKS provider failed"); + const jwksFetch = vi.fn(async () => { + throw providerError; + }); + + await expect( + verify(token, { + jwksFetch, + verifyOptions: { issuer, audience }, + }), + ).rejects.toBe(providerError); + expect(jwksFetch).toHaveBeenCalledOnce(); + vi.resetModules(); + }); + it("should fetch a function jwks source once across verifications sharing a jwksCacheKey", async () => { vi.resetModules(); const { verifyJwsAccessToken: verify } = await import("./verify"); @@ -962,6 +1018,77 @@ describe("verifyBearerToken", () => { } }); + /** + * @see https://github.com/better-auth/better-auth/issues/11611 + */ + describe("rejected JWKS fetch", () => { + it("should report the failure even when the logger throws", async () => { + const { privateKey, kid } = await createTestJWKS(); + const token = await createSignedToken(privateKey, kid); + mockedFetch.mockRejectedValueOnce(new TypeError("fetch failed")); + vi.spyOn(logger, "error").mockImplementation(() => { + throw new TypeError("logger failed"); + }); + + const error = await verifyBearerToken(token, { + jwksUrl, + verifyOptions: { issuer, audience }, + }).then( + () => undefined, + (thrown: unknown) => thrown, + ); + + expect(error).toMatchObject({ message: "JWKS fetch failed" }); + }); + + it("should report a refused connection instead of a bad token", async () => { + const { privateKey, kid } = await createTestJWKS(); + const token = await createSignedToken(privateKey, kid); + mockedFetch.mockRejectedValueOnce(new TypeError("fetch failed")); + + const error = await verifyAccessTokenRequest( + { + authorizationHeader: `Bearer ${token}`, + method: "GET", + url: audience, + }, + { jwksUrl, verifyOptions: { issuer, audience } }, + ).then( + () => undefined, + (thrown: unknown) => thrown, + ); + + expect(error).not.toBeInstanceOf(APIError); + expect(error).toMatchObject({ message: "JWKS fetch failed" }); + }); + + it("should not fall through to opaque verification", async () => { + const { privateKey, kid } = await createTestJWKS(); + const token = await createSignedToken(privateKey, kid); + mockedFetch.mockRejectedValueOnce( + new TypeError("fetch failed for https://internal.example/private-jwks"), + ); + + const error = await verifyBearerToken(token, { + jwksUrl, + verifyOptions: { issuer, audience }, + remoteVerify: { + introspectUrl: `${issuer}/oauth2/introspect`, + clientId: "rs-client", + clientSecret: "rs-secret", + }, + }).then( + () => undefined, + (thrown: unknown) => thrown, + ); + + expect(error).toBeInstanceOf(Error); + expect(error).toMatchObject({ message: "JWKS fetch failed" }); + expect(error).not.toHaveProperty("cause"); + expect(mockedFetch).toHaveBeenCalledOnce(); + }); + }); + describe("remote introspection audience validation", () => { const introspectUrl = `${issuer}/oauth2/introspect`; const remoteVerify = { diff --git a/packages/core/src/oauth2/verify.ts b/packages/core/src/oauth2/verify.ts index 57316d1748..ceb863d9a6 100644 --- a/packages/core/src/oauth2/verify.ts +++ b/packages/core/src/oauth2/verify.ts @@ -194,22 +194,36 @@ function shouldRefetchCachedJwksWithoutKid( async function fetchJwks( jwksFetch: JwksFetchOptions["jwksFetch"], ): Promise { - const jwks = - typeof jwksFetch === "string" - ? await fetchRefusingRedirects(jwksFetch, { - headers: { - Accept: "application/json", - }, - }).then(async (res) => { - if (res.error) - throw new Error( - `Jwks failed: ${res.error.message ?? res.error.statusText}`, - ); - return res.data; - }) - : await jwksFetch(); - if (!jwks) throw new Error("No jwks found"); - return jwks; + try { + const jwks = + typeof jwksFetch === "string" + ? await fetchRefusingRedirects(jwksFetch, { + headers: { + Accept: "application/json", + }, + }).then(async (res) => { + if (res.error) + throw new Error( + `Jwks failed: ${res.error.message ?? res.error.statusText}`, + ); + return res.data; + }) + : await jwksFetch(); + if (!jwks) throw new Error("No jwks found"); + return jwks; + } catch (error) { + if (error instanceof TypeError) { + // A TypeError from a JWKS source is an infrastructure failure, not an + // opaque token. Do not expose runtime-specific details to callers. + try { + logger.error("JWKS fetch failed:", error); + } catch { + // A throwing custom logger must not turn this back into a TypeError. + } + throw new Error("JWKS fetch failed"); + } + throw error; + } } export interface VerifyAccessTokenRemote { diff --git a/packages/oauth-provider/src/jwt-verification-errors.test.ts b/packages/oauth-provider/src/jwt-verification-errors.test.ts index dc9ea59532..50b6de8fe4 100644 --- a/packages/oauth-provider/src/jwt-verification-errors.test.ts +++ b/packages/oauth-provider/src/jwt-verification-errors.test.ts @@ -360,3 +360,52 @@ describe("oauth-provider - JWKS failures during access token verification", asyn expect((await revoke(server, token)).error?.status).toBe(500); }); }); + +/** + * @see https://github.com/better-auth/better-auth/issues/11611 + */ +describe("oauth-provider - unreachable remote JWKS", async () => { + const jwksUrl = "https://jwks-down.example.com/jwks"; + const server = await setup( + jwt({ + jwt: { issuer: authServerBaseUrl }, + jwks: { remoteUrl: jwksUrl, keyPairConfig: { alg: "EdDSA" } }, + }), + ); + + beforeEach(() => { + // A refused connection: `fetch` rejects with a TypeError. + const networkFetch = globalThis.fetch.bind(globalThis); + vi.stubGlobal("fetch", (input: RequestInfo | URL, init?: RequestInit) => { + const requestedUrl = + typeof input === "string" + ? input + : input instanceof URL + ? input.href + : input.url; + if (requestedUrl === jwksUrl) { + return Promise.reject(new TypeError("fetch failed")); + } + return networkFetch(input, init); + }); + }); + + afterEach(() => { + vi.unstubAllGlobals(); + }); + + it("surfaces the outage at introspection", async () => { + const token = await signWithUnknownKey("remote-kid"); + expect((await introspect(server, token)).error?.status).toBe(500); + }); + + it("surfaces the outage at userinfo", async () => { + const token = await signWithUnknownKey("remote-kid"); + expect((await userinfo(server, token)).error?.status).toBe(500); + }); + + it("surfaces the outage at revocation", async () => { + const token = await signWithUnknownKey("remote-kid"); + expect((await revoke(server, token)).error?.status).toBe(500); + }); +});