mirror of
https://github.com/better-auth/better-auth.git
synced 2026-10-18 11:32:15 -05:00
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 <g.valverde02@gmail.com>
This commit is contained in:
co-authored by
vhinayindia
Gustavo Valverde
parent
963b7f9417
commit
fedd05586c
@@ -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.
|
||||
@@ -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 = {
|
||||
|
||||
@@ -194,22 +194,36 @@ function shouldRefetchCachedJwksWithoutKid(
|
||||
async function fetchJwks(
|
||||
jwksFetch: JwksFetchOptions["jwksFetch"],
|
||||
): Promise<JSONWebKeySet> {
|
||||
const jwks =
|
||||
typeof jwksFetch === "string"
|
||||
? await fetchRefusingRedirects<JSONWebKeySet>(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<JSONWebKeySet>(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 {
|
||||
|
||||
@@ -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);
|
||||
});
|
||||
});
|
||||
|
||||
Reference in New Issue
Block a user