From 92dfceaa14c287543f7cf62239641f4f7406cfb2 Mon Sep 17 00:00:00 2001 From: Gustavo Valverde Date: Fri, 9 Oct 2026 06:44:52 +0000 Subject: [PATCH] fix(session): defer secondary-storage writes only when the database holds the session (#11643) --- .changeset/sign-up-deferred-session-cache.md | 6 + .../src/api/routes/sign-up.test.ts | 140 ++++++++++++++++++ .../better-auth/src/api/routes/sign-up.ts | 3 + .../src/db/internal-adapter.test.ts | 41 +++++ .../better-auth/src/db/internal-adapter.ts | 80 ++++++---- packages/core/src/types/context.ts | 12 +- 6 files changed, 254 insertions(+), 28 deletions(-) create mode 100644 .changeset/sign-up-deferred-session-cache.md diff --git a/.changeset/sign-up-deferred-session-cache.md b/.changeset/sign-up-deferred-session-cache.md new file mode 100644 index 0000000000..81a0e5f8e0 --- /dev/null +++ b/.changeset/sign-up-deferred-session-cache.md @@ -0,0 +1,6 @@ +--- +"better-auth": patch +"@better-auth/core": patch +--- + +A sign-up that fails after its session is created no longer leaves that session in secondary storage. With `storeSessionInDatabase` enabled and `preserveSessionInDatabase` off, the secondary-storage copy is written only after the sign-up commits, and listing a user's sessions now reads the database, so a session whose cached copy failed to write is still listed and revoked. With other secondary-storage settings, sign-up, passkey registration and SSO sign-in with `resolveUser` keep the session write inside their transaction, so a storage failure rolls back the new user, passkey or account instead of keeping them without a session. A session revoked by a `session.create.after` database hook also stays revoked. diff --git a/packages/better-auth/src/api/routes/sign-up.test.ts b/packages/better-auth/src/api/routes/sign-up.test.ts index 5d5fc268d4..c883bb6177 100644 --- a/packages/better-auth/src/api/routes/sign-up.test.ts +++ b/packages/better-auth/src/api/routes/sign-up.test.ts @@ -200,6 +200,146 @@ describe("sign-up with custom fields", async () => { }); }); +/** + * @see https://github.com/better-auth/better-auth/issues/11642 + */ +describe("sign-up session caching", () => { + function createStorage(store: Map, shouldFail = () => false) { + return { + set(key: string, value: string) { + if (shouldFail()) throw new Error("storage unavailable"); + store.set(key, value); + }, + get(key: string) { + return store.get(key) || null; + }, + getAndDelete(key: string) { + const value = store.get(key) || null; + store.delete(key); + return value; + }, + increment(key: string) { + const count = Number(store.get(key) ?? 0) + 1; + store.set(key, String(count)); + return count; + }, + delete(key: string) { + store.delete(key); + }, + }; + } + + const body = { + email: "cache-rollback@test.com", + password: "password", + name: "Cache Rollback", + }; + + it("does not cache the session when the sign-up rolls back after creating it", async () => { + const store = new Map(); + const { auth, db } = await getTestInstance( + { + secondaryStorage: createStorage(store), + session: { storeSessionInDatabase: true }, + }, + { disableTestUser: true }, + ); + const ctx = await auth.$context; + const transaction = ctx.adapter.transaction; + vi.spyOn(ctx.adapter, "transaction").mockImplementation((cb) => + transaction(async (trx) => { + await cb(trx); + throw new Error("rolled back"); + }), + ); + + await expect(auth.api.signUpEmail({ body })).rejects.toThrow("rolled back"); + + expect([...store.keys()]).toEqual([]); + const users = await db.findMany<{ email: string }>({ model: "user" }); + expect(users.find((u) => u.email === body.email)).toBeUndefined(); + }); + + it("rolls back the user when secondary storage cannot store the session", async () => { + const store = new Map(); + const { auth, db } = await getTestInstance( + { secondaryStorage: createStorage(store, () => true) }, + { disableTestUser: true }, + ); + + await expect(auth.api.signUpEmail({ body })).rejects.toThrow(); + + const users = await db.findMany<{ email: string }>({ model: "user" }); + expect(users.find((u) => u.email === body.email)).toBeUndefined(); + }); + + it("rolls back the user when a preserved session cannot be cached", async () => { + const store = new Map(); + const { auth, db } = await getTestInstance( + { + secondaryStorage: createStorage(store, () => true), + session: { + storeSessionInDatabase: true, + preserveSessionInDatabase: true, + }, + }, + { disableTestUser: true }, + ); + + await expect(auth.api.signUpEmail({ body })).rejects.toThrow(); + + const users = await db.findMany<{ email: string }>({ model: "user" }); + expect(users.find((u) => u.email === body.email)).toBeUndefined(); + }); + + it("keeps a session revoked by its create.after hook revoked", async () => { + const store = new Map(); + let revoke: (token: string) => Promise = async () => {}; + const { auth } = await getTestInstance( + { + secondaryStorage: createStorage(store), + session: { storeSessionInDatabase: true }, + databaseHooks: { + session: { + create: { + async after(session) { + await revoke(session.token); + }, + }, + }, + }, + }, + { disableTestUser: true }, + ); + const ctx = await auth.$context; + revoke = (token) => ctx.internalAdapter.deleteSession(token); + + const { token } = await auth.api.signUpEmail({ body }); + + expect(token).toBeTruthy(); + expect(await ctx.internalAdapter.findSession(token!)).toBeNull(); + }); + + it("lists the session when its cached copy failed to write", async () => { + const store = new Map(); + let failWrites = true; + const { auth } = await getTestInstance( + { + secondaryStorage: createStorage(store, () => failWrites), + session: { storeSessionInDatabase: true }, + }, + { disableTestUser: true }, + ); + const ctx = await auth.$context; + + const { token, user } = await auth.api.signUpEmail({ body }); + failWrites = false; + + const sessions = await ctx.internalAdapter.listSessions(user.id); + expect(sessions.map((session) => session.token)).toEqual([token]); + }); +}); + describe("validateUserInfo sign-up", async () => { it("should allow email/password sign-up when validateUserInfo returns void", async () => { let capturedSourceType: string | undefined; diff --git a/packages/better-auth/src/api/routes/sign-up.ts b/packages/better-auth/src/api/routes/sign-up.ts index 629ca8513c..30d5345918 100644 --- a/packages/better-auth/src/api/routes/sign-up.ts +++ b/packages/better-auth/src/api/routes/sign-up.ts @@ -420,6 +420,9 @@ export const signUpEmail = () => const session = await ctx.context.internalAdapter.createSession( createdUser.id, rememberMe === false, + undefined, + undefined, + { deferSecondaryStorageWrites: true }, ); if (!session) { throw APIError.from( diff --git a/packages/better-auth/src/db/internal-adapter.test.ts b/packages/better-auth/src/db/internal-adapter.test.ts index 9a88e8fb43..d681970ac8 100644 --- a/packages/better-auth/src/db/internal-adapter.test.ts +++ b/packages/better-auth/src/db/internal-adapter.test.ts @@ -1261,6 +1261,47 @@ describe("internal adapter test", async () => { ).rejects.toThrow("secondary storage unavailable"); }); + /** + * @see https://github.com/better-auth/better-auth/issues/11642 + */ + it("keeps a secondary-only session write inside the transaction even when deferral is requested", async () => { + const testMap = new Map(); + const secondaryStorage = createStringSecondaryStorage(testMap); + const testOpts = { + database: new DatabaseSync(":memory:"), + secondaryStorage: { + ...secondaryStorage, + set: async () => { + throw new Error("secondary storage unavailable"); + }, + }, + } satisfies BetterAuthOptions; + (await getMigrations(testOpts)).runMigrations(); + const testCtx = await init(testOpts); + + await expect( + runWithTransaction(testCtx.adapter, async () => { + const user = await testCtx.internalAdapter.createUser( + { + name: "rolled-back-user", + email: "rolled-back@example.com", + }, + { method: "test" }, + ); + await testCtx.internalAdapter.createSession( + user.id, + undefined, + undefined, + undefined, + { deferSecondaryStorageWrites: true }, + ); + }), + ).rejects.toThrow("secondary storage unavailable"); + expect( + await testCtx.internalAdapter.findUserByEmail("rolled-back@example.com"), + ).toBeNull(); + }); + /** * @see https://github.com/better-auth/better-auth/pull/10390#discussion_r3585595438 */ diff --git a/packages/better-auth/src/db/internal-adapter.ts b/packages/better-auth/src/db/internal-adapter.ts index da470b525d..e5772979e6 100644 --- a/packages/better-auth/src/db/internal-adapter.ts +++ b/packages/better-auth/src/db/internal-adapter.ts @@ -64,6 +64,12 @@ export const createInternalAdapter = ( const secondaryStorage = options.secondaryStorage; const verificationConsumeLocks = new Map>(); const sessionExpiration = options.session?.expiresIn || 60 * 60 * 24 * 7; // 7 days + // With secondary storage, the database is the authoritative session store + // only when sessions are written to it and revoked rows are deleted. + const databaseSessionFallbackEnabled = + !!secondaryStorage && + options.session?.storeSessionInDatabase === true && + options.session?.preserveSessionInDatabase !== true; const { createWithHooks, updateWithHooks, @@ -330,6 +336,23 @@ export const createInternalAdapter = ( userId: string, options?: { onlyActiveSessions?: boolean | undefined } | undefined, ) => { + if (databaseSessionFallbackEnabled) { + // The database holds every session, including ones whose cached copy + // failed to write, so list from it. Expired rows are skipped, as the + // cached list does. + const sessions = await ( + await getCurrentAdapter(adapter) + ).findMany({ + model: "session", + where: [ + { field: "userId", value: userId }, + { field: "expiresAt", value: new Date(), operator: "gt" }, + ], + }); + return sessions.map((session) => + parseSessionOutput(ctx.options, session), + ); + } if (secondaryStorage) { const currentList = await secondaryStorage.get( `active-sessions-${userId}`, @@ -477,9 +500,12 @@ export const createInternalAdapter = ( return ctx?.headers || ctx?.request?.headers; })(); const storeInDb = options.session?.storeSessionInDatabase; - const databaseSessionFallbackEnabled = - storeInDb === true && - options.session?.preserveSessionInDatabase !== true; + // Deferring is only safe when the database can serve the session. When + // secondary storage is the session store, its write is the session + // itself and must fail inside the caller's transaction. + const deferMirror = + storageOptions?.deferSecondaryStorageWrites === true && + databaseSessionFallbackEnabled; const { // always ignore override id - new sessions must have new ids id: _, @@ -568,35 +594,35 @@ export const createInternalAdapter = ( secondaryStorage ? { fn: async (sessionData) => { - return storageOptions?.deferSecondaryStorageWrites - ? sessionData - : mirrorSessionToSecondaryStorage(sessionData as Session); + if (!deferMirror) { + return mirrorSessionToSecondaryStorage( + sessionData as Session, + ); + } + // Queued here, before createWithHooks queues the session's + // after hooks, so a hook that revokes the session runs after + // the cached copy exists and removes it. + await queueAfterTransactionHook( + async () => { + await mirrorSessionToSecondaryStorage( + sessionData as Session, + ); + }, + { + onError(error: unknown) { + logger.error( + "Failed to mirror committed session to secondary storage", + error, + ); + }, + }, + ); + return sessionData; }, executeMainFn: storeInDb, } : undefined, ); - if ( - secondaryStorage && - storageOptions?.deferSecondaryStorageWrites && - res - ) { - await queueAfterTransactionHook( - async () => { - await mirrorSessionToSecondaryStorage(res as Session); - }, - databaseSessionFallbackEnabled - ? { - onError(error: unknown) { - logger.error( - "Failed to mirror committed session to secondary storage", - error, - ); - }, - } - : undefined, - ); - } return res as Session; }, findSession: async ( diff --git a/packages/core/src/types/context.ts b/packages/core/src/types/context.ts index d4d85e3bb7..37210685a0 100644 --- a/packages/core/src/types/context.ts +++ b/packages/core/src/types/context.ts @@ -157,7 +157,17 @@ export interface InternalAdapter< override?: (Partial & Record) | undefined, overrideAll?: boolean | undefined, storageOptions?: - | { deferSecondaryStorageWrites?: boolean | undefined } + | { + /** + * Write the secondary-storage copy only after the current + * transaction commits, so a rollback can't leave it cached. + * Ignored when secondary storage is the session store + * (`storeSessionInDatabase` off or `preserveSessionInDatabase` + * on): the write then stays inside the transaction so a failure + * rolls it back. + */ + deferSecondaryStorageWrites?: boolean | undefined; + } | undefined, ): Promise;