fix(oauth-provider): defer logout effects until commit (#10472)

This commit is contained in:
Gustavo Valverde
2026-07-22 06:09:55 -04:00
committed by GitHub
parent dbd302e422
commit 69acb7a3db
6 changed files with 432 additions and 71 deletions
+5
View File
@@ -0,0 +1,5 @@
---
"@better-auth/oauth-provider": patch
---
Revoke session-bound OAuth tokens and deliver back-channel logout only after the related session deletion succeeds.
+1
View File
@@ -71,6 +71,7 @@
},
"devDependencies": {
"@better-auth/core": "workspace:*",
"@better-auth/memory-adapter": "workspace:*",
"better-auth": "workspace:*",
"listhen": "^1.9.0",
"tsdown": "catalog:"
@@ -1,9 +1,16 @@
import { createServer } from "node:http";
import type { AddressInfo } from "node:net";
import { createAuthEndpoint } from "@better-auth/core/api";
import {
getCurrentAdapter,
runWithAdapter,
runWithTransaction,
} from "@better-auth/core/context";
import {
authorizationCodeRequest,
createAuthorizationURL,
} from "@better-auth/core/oauth2";
import { memoryAdapter } from "@better-auth/memory-adapter";
import { createAuthClient } from "better-auth/client";
import { generateRandomString } from "better-auth/crypto";
import { toNodeHandler } from "better-auth/node";
@@ -21,6 +28,7 @@ import {
expect,
it,
} from "vitest";
import * as z from "zod";
import { oauthProviderClient } from "./client";
import { oauthProvider } from "./oauth";
@@ -74,18 +82,46 @@ async function startMockRp(
type MakeRequired<T, K extends keyof T> = Omit<T, K> & Required<Pick<T, K>>;
describe("oauth back-channel logout", async () => {
const database = {
account: [],
jwks: [],
oauthAccessToken: [],
oauthClient: [],
oauthClientAssertion: [],
oauthClientResource: [],
oauthConsent: [],
oauthRefreshToken: [],
oauthResource: [],
session: [],
user: [],
verification: [],
};
const port = 3010;
const baseUrl = `http://localhost:${port}`;
const state = "123";
const scopes = ["openid", "email", "profile", "offline_access"];
let shouldVetoSessionDeletion = false;
const { auth, signInWithTestUser, customFetchImpl } = await getTestInstance({
baseURL: baseUrl,
database: memoryAdapter(database),
databaseHooks: {
session: {
delete: {
async before() {
if (!shouldVetoSessionDeletion) return;
shouldVetoSessionDeletion = false;
return false;
},
},
},
},
plugins: [
oauthProvider({
loginPage: "/login",
consentPage: "/consent",
allowDynamicClientRegistration: true,
pairwiseSecret: "test-backchannel-pairwise-secret-32-chars",
silenceWarnings: {
oauthAuthServerConfig: true,
openidConfig: true,
@@ -93,6 +129,91 @@ describe("oauth back-channel logout", async () => {
scopes,
}),
jwt(),
{
id: "transactional-session-revocation-test",
endpoints: {
testTransactionalSessionRevocation: createAuthEndpoint(
"/test/transactional-session-revocation",
{
method: "POST",
body: z.object({
clientId: z.string().optional(),
rollback: z.boolean(),
sessionToken: z.string(),
trackActiveAdapterWrites: z.boolean().optional(),
transactionAccessTokenId: z.string().optional(),
}),
},
async (ctx) => {
const rollbackMarker = Symbol("rollback session revocation");
const updatedModelsThroughActiveAdapter: string[] = [];
const revokeSession = async () => {
try {
await runWithTransaction(ctx.context.adapter, async () => {
if (
ctx.body.clientId &&
ctx.body.transactionAccessTokenId
) {
const activeSession =
await ctx.context.internalAdapter.findSession(
ctx.body.sessionToken,
);
if (!activeSession) throw new Error("Session not found");
const adapter = await getCurrentAdapter(
ctx.context.adapter,
);
await adapter.create({
model: "oauthAccessToken",
forceAllowId: true,
data: {
id: ctx.body.transactionAccessTokenId,
token: ctx.body.transactionAccessTokenId,
clientId: ctx.body.clientId,
sessionId: activeSession.session.id,
userId: activeSession.user.id,
scopes: ["openid"],
expiresAt: new Date(Date.now() + 60_000),
createdAt: new Date(),
revoked: null,
},
});
}
await ctx.context.internalAdapter.deleteSession(
ctx.body.sessionToken,
);
if (ctx.body.rollback) throw rollbackMarker;
});
} catch (error) {
if (error !== rollbackMarker) throw error;
return false;
}
return true;
};
const committed = ctx.body.trackActiveAdapterWrites
? await (async () => {
const baseAdapter = await getCurrentAdapter(
ctx.context.adapter,
);
const activeAdapter = {
...baseAdapter,
async updateMany(input) {
updatedModelsThroughActiveAdapter.push(input.model);
return baseAdapter.updateMany(input);
},
} satisfies typeof baseAdapter;
return runWithAdapter(activeAdapter, revokeSession);
})()
: await revokeSession();
return ctx.json({
committed,
...(ctx.body.trackActiveAdapterWrites
? { updatedModelsThroughActiveAdapter }
: {}),
});
},
),
},
},
],
});
let { headers } = await signInWithTestUser();
@@ -112,6 +233,7 @@ describe("oauth back-channel logout", async () => {
if (server) await server.close();
});
beforeEach(async () => {
shouldVetoSessionDeletion = false;
rp = await startMockRp();
const signed = await signInWithTestUser();
headers = signed.headers;
@@ -125,6 +247,7 @@ describe("oauth back-channel logout", async () => {
enable_end_session: boolean;
backchannel_logout_uri: string | undefined;
backchannel_logout_session_required: boolean;
subject_type: "pairwise" | "public";
}> = {},
) {
const response = await auth.api.adminCreateOAuthClient({
@@ -210,6 +333,53 @@ describe("oauth back-channel logout", async () => {
}
}
async function revokeSessionOverHTTP(input: {
clientId?: string;
rollback: boolean;
sessionToken: string;
trackActiveAdapterWrites?: boolean;
transactionAccessTokenId?: string;
}) {
const response = await fetch(
`${baseUrl}/api/auth/test/transactional-session-revocation`,
{
method: "POST",
headers: { "Content-Type": "application/json" },
body: JSON.stringify(input),
},
);
if (!response.ok) {
throw new Error(
`Transactional session revocation failed: ${await response.text()}`,
);
}
return (await response.json()) as {
committed: boolean;
updatedModelsThroughActiveAdapter?: string[];
};
}
async function readClientTokenRevocation(clientId: string) {
const ctx = await auth.$context;
const [accessTokens, refreshTokens] = await Promise.all([
ctx.adapter.findMany<{ revoked?: Date | null }>({
model: "oauthAccessToken",
where: [{ field: "clientId", value: clientId }],
}),
ctx.adapter.findMany<{ revoked?: Date | null }>({
model: "oauthRefreshToken",
where: [{ field: "clientId", value: clientId }],
}),
]);
return { accessTokens, refreshTokens };
}
async function getSessionToken(requestHeaders: Headers) {
const session = await auth.api.getSession({ headers: requestHeaders });
if (!session) throw new Error("Expected an active test session");
return session.session.token;
}
it("dispatches a conformant logout token when the session is signed out", async () => {
const oauthClient = await registerClient();
const tokens = await issueTokens({ client: oauthClient });
@@ -251,6 +421,103 @@ describe("oauth back-channel logout", async () => {
expect(payload.sid).toBe(idTokenPayload.sid);
});
it("dispatches logout only after a transactional session revocation commits", async () => {
const signedIn = await signInWithTestUser();
headers = signedIn.headers;
const oauthClient = await registerClient();
await issueTokens({ client: oauthClient });
const rolledBack = await revokeSessionOverHTTP({
rollback: true,
sessionToken: await getSessionToken(headers),
});
expect(rolledBack).toEqual({ committed: false });
await new Promise((resolve) => setTimeout(resolve, 200));
expect(rp.received).toHaveLength(0);
expect(await auth.api.getSession({ headers })).not.toBeNull();
const tokensAfterRollback = await readClientTokenRevocation(
oauthClient.client_id,
);
expect(tokensAfterRollback.accessTokens.length).toBeGreaterThan(0);
expect(tokensAfterRollback.refreshTokens.length).toBeGreaterThan(0);
for (const token of tokensAfterRollback.accessTokens) {
expect(token.revoked ?? null).toBeNull();
}
for (const token of tokensAfterRollback.refreshTokens) {
expect(token.revoked ?? null).toBeNull();
}
const committed = await revokeSessionOverHTTP({
clientId: oauthClient.client_id,
rollback: false,
sessionToken: await getSessionToken(headers),
transactionAccessTokenId: "transaction-only-access-token",
});
expect(committed).toEqual({ committed: true });
await waitForDispatches();
expect(rp.received).toHaveLength(1);
expect(await auth.api.getSession({ headers })).toBeNull();
const tokensAfterCommit = await readClientTokenRevocation(
oauthClient.client_id,
);
for (const token of tokensAfterCommit.accessTokens) {
expect(token.revoked).toBeInstanceOf(Date);
}
for (const token of tokensAfterCommit.refreshTokens) {
expect(token.revoked ?? null).toBeNull();
}
const transactionOnlyToken = await (await auth.$context).adapter.findOne<{
revoked?: Date | null;
}>({
model: "oauthAccessToken",
where: [{ field: "id", value: "transaction-only-access-token" }],
});
expect(transactionOnlyToken?.revoked).toBeInstanceOf(Date);
});
it("applies committed token revocation through the active adapter", async () => {
const signedIn = await signInWithTestUser();
headers = signedIn.headers;
const oauthClient = await registerClient();
await issueTokens({
client: oauthClient,
requestScopes: ["openid", "email", "profile"],
});
const committed = await revokeSessionOverHTTP({
rollback: false,
sessionToken: await getSessionToken(headers),
trackActiveAdapterWrites: true,
});
expect(committed).toEqual({
committed: true,
updatedModelsThroughActiveAdapter: ["oauthAccessToken"],
});
});
it("keeps the session and tokens active when a later hook vetoes deletion", async () => {
const signedIn = await signInWithTestUser();
headers = signedIn.headers;
const oauthClient = await registerClient();
await issueTokens({ client: oauthClient });
shouldVetoSessionDeletion = true;
const result = await revokeSessionOverHTTP({
rollback: false,
sessionToken: await getSessionToken(headers),
});
expect(result).toEqual({ committed: true });
await new Promise((resolve) => setTimeout(resolve, 200));
expect(await auth.api.getSession({ headers })).not.toBeNull();
expect(rp.received).toHaveLength(0);
const tokens = await readClientTokenRevocation(oauthClient.client_id);
for (const token of [...tokens.accessTokens, ...tokens.refreshTokens]) {
expect(token.revoked ?? null).toBeNull();
}
});
it("dispatches when RP-Initiated Logout tears down the session", async () => {
const oauthClient = await registerClient();
const tokens = await issueTokens({ client: oauthClient });
@@ -371,6 +638,35 @@ describe("oauth back-channel logout", async () => {
await waitForDispatches();
expect(rp.received).toHaveLength(1);
});
it("isolates a malformed pairwise client from revocation and healthy RP delivery", async () => {
const healthyClient = await registerClient();
const malformedClient = await registerClient({ subject_type: "pairwise" });
await issueTokens({ client: healthyClient });
await issueTokens({ client: malformedClient });
const ctx = await auth.$context;
await ctx.adapter.update({
model: "oauthClient",
where: [{ field: "clientId", value: malformedClient.client_id }],
update: { redirectUris: [] },
});
await client.signOut({ fetchOptions: { headers } });
await waitForDispatches();
expect(rp.received).toHaveLength(1);
for (const clientId of [
healthyClient.client_id,
malformedClient.client_id,
]) {
const tokens = await readClientTokenRevocation(clientId);
expect(tokens.accessTokens.length).toBeGreaterThan(0);
for (const token of tokens.accessTokens) {
expect(token.revoked).toBeInstanceOf(Date);
}
}
});
});
describe("oauth back-channel logout (jwt plugin disabled)", async () => {
@@ -466,8 +762,8 @@ describe("oauth back-channel logout (jwt plugin disabled)", async () => {
expect(before.length).toBeGreaterThan(0);
for (const t of before) expect(t.revoked ?? null).toBeNull();
// Revocation runs inline in `session.delete.before`; with no jwt plugin
// there is no async delivery to wait for.
// With no JWT plugin, the post-delete phase revokes tokens without
// attempting Logout Token delivery.
await client.signOut({ fetchOptions: { headers } });
const after = await ctx.adapter.findMany<{ revoked?: Date | null }>({
+86 -53
View File
@@ -1,4 +1,5 @@
import type { GenericEndpointContext } from "@better-auth/core";
import { getCurrentAdapter } from "@better-auth/core/context";
import { generateRandomString } from "better-auth/crypto";
import { getJwks } from "better-auth/oauth2";
import { resolveSigningKey, signJWT } from "better-auth/plugins";
@@ -51,7 +52,9 @@ interface BackchannelLogoutTarget {
* always present because every session-end path that reaches here carries the
* id of the session being terminated.
*/
interface BackchannelLogoutPlan {
export interface BackchannelLogoutPlan {
accessTokenIds: string[];
refreshTokenIds: string[];
sessionId: string;
targets: BackchannelLogoutTarget[];
}
@@ -96,10 +99,10 @@ async function signLogoutToken(
}
/**
* Synchronous phase: enumerate tokens for the session being terminated, revoke
* them, and return a plan for the asynchronous delivery phase. Runs inline in
* the `session.delete.before` hook so the DB state is consistent before the
* session row disappears.
* Build the immutable work plan before a session is deleted. This phase only
* reads database state because a later delete hook may still veto the mutation.
* Token identifiers must be captured here because their `sessionId` foreign key
* is cleared when the session row disappears.
*
* Revocation is the stored backstop, not the primary enforcement: introspection
* and `/userinfo` already treat a token whose session has ended as inactive
@@ -110,14 +113,14 @@ async function signLogoutToken(
* revoked; `offline_access` refresh tokens survive so long-lived API access can
* outlive the browser session.
*
* Revocation runs regardless of the JWT plugin (refresh-token revocation has no
* dependency on signing). Only the Logout Token delivery plan needs the JWT
* plugin, so when it is disabled we still revoke but never build a plan.
* Token revocation runs regardless of the JWT plugin (refresh-token revocation
* has no dependency on signing). Only Logout Token delivery needs the JWT
* plugin, so a plan may contain token identifiers with no delivery targets.
*
* Returns `null` when there is nothing to do, so the caller can skip the
* background handoff entirely.
*/
async function revokeAndPlanBackchannelLogout(
async function prepareBackchannelLogoutPlan(
ctx: GenericEndpointContext,
opts: OAuthOptions<Scope[]>,
input: { sessionId: string; userId: string },
@@ -127,14 +130,15 @@ async function revokeAndPlanBackchannelLogout(
const logger = ctx.context.logger;
try {
const adapter = await getCurrentAdapter(ctx.context.adapter);
const where = [{ field: "sessionId", value: sessionId }];
const [accessTokens, refreshTokens] = await Promise.all([
ctx.context.adapter.findMany<TokenRow>({
adapter.findMany<TokenRow>({
model: "oauthAccessToken",
where,
}),
ctx.context.adapter.findMany<TokenRow>({
adapter.findMany<TokenRow>({
model: "oauthRefreshToken",
where,
}),
@@ -145,7 +149,7 @@ async function revokeAndPlanBackchannelLogout(
for (const t of refreshTokens) affectedClientIds.add(t.clientId);
if (affectedClientIds.size === 0) return null;
const clients = await ctx.context.adapter.findMany<SchemaClient<Scope[]>>({
const clients = await adapter.findMany<SchemaClient<Scope[]>>({
model: "oauthClient",
where: [
{
@@ -159,7 +163,6 @@ async function revokeAndPlanBackchannelLogout(
// Access tokens are always revoked (OP hardening). Refresh tokens follow
// §2.7: revoke unless `offline_access` was granted. The non-offline_access
// branch is reachable via refresh-token scope narrowing, so it must stay.
const revokedAt = new Date();
const accessToRevokeIds = accessTokens
.filter((t) => !t.revoked)
.map((t) => t.id);
@@ -167,57 +170,87 @@ async function revokeAndPlanBackchannelLogout(
.filter((t) => !t.revoked && !t.scopes?.includes("offline_access"))
.map((t) => t.id);
const revocations = await Promise.allSettled([
accessToRevokeIds.length > 0
? ctx.context.adapter.updateMany({
model: "oauthAccessToken",
where: [{ field: "id", operator: "in", value: accessToRevokeIds }],
update: { revoked: revokedAt },
})
: Promise.resolve(),
refreshToRevokeIds.length > 0
? ctx.context.adapter.updateMany({
model: "oauthRefreshToken",
where: [{ field: "id", operator: "in", value: refreshToRevokeIds }],
update: { revoked: revokedAt },
})
: Promise.resolve(),
]);
// Surface a failed revocation write at error level. Dispatch still
// proceeds (RP notification is independent and the session-liveness
// checks in introspection remain authoritative), but operators need a
// signal that the stored `revoked` backstop drifted from session state.
for (const result of revocations) {
if (result.status === "rejected") {
logger.error(
"back-channel logout: token revocation update failed",
result.reason,
);
}
}
// Logout Tokens are signed through the JWT plugin's JWKS, so skip the
// delivery plan when it is disabled. Registration already rejects
// `backchannel_logout_uri` in that mode; this also guards stale clients.
const eligibleClients = opts.disableJwtPlugin
? []
: clients.filter((c) => Boolean(c.backchannelLogoutUri) && !c.disabled);
if (eligibleClients.length === 0) return null;
const targets: BackchannelLogoutTarget[] = await Promise.all(
eligibleClients.map(async (client) => ({
client,
sub: await resolveSubjectIdentifier(userId, client, opts),
})),
);
const targets = (
await Promise.all(
eligibleClients.map(async (client) => {
try {
return {
client,
sub: await resolveSubjectIdentifier(userId, client, opts),
} satisfies BackchannelLogoutTarget;
} catch (error) {
logger.warn(
`back-channel logout: unable to resolve subject for client ${client.clientId}`,
error,
);
return null;
}
}),
)
).filter((target): target is BackchannelLogoutTarget => target !== null);
return { sessionId, targets };
return {
accessTokenIds: accessToRevokeIds,
refreshTokenIds: refreshToRevokeIds,
sessionId,
targets,
};
} catch (error) {
logger.error("back-channel logout revocation failed", error);
logger.error("back-channel logout planning failed", error);
return null;
}
}
/**
* Apply a prepared logout plan after the session deletion succeeds. Core runs
* `session.delete.after` after the row is consumed and, for transactional
* callers, only after the transaction commits.
*/
async function applyBackchannelLogoutPlan(
ctx: GenericEndpointContext,
plan: BackchannelLogoutPlan,
): Promise<void> {
const revokedAt = new Date();
const adapter = await getCurrentAdapter(ctx.context.adapter);
const revocations = await Promise.allSettled([
plan.accessTokenIds.length > 0
? adapter.updateMany({
model: "oauthAccessToken",
where: [{ field: "id", operator: "in", value: plan.accessTokenIds }],
update: { revoked: revokedAt },
})
: Promise.resolve(),
plan.refreshTokenIds.length > 0
? adapter.updateMany({
model: "oauthRefreshToken",
where: [{ field: "id", operator: "in", value: plan.refreshTokenIds }],
update: { revoked: revokedAt },
})
: Promise.resolve(),
]);
// Session liveness remains authoritative, but operators still need a signal
// if the stored revocation backstop drifts from the committed session state.
for (const result of revocations) {
if (result.status === "rejected") {
ctx.context.logger.error(
"back-channel logout: token revocation update failed",
result.reason,
);
}
}
if (plan.targets.length > 0) {
await deliverBackchannelLogoutTokens(ctx, plan);
}
}
/**
* Asynchronous phase: sign one Logout Token per target client and POST it to
* the registered `backchannel_logout_uri`. The caller hands this to
@@ -290,7 +323,7 @@ async function deliverBackchannelLogoutTokens(
);
}
export { deliverBackchannelLogoutTokens, revokeAndPlanBackchannelLogout };
export { applyBackchannelLogoutPlan, prepareBackchannelLogoutPlan };
/**
* RP-Initiated Logout (OIDC RP-Initiated Logout 1.0). The RP presents a signed
@@ -298,7 +331,7 @@ export { deliverBackchannelLogoutTokens, revokeAndPlanBackchannelLogout };
* and optionally redirects to `post_logout_redirect_uri`.
*
* Session termination goes through `internalAdapter.deleteSession`, which fires
* `session.delete.before` so the hook drives revocation and back-channel
* `session.delete.after` so the hook drives revocation and back-channel
* notifications to every RP with tokens on the session.
*
* @see https://openid.net/specs/openid-connect-rpinitiated-1_0.html
+39 -16
View File
@@ -23,9 +23,10 @@ import { consentEndpoint } from "./consent";
import { continueEndpoint } from "./continue";
import { validateOAuthProviderExtensions } from "./extensions";
import { introspectEndpoint } from "./introspect";
import type { BackchannelLogoutPlan } from "./logout";
import {
deliverBackchannelLogoutTokens,
revokeAndPlanBackchannelLogout,
applyBackchannelLogoutPlan,
prepareBackchannelLogoutPlan,
rpInitiatedLogoutEndpoint,
} from "./logout";
import {
@@ -93,6 +94,10 @@ export const oAuthState = defineRequestState<{
postLoginClearedForSession?: string;
} | null>(() => null);
export const getOAuthProviderState = oAuthState.get;
const backchannelLogoutPlansByContext = new WeakMap<
GenericEndpointContext,
Map<string, BackchannelLogoutPlan>
>();
const signedQueryIssuedAtMsKey = "signedQueryIssuedAtMs";
function getServerContextSignedQueryIssuedAt(value: unknown) {
@@ -523,12 +528,8 @@ export const oauthProvider = <O extends OAuthOptions<Scope[]>>(options: O) => {
}
// The hook must register for every configuration path (including
// dynamic baseURL). Revocation runs inline because it mutates DB
// state we rely on. The HTTP fan-out goes through
// `runInBackgroundOrAwait`: with a background handler configured
// (Vercel `waitUntil`, CF `ctx.waitUntil`) it runs after the
// response; without one it is awaited inline so delivery is not lost
// on request teardown. Awaiting here keeps both paths reliable.
// dynamic baseURL). Core runs delete.after only after the session row is
// consumed and queues it until the surrounding transaction commits.
return {
options: {
databaseHooks: {
@@ -536,7 +537,7 @@ export const oauthProvider = <O extends OAuthOptions<Scope[]>>(options: O) => {
delete: {
async before(session, hookCtx) {
if (!hookCtx) return;
const plan = await revokeAndPlanBackchannelLogout(
const plan = await prepareBackchannelLogoutPlan(
hookCtx,
opts,
{
@@ -545,15 +546,37 @@ export const oauthProvider = <O extends OAuthOptions<Scope[]>>(options: O) => {
},
);
if (!plan) return;
// TODO: re-evaluate this await. It makes delivery reliable on
// every runtime, but without an `advanced.backgroundTasks.handler`
// a hung RP can add up to the per-RP timeout to sign-out latency.
// Alternative to weigh: keep delivery non-blocking and instead
// hard-require a background handler when back-channel logout is on.
await hookCtx.context.runInBackgroundOrAwait(
deliverBackchannelLogoutTokens(hookCtx, plan),
const logoutPlanBySessionId =
backchannelLogoutPlansByContext.get(hookCtx) ??
new Map<string, BackchannelLogoutPlan>();
logoutPlanBySessionId.set(session.id, plan);
backchannelLogoutPlansByContext.set(
hookCtx,
logoutPlanBySessionId,
);
},
async after(session, hookCtx) {
if (!hookCtx) return;
const logoutPlanBySessionId =
backchannelLogoutPlansByContext.get(hookCtx);
if (!logoutPlanBySessionId) return;
const plan = logoutPlanBySessionId.get(session.id);
logoutPlanBySessionId.delete(session.id);
if (logoutPlanBySessionId.size === 0) {
backchannelLogoutPlansByContext.delete(hookCtx);
}
if (!plan) return;
const logoutTask = applyBackchannelLogoutPlan(
hookCtx,
plan,
).catch((error) => {
hookCtx.context.logger.error(
"Back-channel logout failed after session deletion",
error,
);
});
await hookCtx.context.runInBackgroundOrAwait(logoutTask);
},
},
},
},
+3
View File
@@ -1472,6 +1472,9 @@ importers:
'@better-auth/core':
specifier: workspace:*
version: link:../core
'@better-auth/memory-adapter':
specifier: workspace:*
version: link:../memory-adapter
better-auth:
specifier: workspace:*
version: link:../better-auth