diff --git a/.changeset/preserved-session-delete-hooks.md b/.changeset/preserved-session-delete-hooks.md new file mode 100644 index 0000000000..7e7c842bb6 --- /dev/null +++ b/.changeset/preserved-session-delete-hooks.md @@ -0,0 +1,5 @@ +--- +"better-auth": minor +--- + +Signing out with `secondaryStorage` and `session.preserveSessionInDatabase` now runs your configured `session.delete` hooks and marks the preserved session row as ended. OAuth Provider access and refresh tokens bound to that session are revoked and back-channel logout is dispatched on sign-out. Previously these hooks were skipped in this setup, so the tokens stayed valid until they expired. diff --git a/docs/content/docs/concepts/session-management.mdx b/docs/content/docs/concepts/session-management.mdx index 75aaffdbb3..7208436612 100644 --- a/docs/content/docs/concepts/session-management.mdx +++ b/docs/content/docs/concepts/session-management.mdx @@ -338,7 +338,7 @@ export const auth = betterAuth({ ### Preserving Sessions When a session is revoked, it will be removed from the secondary storage, however if you enable `preserveSessionInDatabase`, -the session will be preserved in the database and not be deleted. +the session row will be kept in the database instead of deleted. The preserved row is marked ended (its `expiresAt` is set to the revocation time), so it is never restored as a live session and any token bound to it (for example OAuth Provider access tokens) is treated as ended. This is useful if you want to keep track of the sessions that have been revoked. diff --git a/docs/content/docs/plugins/oauth-provider.mdx b/docs/content/docs/plugins/oauth-provider.mdx index 52b75a70ff..7a8fffd17f 100644 --- a/docs/content/docs/plugins/oauth-provider.mdx +++ b/docs/content/docs/plugins/oauth-provider.mdx @@ -702,10 +702,6 @@ The Logout Token carries the ยง2.4 claims (`iss`, `aud`, `iat`, `exp`, `jti`, `e Delivery runs through `advanced.backgroundTasks.handler` when one is configured (Vercel `waitUntil`, Cloudflare `ctx.waitUntil`), so a slow RP cannot delay sign-out. Without a handler it completes inline before the sign-out response returns: reliable on persistent servers, but it can add latency when an RP is slow. Configure a handler on serverless runtimes. - - With `secondaryStorage` and `session.preserveSessionInDatabase: true`, session deletion keeps the database row and skips the session-delete hook, so tokens are not revoked and Logout Tokens are not dispatched on session end. Avoid that combination if you rely on back-channel logout. - - The OP advertises `backchannel_logout_supported: true` and `backchannel_logout_session_supported: true` on both `.well-known/openid-configuration` and `.well-known/oauth-authorization-server`. RPs use these fields during dynamic client registration to decide whether to register a `backchannel_logout_uri`. diff --git a/packages/better-auth/src/db/internal-adapter.ts b/packages/better-auth/src/db/internal-adapter.ts index e3ff369253..7a94b0d9d2 100644 --- a/packages/better-auth/src/db/internal-adapter.ts +++ b/packages/better-auth/src/db/internal-adapter.ts @@ -64,6 +64,38 @@ export const createInternalAdapter = ( consumeOneWithHooks, } = getWithHooks(adapter, ctx); + /** + * Ends the live session rows matched by `where` without physically deleting + * them. + * + * Used for `secondaryStorage` + `preserveSessionInDatabase`, where the row is + * kept for audit. The session-delete hooks still run (so OAuth token + * revocation and back-channel logout fire on session end), and the preserved + * row's `expiresAt` is set to now so every liveness check that keys off the + * session row (introspection, `/userinfo`) treats it as ended. + * + * Matching is restricted to still-live rows. The preserved row outlives the + * session, so without this a later delete call (a repeated `deleteSession`, + * or `deleteUserSessions` sweeping a user's accumulated preserved rows) would + * re-match it and re-fire the hooks, re-dispatching back-channel logout. + */ + const endPreservedSessions = (where: Where[]) => { + const liveSessions: Where[] = [ + ...where, + { field: "expiresAt", value: new Date(), operator: "gt" }, + ]; + return deleteManyWithHooks(liveSessions, "session", { + fn: async () => { + await (await getCurrentAdapter(adapter)).updateMany({ + model: "session", + where: liveSessions, + update: { expiresAt: new Date() }, + }); + }, + executeMainFn: false, + }); + }; + async function refreshUserSessions(user: User) { if (!secondaryStorage) return; @@ -732,10 +764,11 @@ export const createInternalAdapter = ( await secondaryStorage.delete(token); - if ( - !options.session?.storeSessionInDatabase || - ctx.options.session?.preserveSessionInDatabase - ) { + if (!options.session?.storeSessionInDatabase) { + return; + } + if (ctx.options.session?.preserveSessionInDatabase) { + await endPreservedSessions([{ field: "token", value: token }]); return; } } @@ -789,10 +822,11 @@ export const createInternalAdapter = ( } await secondaryStorage.delete(`active-sessions-${userId}`); - if ( - !options.session?.storeSessionInDatabase || - ctx.options.session?.preserveSessionInDatabase - ) { + if (!options.session?.storeSessionInDatabase) { + return; + } + if (ctx.options.session?.preserveSessionInDatabase) { + await endPreservedSessions([{ field: "userId", value: userId }]); return; } } @@ -816,10 +850,13 @@ export const createInternalAdapter = ( } } - if ( - !options.session?.storeSessionInDatabase || - ctx.options.session?.preserveSessionInDatabase - ) { + if (!options.session?.storeSessionInDatabase) { + return; + } + if (ctx.options.session?.preserveSessionInDatabase) { + await endPreservedSessions([ + { field: "token", value: sessionTokens, operator: "in" }, + ]); return; } } diff --git a/packages/better-auth/src/db/secondary-storage.test.ts b/packages/better-auth/src/db/secondary-storage.test.ts index 227bdf18fa..676a0eea0a 100644 --- a/packages/better-auth/src/db/secondary-storage.test.ts +++ b/packages/better-auth/src/db/secondary-storage.test.ts @@ -230,6 +230,66 @@ describe("secondary storage - storeSessionInDatabase", () => { const after = await client.getSession({ fetchOptions: { headers } }); expect(after.data).toBeNull(); }); + + it("runs the session-delete hook and marks the preserved row ended", async () => { + const store = new Map(); + const deletedSessionIds: string[] = []; + + const { auth, client, db, signInWithTestUser } = await getTestInstance({ + secondaryStorage: { + set(key, value) { + store.set(key, value); + }, + get(key) { + return store.get(key) || null; + }, + delete(key) { + store.delete(key); + }, + }, + session: { + storeSessionInDatabase: true, + preserveSessionInDatabase: true, + }, + databaseHooks: { + session: { + delete: { + before: async (session) => { + deletedSessionIds.push(session.id); + }, + }, + }, + }, + rateLimit: { enabled: false }, + }); + + const { headers } = await signInWithTestUser(); + const token = (await client.getSession({ fetchOptions: { headers } })) + .data!.session.token; + + await client.revokeSession({ fetchOptions: { headers }, token }); + + // The hook fires even though the row is preserved, so OAuth token + // revocation and back-channel logout run on session end. + expect(deletedSessionIds).toHaveLength(1); + + // The row is kept for audit but marked ended, so session-row liveness + // checks (introspection, /userinfo) treat bound tokens as inactive. + const preserved = await db.findOne<{ id: string; expiresAt: Date }>({ + model: "session", + where: [{ field: "token", value: token }], + }); + expect(preserved).not.toBeNull(); + expect(new Date(preserved!.expiresAt).getTime()).toBeLessThanOrEqual( + Date.now(), + ); + + // Ending an already-ended session must not re-fire the hook. The + // preserved row outlives the session, so a repeat delete would + // otherwise re-dispatch back-channel logout. + await (await auth.$context).internalAdapter.deleteSession(token); + expect(deletedSessionIds).toHaveLength(1); + }); }); }); diff --git a/packages/oauth-provider/src/backchannel-logout.test.ts b/packages/oauth-provider/src/backchannel-logout.test.ts index cd96766117..882e6568f9 100644 --- a/packages/oauth-provider/src/backchannel-logout.test.ts +++ b/packages/oauth-provider/src/backchannel-logout.test.ts @@ -477,3 +477,124 @@ describe("oauth back-channel logout (jwt plugin disabled)", async () => { for (const t of after) expect(t.revoked).toBeInstanceOf(Date); }); }); + +describe("oauth back-channel logout - secondaryStorage + preserveSessionInDatabase", async () => { + // With this topology, sign-out used to delete the secondary-storage entry and + // return before the session-delete hook, so OAuth tokens were never revoked + // and Logout Tokens were never dispatched while the preserved DB row stayed + // live. The hook must still fire on session end when the row is preserved. + const baseUrl = "http://localhost:3030"; + const redirectUri = `${baseUrl}/callback`; + const state = "preserve-state"; + const store = new Map(); + + const { auth, signInWithTestUser, customFetchImpl } = await getTestInstance({ + baseURL: baseUrl, + secondaryStorage: { + set(key, value) { + store.set(key, value); + }, + get(key) { + return store.get(key) || null; + }, + delete(key) { + store.delete(key); + }, + }, + session: { + storeSessionInDatabase: true, + preserveSessionInDatabase: true, + }, + plugins: [ + oauthProvider({ + loginPage: "/login", + consentPage: "/consent", + disableJwtPlugin: true, + silenceWarnings: { + oauthAuthServerConfig: true, + openidConfig: true, + }, + }), + ], + }); + const { headers } = await signInWithTestUser(); + const client = createAuthClient({ + plugins: [oauthProviderClient()], + baseURL: baseUrl, + fetchOptions: { customFetchImpl }, + }); + + it("revokes session-bound access tokens on sign-out when the row is preserved", async () => { + const oauthClient = await auth.api.adminCreateOAuthClient({ + headers, + body: { + redirect_uris: [redirectUri], + skip_consent: true, + enable_end_session: true, + }, + }); + if (!oauthClient?.client_id || !oauthClient?.client_secret) { + throw new Error("client registration failed"); + } + + const codeVerifier = generateRandomString(32); + const { url: authUrl } = await createAuthorizationURL({ + id: "test", + options: { + clientId: oauthClient.client_id, + clientSecret: oauthClient.client_secret, + redirectURI: redirectUri, + }, + redirectURI: "", + authorizationEndpoint: `${baseUrl}/api/auth/oauth2/authorize`, + state, + scopes: ["openid", "profile"], + codeVerifier, + }); + let callbackRedirectUrl = ""; + await client.$fetch(authUrl.toString(), { + headers, + onError(context) { + callbackRedirectUrl = context.response.headers.get("Location") || ""; + }, + }); + const code = new URL(callbackRedirectUrl).searchParams.get("code"); + if (!code) { + throw new Error(`no authorization code in ${callbackRedirectUrl}`); + } + const { body, headers: tokenHeaders } = await authorizationCodeRequest({ + code, + codeVerifier, + redirectURI: redirectUri, + options: { + clientId: oauthClient.client_id, + clientSecret: oauthClient.client_secret, + redirectURI: redirectUri, + }, + } satisfies MakeRequired< + Parameters[0], + "code" + >); + const tokens = await client.$fetch<{ access_token: string }>( + "/oauth2/token", + { method: "POST", body, headers: tokenHeaders }, + ); + expect(tokens.data?.access_token).toBeDefined(); + + const ctx = await auth.$context; + const before = await ctx.adapter.findMany<{ revoked?: Date | null }>({ + model: "oauthAccessToken", + where: [{ field: "clientId", value: oauthClient.client_id }], + }); + expect(before.length).toBeGreaterThan(0); + for (const t of before) expect(t.revoked ?? null).toBeNull(); + + await client.signOut({ fetchOptions: { headers } }); + + const after = await ctx.adapter.findMany<{ revoked?: Date | null }>({ + model: "oauthAccessToken", + where: [{ field: "clientId", value: oauthClient.client_id }], + }); + for (const t of after) expect(t.revoked).toBeInstanceOf(Date); + }); +}); diff --git a/packages/oauth-provider/src/oauth.ts b/packages/oauth-provider/src/oauth.ts index 67963a9554..2f41e0d4e5 100644 --- a/packages/oauth-provider/src/oauth.ts +++ b/packages/oauth-provider/src/oauth.ts @@ -287,23 +287,6 @@ export const oauthProvider = >(options: O) => { ); } - // With secondaryStorage + preserveSessionInDatabase, deleteSession - // keeps the DB row and returns before the session-delete hook runs, so - // OAuth token revocation and back-channel logout never fire on session - // end. Surface it so operators don't assume sign-out invalidates tokens. - // TODO: warning-only is a stopgap. A complete fix needs core to fire - // the session-delete hook (or expose a dedicated session-end seam) even - // when the row is preserved, so revocation and back-channel dispatch run - // for this config. Re-evaluate moving off the delete hook then. - if ( - ctx.options.secondaryStorage && - ctx.options.session?.preserveSessionInDatabase - ) { - logger.warn( - "OAuth Provider: `session.preserveSessionInDatabase: true` with secondaryStorage skips the session-delete hook, so OAuth access/refresh tokens are not revoked and back-channel logout is not dispatched on session end.", - ); - } - // Well-known warnings are best-effort and only make sense with the // JWT plugin. A dynamic baseURL resolves per-request, so there is // nothing to emit at init time for that deployment shape either.