fix(session): defer secondary-storage writes only when the database holds the session (#11643)

.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.

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<string, string>, 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<string, string>();
+		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<string, string>();
+		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<string, string>();
+		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<string, string>();
+		let revoke: (token: string) => Promise<void> = 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<string, string>();
+		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;

packages/better-auth/src/api/routes/sign-up.ts

@@ -420,6 +420,9 @@ export const signUpEmail = <O extends BetterAuthOptions>() =>
 				const session = await ctx.context.internalAdapter.createSession(
 					createdUser.id,
 					rememberMe === false,
+					undefined,
+					undefined,
+					{ deferSecondaryStorageWrites: true },
 				);
 				if (!session) {
 					throw APIError.from(

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<string, string>();
+		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
 	 */

packages/better-auth/src/db/internal-adapter.ts

@@ -64,6 +64,12 @@ export const createInternalAdapter = (
 	const secondaryStorage = options.secondaryStorage;
 	const verificationConsumeLocks = new Map<string, Promise<void>>();
 	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<Session>({
+					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 (

packages/core/src/types/context.ts

@@ -157,7 +157,17 @@ export interface InternalAdapter<
 		override?: (Partial<Session> & Record<string, any>) | 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<Session>;