mirror of
https://github.com/better-auth/better-auth.git
synced 2026-10-18 11:32:15 -05:00
fix(oauth-provider): accept key-rotation assertions when baseURL ends in a slash (#11804)
This commit is contained in:
@@ -3,8 +3,9 @@
|
||||
* token audience, protected-resource metadata, and the organization claim.
|
||||
* Server-only: `BETTER_AUTH_URL` is not exposed to the client bundle.
|
||||
*/
|
||||
export const baseURL =
|
||||
process.env.BETTER_AUTH_URL || "https://demo.better-auth.com";
|
||||
export const baseURL = new URL(
|
||||
process.env.BETTER_AUTH_URL || "https://demo.better-auth.com",
|
||||
).origin;
|
||||
|
||||
export const mcpResource = `${baseURL}/api/mcp`;
|
||||
|
||||
|
||||
@@ -3,6 +3,7 @@ import { once } from "node:events";
|
||||
import { createServer } from "node:http";
|
||||
import type { AddressInfo } from "node:net";
|
||||
import { DatabaseSync } from "node:sqlite";
|
||||
import type { TestContext } from "node:test";
|
||||
import { describe, it } from "node:test";
|
||||
import { oauthProvider } from "@better-auth/oauth-provider";
|
||||
import type { BetterAuthOptions } from "better-auth";
|
||||
@@ -52,12 +53,15 @@ const setups: ServerSetup[] = [
|
||||
|
||||
const redirectUri = "https://client.example.test/callback";
|
||||
|
||||
async function startAuthServer(setup: ServerSetup) {
|
||||
const placeholder = createServer();
|
||||
placeholder.listen(0);
|
||||
await once(placeholder, "listening");
|
||||
const port = (placeholder.address() as AddressInfo).port;
|
||||
placeholder.close();
|
||||
async function startAuthServer(setup: ServerSetup, t: TestContext) {
|
||||
// Listen before building auth, which needs the port for `baseURL`.
|
||||
let handler: ReturnType<typeof toNodeHandler> | undefined;
|
||||
const server = createServer((req, res) => handler!(req, res));
|
||||
server.listen(0);
|
||||
// Registered now, so a setup failure below still closes the server.
|
||||
t.after(() => server.close());
|
||||
await once(server, "listening");
|
||||
const port = (server.address() as AddressInfo).port;
|
||||
|
||||
const serverOrigin = `http://localhost:${port}`;
|
||||
const publicOrigin = setup.publicOrigin ?? serverOrigin;
|
||||
@@ -79,9 +83,7 @@ async function startAuthServer(setup: ServerSetup) {
|
||||
const { runMigrations } = await getMigrations(auth.options);
|
||||
await runMigrations();
|
||||
|
||||
const server = createServer(toNodeHandler(auth.handler));
|
||||
server.listen(port);
|
||||
await once(server, "listening");
|
||||
handler = toNodeHandler(auth.handler);
|
||||
|
||||
// The proxy: requests leave for the public URL and reach the local server.
|
||||
const toServerUrl = (url: string) =>
|
||||
@@ -91,17 +93,16 @@ async function startAuthServer(setup: ServerSetup) {
|
||||
const proxyFetch = (url: string, init: RequestInit) =>
|
||||
fetch(toServerUrl(url), init);
|
||||
|
||||
return { auth, server, publicOrigin, toServerUrl, proxyFetch };
|
||||
return { auth, publicOrigin, toServerUrl, proxyFetch };
|
||||
}
|
||||
|
||||
describe("DPoP interop with an independent OAuth client", () => {
|
||||
for (const setup of setups) {
|
||||
it(`binds tokens and UserInfo to the proof key: ${setup.name}`, {
|
||||
it(`accepts the client's DPoP proofs at the token and UserInfo endpoints: ${setup.name}`, {
|
||||
todo: setup.todo,
|
||||
}, async (t) => {
|
||||
const { auth, server, publicOrigin, toServerUrl, proxyFetch } =
|
||||
await startAuthServer(setup);
|
||||
t.after(() => server.close());
|
||||
const { auth, publicOrigin, toServerUrl, proxyFetch } =
|
||||
await startAuthServer(setup, t);
|
||||
|
||||
const signUp = await auth.api.signUpEmail({
|
||||
body: {
|
||||
|
||||
@@ -6,7 +6,7 @@ import { memoryAdapter } from "@better-auth/memory-adapter";
|
||||
import { jwt } from "better-auth/plugins/jwt";
|
||||
import { getTestInstance } from "better-auth/test";
|
||||
import type { JWK } from "jose";
|
||||
import { exportJWK, generateKeyPair } from "jose";
|
||||
import { exportJWK, generateKeyPair, importJWK, SignJWT } from "jose";
|
||||
import { describe, expect, it, vi } from "vitest";
|
||||
import { oauthProvider } from "../oauth";
|
||||
|
||||
@@ -42,11 +42,14 @@ function sign(clientId: string, key: TestKey, audience = rotateURL) {
|
||||
});
|
||||
}
|
||||
|
||||
async function setup(testWith: "sqlite" | "postgres" = "sqlite") {
|
||||
async function setup(
|
||||
testWith: "sqlite" | "postgres" = "sqlite",
|
||||
authBaseURL = baseURL,
|
||||
) {
|
||||
const trustedClients = new Set<string>();
|
||||
const { auth, signInWithTestUser, customFetchImpl } = await getTestInstance(
|
||||
{
|
||||
baseURL,
|
||||
baseURL: authBaseURL,
|
||||
trustedOrigins: ["https://trusted.example.com"],
|
||||
plugins: [
|
||||
jwt(),
|
||||
@@ -238,6 +241,37 @@ describe("client key rotation", async () => {
|
||||
expect(await authenticate(clientId, oldKey)).toBe(200);
|
||||
});
|
||||
|
||||
it("accepts an audience list only when it names the rotation endpoint", async () => {
|
||||
const oldKey = await createKey("old");
|
||||
const clientId = await createClient([oldKey]);
|
||||
const signFor = async (audience: string[]) =>
|
||||
new SignJWT({})
|
||||
.setProtectedHeader({ alg: "ES256", kid: oldKey.kid })
|
||||
.setIssuer(clientId)
|
||||
.setSubject(clientId)
|
||||
.setAudience(audience)
|
||||
.setIssuedAt()
|
||||
.setExpirationTime("1m")
|
||||
.setJti(crypto.randomUUID())
|
||||
.sign(await importJWK(oldKey.privateJwk, "ES256"));
|
||||
|
||||
const elsewhere = await rotate({
|
||||
clientId,
|
||||
signer: oldKey,
|
||||
next: [await createKey("new")],
|
||||
assertion: await signFor([apiURL, baseURL, `${apiURL}/oauth2/token`]),
|
||||
});
|
||||
const addressed = await rotate({
|
||||
clientId,
|
||||
signer: oldKey,
|
||||
next: [await createKey("new")],
|
||||
assertion: await signFor([apiURL, rotateURL]),
|
||||
});
|
||||
|
||||
expect(elsewhere.status).toBe(401);
|
||||
expect(addressed.status).toBe(200);
|
||||
});
|
||||
|
||||
it("rejects a malformed or unsafe replacement key set before touching the client", async () => {
|
||||
const oldKey = await createKey("old");
|
||||
const clientId = await createClient([oldKey]);
|
||||
@@ -320,6 +354,26 @@ describe("client key rotation", async () => {
|
||||
});
|
||||
});
|
||||
|
||||
/**
|
||||
* @see https://github.com/better-auth/better-auth/issues/10800
|
||||
*/
|
||||
describe("client key rotation with a trailing-slash baseURL", async () => {
|
||||
const { createClient, rotate } = await setup("sqlite", `${apiURL}/`);
|
||||
|
||||
it("accepts an assertion addressed to the URL the client calls", async () => {
|
||||
const oldKey = await createKey("old");
|
||||
const clientId = await createClient([oldKey]);
|
||||
|
||||
const response = await rotate({
|
||||
clientId,
|
||||
signer: oldKey,
|
||||
next: [await createKey("new")],
|
||||
});
|
||||
|
||||
expect(response.status).toBe(200);
|
||||
});
|
||||
});
|
||||
|
||||
/**
|
||||
* A rotation assertion must be single-use: replaying a captured one while its
|
||||
* signing key is still current would hand the client's keys to whoever holds
|
||||
|
||||
@@ -3,6 +3,7 @@ import { APIError, getSessionFromCtx } from "better-auth/api";
|
||||
import { generateRandomString } from "better-auth/crypto";
|
||||
import type { ClientJwks } from "../client-jwks";
|
||||
import { assertValidClientJwks } from "../client-jwks";
|
||||
import { getEndpointUrl } from "../dpop";
|
||||
import type { OAuthClientRegistrationMetadata } from "../register";
|
||||
import { checkOAuthClient, oauthToSchema, schemaToOAuth } from "../register";
|
||||
import type {
|
||||
@@ -494,7 +495,12 @@ export async function rotateClientKeysEndpoint(
|
||||
body.client_assertion,
|
||||
body.client_assertion_type,
|
||||
undefined,
|
||||
`${ctx.context.baseURL}/oauth2/client/rotate-keys`,
|
||||
// Without the request, so the audience stays the fixed endpoint path; this
|
||||
// still drops a trailing slash from `baseURL`.
|
||||
getEndpointUrl(
|
||||
{ context: ctx.context, request: undefined },
|
||||
"/oauth2/client/rotate-keys",
|
||||
),
|
||||
{
|
||||
endpointAudienceOnly: true,
|
||||
assertClient: (candidate) => assertRotatable(opts, candidate),
|
||||
|
||||
Reference in New Issue
Block a user