diff --git a/packages/better-auth/src/db/internal-adapter.test.ts b/packages/better-auth/src/db/internal-adapter.test.ts index a6b683c281..40f549def7 100644 --- a/packages/better-auth/src/db/internal-adapter.test.ts +++ b/packages/better-auth/src/db/internal-adapter.test.ts @@ -28,6 +28,7 @@ import type { UserProvisioningSource, } from "../types"; import { getMigrations } from "./get-migration"; +import { revokeUnprovenAccountAccess } from "./revoke-unproven-account-access"; function createStringSecondaryStorage( store: Map, @@ -381,6 +382,118 @@ describe("internal adapter test", async () => { const authContext = await init(opts); const internalAdapter = authContext.internalAdapter; + it("retries account cleanup when another cleanup lock outlives the wait deadline", async () => { + vi.useFakeTimers(); + const database = new DatabaseSync(":memory:"); + const testOpts = { database } satisfies BetterAuthOptions; + (await getMigrations(testOpts)).runMigrations(); + const context = await init(testOpts); + const endpointContext = { + context, + } as unknown as GenericEndpointContext; + const user = await context.internalAdapter.createUser( + { + email: "cleanup-lock@example.com", + name: "Cleanup Lock", + emailVerified: false, + }, + { method: "test" }, + ); + const lockIdentifier = `revoke-unproven-account-access:${user.id}`; + expect(user.emailVerified).toBe(false); + await context.internalAdapter.linkAccount( + user.id, + { + issuer: "https://cleanup-lock.example.com", + providerAccountId: "cleanup-lock-provider-user", + }, + { + providerId: "cleanup-lock-provider", + providerInstanceId: "cleanup-lock-provider", + }, + ); + await context.internalAdapter.createSession(user.id); + await context.internalAdapter.reserveVerificationValue({ + identifier: lockIdentifier, + value: user.id, + expiresAt: new Date(Date.now() + 5_000), + }); + expect( + await context.internalAdapter.reserveVerificationValue({ + identifier: lockIdentifier, + value: user.id, + expiresAt: new Date(Date.now() + 5_000), + }), + ).toBe(false); + + const reserveSpy = vi.spyOn( + context.internalAdapter, + "reserveVerificationValue", + ); + try { + const cleanup = revokeUnprovenAccountAccess(endpointContext, user.id); + while (reserveSpy.mock.results.length === 0) { + await Promise.resolve(); + } + await reserveSpy.mock.results[0]!.value; + await vi.advanceTimersByTimeAsync(0); + await vi.advanceTimersByTimeAsync(2_250); + await context.internalAdapter.deleteVerificationByIdentifier( + lockIdentifier, + ); + await vi.advanceTimersByTimeAsync(250); + + await expect(cleanup).resolves.toMatchObject({ emailVerified: true }); + await expect( + context.internalAdapter.findUserById(user.id), + ).resolves.toMatchObject({ emailVerified: true }); + await expect( + context.internalAdapter.listUserAccounts(user.id), + ).resolves.toEqual([]); + await expect( + context.internalAdapter.listSessions(user.id), + ).resolves.toEqual([]); + } finally { + reserveSpy.mockRestore(); + vi.useRealTimers(); + database.close(); + } + }); + + it("returns null when a cached user update matches no row", async () => { + const database = new DatabaseSync(":memory:"); + const log = vi.fn(); + const testOpts = { + database, + logger: { log }, + secondaryStorage: createStringSecondaryStorage(new Map()), + } satisfies BetterAuthOptions; + (await getMigrations(testOpts)).runMigrations(); + const context = await init(testOpts); + + try { + await expect( + context.internalAdapter.updateUser("missing-user", { + name: "Missing User", + }), + ).resolves.toBeNull(); + await expect( + context.internalAdapter.updateUserByEmail("missing@example.com", { + name: "Missing User", + }), + ).resolves.toBeNull(); + expect(log).not.toHaveBeenCalledWith( + "error", + expect.stringContaining( + "Failed to refresh committed user sessions in secondary storage", + ), + expect.anything(), + ); + } finally { + database.close(); + } + }); + it("creates a user with one identity and one account", async () => { const created = await internalAdapter.createUserWithAccount( { @@ -3337,6 +3450,29 @@ describe("internal adapter test", async () => { ); }); + it("findSession should return null for valid JSON with a malformed session shape", async () => { + const testMap = new Map(); + const testOpts = { + database: new DatabaseSync(":memory:"), + secondaryStorage: createStringSecondaryStorage(testMap), + } satisfies BetterAuthOptions; + (await getMigrations(testOpts)).runMigrations(); + const testInternalAdapter = (await init(testOpts)).internalAdapter; + const user = await testInternalAdapter.createUser( + { + name: "test-user-malformed-find", + email: "test-malformed-find@email.com", + }, + { method: "test" }, + ); + const session = await testInternalAdapter.createSession(user.id); + testMap.set(session.token, JSON.stringify({ session: null, user: null })); + + await expect( + testInternalAdapter.findSession(session.token), + ).resolves.toBeNull(); + }); + it("listSessions should skip corrupt/unparsable sessions without blanking the list", async () => { const testMap = new Map(); @@ -3407,7 +3543,7 @@ describe("internal adapter test", async () => { expect(sessions.length).toBe(0); }); - it("findSessions should skip corrupt sessions without blanking the list", async () => { + it("findSessions should skip malformed JSON values without blanking the list", async () => { const testMap = new Map(); const testOpts = { @@ -3433,8 +3569,8 @@ describe("internal adapter test", async () => { const session2 = await testInternalAdapter.createSession(user.id); const session3 = await testInternalAdapter.createSession(user.id); - // Corrupt session2 data - testMap.set(session2.token, "invalid-json{{{"); + // JSON.parse succeeds, but the value is not a cached session. + testMap.set(session2.token, "null"); // findSessions should still return session1 and session3 const sessions = await testInternalAdapter.findSessions([ @@ -3545,6 +3681,41 @@ describe("internal adapter test", async () => { expect(updatedTTL! - expectedTTL).toBeGreaterThanOrEqual(0); }); + it("should evict an expired session from secondary storage when it is updated", async () => { + const testMap = new Map(); + const testOpts = { + database: new DatabaseSync(":memory:"), + secondaryStorage: createStringSecondaryStorage(testMap), + } satisfies BetterAuthOptions; + (await getMigrations(testOpts)).runMigrations(); + const testInternalAdapter = (await init(testOpts)).internalAdapter; + const user = await testInternalAdapter.createUser( + { + name: "test-user-expired-update", + email: "test-expired-update@email.com", + }, + { method: "test" }, + ); + const expiredSession = await testInternalAdapter.createSession(user.id); + const liveSession = await testInternalAdapter.createSession(user.id); + + await testInternalAdapter.updateSession(expiredSession.token, { + expiresAt: new Date(Date.now() - 1_000), + }); + + expect(testMap.has(expiredSession.token)).toBe(false); + expect(testMap.has(liveSession.token)).toBe(true); + const activeSessions = safeJSONParse< + { token: string; expiresAt: number }[] + >(testMap.get(`active-sessions-${user.id}`)!); + expect(activeSessions?.map(({ token }) => token)).toEqual([ + liveSession.token, + ]); + await expect( + testInternalAdapter.findSession(expiredSession.token), + ).resolves.toBeNull(); + }); + it("should deduplicate sessions when active-sessions list contains duplicates", async () => { const testMap = new Map(); const testExpirationMap = new Map(); @@ -4796,6 +4967,42 @@ describe("internal adapter test", async () => { expect(second).toBe(true); }); + it("rolls back reservation writes and their secondary-storage entries", async () => { + const database = new DatabaseSync(":memory:"); + const secondaryStore = new Map(); + const opts = { + database, + secondaryStorage: createStringSecondaryStorage(secondaryStore), + verification: { storeInDatabase: true }, + } satisfies BetterAuthOptions; + (await getMigrations(opts)).runMigrations(); + const ctx = await init(opts); + const reservation = { + identifier: "reserve:rollback", + value: "jti-rollback", + expiresAt: new Date(Date.now() + 60_000), + }; + + try { + await expect( + runWithTransaction(ctx.adapter, async () => { + expect( + await ctx.internalAdapter.reserveVerificationValue(reservation), + ).toBe(true); + throw new Error("rollback reservation"); + }), + ).rejects.toThrow("rollback reservation"); + + expect(secondaryStore.size).toBe(0); + expect( + await ctx.internalAdapter.reserveVerificationValue(reservation), + ).toBe(true); + expect(secondaryStore.size).toBe(1); + } finally { + database.close(); + } + }); + it("fails closed when verification reservation is secondary-storage-only", async () => { const adapter = await makeAdapter({ verification: { storeInDatabase: false }, diff --git a/packages/better-auth/src/db/internal-adapter/sessions.ts b/packages/better-auth/src/db/internal-adapter/sessions.ts index a3c9d848bb..47fe6e54b1 100644 --- a/packages/better-auth/src/db/internal-adapter/sessions.ts +++ b/packages/better-auth/src/db/internal-adapter/sessions.ts @@ -61,6 +61,30 @@ type CachedSession = { sessionVersion?: string | undefined; }; +function isRecord(value: unknown): value is Record { + return typeof value === "object" && value !== null; +} + +function parseCachedSession(value: unknown): CachedSession | null { + const parsed = + typeof value === "string" ? safeJSONParse(value) : value; + if ( + !isRecord(parsed) || + !isRecord(parsed.session) || + !isRecord(parsed.user) + ) { + return null; + } + if ( + typeof parsed.session.token !== "string" || + typeof parsed.session.userId !== "string" || + typeof parsed.user.id !== "string" + ) { + return null; + } + return parsed as CachedSession; +} + const INITIAL_SESSION_VERSION = "initial"; function getSessionVersionKey(userId: string): string { @@ -194,7 +218,7 @@ export function createSessionAdapterModule( validSessions.map(async ({ token }) => { const cached = await secondaryStorage.get(token); if (!cached) return; - const parsed = safeJSONParse(cached); + const parsed = parseCachedSession(cached); if (!parsed) return; if (!(await isCurrentCachedSession(parsed))) { return; @@ -287,10 +311,8 @@ export function createSessionAdapterModule( if (!data) continue; try { - const parsed = ( - typeof data === "string" ? JSON.parse(data) : data - ) as CachedSession; - if (!parsed?.session) continue; + const parsed = parseCachedSession(data); + if (!parsed) continue; if (!(await isCurrentCachedSession(parsed))) { continue; } @@ -473,7 +495,7 @@ export function createSessionAdapterModule( return null; } if (sessionStringified) { - const s = safeJSONParse(sessionStringified); + const s = parseCachedSession(sessionStringified); if (!s) return null; if (!(await isCurrentCachedSession(s))) { return null; @@ -539,12 +561,8 @@ export function createSessionAdapterModule( const sessionStringified = await secondaryStorage.get(sessionToken); if (sessionStringified) { try { - const s = ( - typeof sessionStringified === "string" - ? JSON.parse(sessionStringified) - : sessionStringified - ) as CachedSession; - if (!s) return []; + const s = parseCachedSession(sessionStringified); + if (!s) continue; if (!(await isCurrentCachedSession(s))) { continue; } @@ -628,8 +646,7 @@ export function createSessionAdapterModule( return null; } - const parsedSession = - safeJSONParse(currentSession); + const parsedSession = parseCachedSession(currentSession); if (!parsedSession) return null; if (!(await isCurrentCachedSession(parsedSession))) { return null; @@ -666,33 +683,34 @@ export function createSessionAdapterModule( }), sessionTTL, ); + } else { + await secondaryStorage.delete(sessionToken); + } - const listKey = `active-sessions-${updatedSession.userId}`; - const listRaw = await secondaryStorage.get(listKey); - const list: { token: string; expiresAt: number }[] = listRaw - ? safeJSONParse(listRaw) || [] - : []; + const listKey = `active-sessions-${updatedSession.userId}`; + const listRaw = await secondaryStorage.get(listKey); + const list: { token: string; expiresAt: number }[] = listRaw + ? safeJSONParse(listRaw) || [] + : []; + const filtered = list.filter( + (s) => s.token !== sessionToken && s.expiresAt > now, + ); + if (sessionTTL > 0) { + filtered.push({ token: sessionToken, expiresAt: expiresMs }); + } - const filtered = list - .filter( - (s) => s.token !== sessionToken && s.expiresAt > now, - ) - .concat([{ token: sessionToken, expiresAt: expiresMs }]); - - const sorted = filtered.sort( - (a, b) => a.expiresAt - b.expiresAt, + const sorted = filtered.sort( + (a, b) => a.expiresAt - b.expiresAt, + ); + const furthestSessionExp = sorted.at(-1)?.expiresAt; + if (furthestSessionExp && furthestSessionExp > now) { + await secondaryStorage.set( + listKey, + JSON.stringify(sorted), + getTTLSeconds(furthestSessionExp, now), ); - const furthestSessionExp = sorted.at(-1)?.expiresAt; - - if (furthestSessionExp && furthestSessionExp > now) { - await secondaryStorage.set( - listKey, - JSON.stringify(sorted), - getTTLSeconds(furthestSessionExp, now), - ); - } else { - await secondaryStorage.delete(listKey); - } + } else { + await secondaryStorage.delete(listKey); } return updatedSession; diff --git a/packages/better-auth/src/db/internal-adapter/user-graph.ts b/packages/better-auth/src/db/internal-adapter/user-graph.ts index ccfe56cc83..25b2fea166 100644 --- a/packages/better-auth/src/db/internal-adapter/user-graph.ts +++ b/packages/better-auth/src/db/internal-adapter/user-graph.ts @@ -1435,14 +1435,16 @@ export function createUserGraphAdapterMethods< "user", undefined, ); - await queueAfterTransactionHook(() => refreshUserSessions(user), { - onError(error) { - logger.error( - "Failed to refresh committed user sessions in secondary storage", - error, - ); - }, - }); + if (user) { + await queueAfterTransactionHook(() => refreshUserSessions(user), { + onError(error) { + logger.error( + "Failed to refresh committed user sessions in secondary storage", + error, + ); + }, + }); + } return user; }, updateUserByEmail: async ( @@ -1463,14 +1465,16 @@ export function createUserGraphAdapterMethods< "user", undefined, ); - await queueAfterTransactionHook(() => refreshUserSessions(user), { - onError(error) { - logger.error( - "Failed to refresh committed user sessions in secondary storage", - error, - ); - }, - }); + if (user) { + await queueAfterTransactionHook(() => refreshUserSessions(user), { + onError(error) { + logger.error( + "Failed to refresh committed user sessions in secondary storage", + error, + ); + }, + }); + } return user; }, updatePassword: async (userId: string, password: string) => { diff --git a/packages/better-auth/src/db/internal-adapter/verification.ts b/packages/better-auth/src/db/internal-adapter/verification.ts index 07f6e70850..c50adc78b8 100644 --- a/packages/better-auth/src/db/internal-adapter/verification.ts +++ b/packages/better-auth/src/db/internal-adapter/verification.ts @@ -1,6 +1,7 @@ import type { BetterAuthOptions, InternalAdapter } from "@better-auth/core"; import { getCurrentAdapter, + queueAfterTransactionHook, runAtomicMutation, } from "@better-auth/core/context"; import type { @@ -476,9 +477,10 @@ export function createVerificationAdapterMethods< "reserveVerificationValue requires database-backed verification storage. Set verification.storeInDatabase to true for flows that reserve verification values.", ); } + const currentAdapter = await getCurrentAdapter(adapter); try { - await adapter.create({ + await currentAdapter.create({ model: "verification", data: { id: reservationId, @@ -494,7 +496,7 @@ export function createVerificationAdapterMethods< // A create error is ambiguous across adapters: confirm it was a // duplicate (the row exists) rather than a real failure before // reporting "lost". - const existing = await adapter.findOne({ + const existing = await currentAdapter.findOne({ model: "verification", where: [{ field: "id", value: reservationId }], }); @@ -505,16 +507,18 @@ export function createVerificationAdapterMethods< if (secondaryStorage) { const ttl = getTTLSeconds(data.expiresAt); if (ttl > 0) { - await secondaryStorage.set( - `verification:${storedIdentifier}`, - JSON.stringify({ - id: reservationId, - identifier: storedIdentifier, - value: data.value, - expiresAt: data.expiresAt, - }), - ttl, - ); + await queueAfterTransactionHook(async () => { + await secondaryStorage.set( + `verification:${storedIdentifier}`, + JSON.stringify({ + id: reservationId, + identifier: storedIdentifier, + value: data.value, + expiresAt: data.expiresAt, + }), + ttl, + ); + }); } } diff --git a/packages/better-auth/src/db/revoke-unproven-account-access.ts b/packages/better-auth/src/db/revoke-unproven-account-access.ts index fa99bc2c03..34b568c5f3 100644 --- a/packages/better-auth/src/db/revoke-unproven-account-access.ts +++ b/packages/better-auth/src/db/revoke-unproven-account-access.ts @@ -47,24 +47,30 @@ export async function revokeUnprovenAccountAccess( userId: string, ): Promise { const lockIdentifier = cleanupLockIdentifier(userId); - const lockAcquired = await ctx.context.internalAdapter - .reserveVerificationValue({ - identifier: lockIdentifier, - value: userId, - expiresAt: new Date(Date.now() + cleanupLockExpiresInMs), - }) - .catch((error) => { - if ( - error instanceof BetterAuthError && - error.message.includes("requires database-backed verification storage") - ) { - return true; - } - throw error; - }); - if (!lockAcquired) { + let lockAcquired = false; + while (!lockAcquired) { + lockAcquired = await ctx.context.internalAdapter + .reserveVerificationValue({ + identifier: lockIdentifier, + value: userId, + expiresAt: new Date(Date.now() + cleanupLockExpiresInMs), + }) + .catch((error) => { + if ( + error instanceof BetterAuthError && + error.message.includes( + "requires database-backed verification storage", + ) + ) { + return true; + } + throw error; + }); + if (lockAcquired) break; + await waitForCleanupLock(ctx, lockIdentifier); - return ctx.context.internalAdapter.findUserById(userId); + const currentUser = await ctx.context.internalAdapter.findUserById(userId); + if (!currentUser || currentUser.emailVerified) return currentUser; } try { diff --git a/packages/better-auth/src/plugins/oauth-proxy/index.ts b/packages/better-auth/src/plugins/oauth-proxy/index.ts index 6fd22c8167..63d970d037 100644 --- a/packages/better-auth/src/plugins/oauth-proxy/index.ts +++ b/packages/better-auth/src/plugins/oauth-proxy/index.ts @@ -284,6 +284,12 @@ export const oAuthProxy = (opts?: O) => { throw redirectOnError(ctx, errorURL, "state_mismatch"); } + const provider = ctx.context.socialProviders.find( + (provider) => + provider.id === + (payload.account.providerInstanceId || + payload.account.providerId), + ); let result: Awaited>; try { result = await authenticateProviderUser(ctx, { @@ -292,6 +298,7 @@ export const oAuthProxy = (opts?: O) => { account: payload.account, callbackURL: payload.callbackURL, disableSignUp: payload.disableSignUp, + overrideUserInfo: provider?.options?.overrideUserInfoOnSignIn, source: { method: "oauth", oauth: { diff --git a/packages/better-auth/src/plugins/oauth-proxy/oauth-proxy.test.ts b/packages/better-auth/src/plugins/oauth-proxy/oauth-proxy.test.ts index 0deabb20c0..1386a8de2e 100644 --- a/packages/better-auth/src/plugins/oauth-proxy/oauth-proxy.test.ts +++ b/packages/better-auth/src/plugins/oauth-proxy/oauth-proxy.test.ts @@ -739,6 +739,90 @@ describe("oauth-proxy", async () => { expect(previewSessions.length).toBe(1); }); + it("should override existing user info when the proxy provider enables it", async () => { + const production = await getTestInstance( + { + baseURL: "http://localhost:3000", + plugins: [oAuthProxy()], + socialProviders: { + google: { + clientId: "test", + clientSecret: "test", + }, + }, + }, + { disableTestUser: true }, + ); + const preview = await getTestInstance( + { + baseURL: "http://preview.example.com", + plugins: [oAuthProxy({ productionURL: "http://localhost:3000" })], + socialProviders: { + google: { + clientId: "test", + clientSecret: "test", + overrideUserInfoOnSignIn: true, + }, + }, + }, + { disableTestUser: true }, + ); + const previewContext = await preview.auth.$context; + const existingUser = await previewContext.internalAdapter.createUser( + { + email: "user@email.com", + name: "Existing User", + emailVerified: true, + }, + { method: "test" }, + ); + await previewContext.internalAdapter.linkAccount( + existingUser.id, + { + issuer: "https://accounts.google.com", + providerAccountId: "1234567890", + }, + { providerId: "google", providerInstanceId: "google" }, + ); + + const signIn = await preview.client.signIn.social( + { provider: "google", callbackURL: "/dashboard" }, + { throw: true }, + ); + const state = new URL(signIn.url!).searchParams.get("state"); + let encryptedProfile: string | null = null; + let callbackURL: string | null = null; + await production.client.$fetch( + `/callback/google?code=test&state=${state}`, + { + onError(context) { + const location = context.response.headers.get("location"); + if (location?.includes("profile=")) { + const url = new URL(location); + encryptedProfile = url.searchParams.get("profile"); + callbackURL = url.searchParams.get("callbackURL"); + } + }, + }, + ); + expect(encryptedProfile).toBeTruthy(); + + await preview.client.$fetch( + `/oauth-proxy-callback?callbackURL=${encodeURIComponent(callbackURL!)}&profile=${encodeURIComponent(encryptedProfile!)}`, + { + onError(context) { + expect(context.response.headers.get("location")).toContain( + "/dashboard", + ); + }, + }, + ); + + await expect( + previewContext.internalAdapter.findUserById(existingUser.id), + ).resolves.toMatchObject({ name: "First Last" }); + }); + it("should forward result.error verbatim instead of collapsing to user_creation_failed", async () => { const production = await getTestInstance( {