fix(auth): harden identity lifecycle edge cases

This commit is contained in:
Gustavo Valverde
2026-07-18 08:28:55 +01:00
parent 539a1adac8
commit 99a46ccfa9
7 changed files with 416 additions and 86 deletions
@@ -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<string, string>,
@@ -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<string, string>();
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<string, string>();
@@ -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<string, string>();
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<string, string>();
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<string, string>();
const testExpirationMap = new Map<string, number>();
@@ -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<string, string>();
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 },
@@ -61,6 +61,30 @@ type CachedSession = {
sessionVersion?: string | undefined;
};
function isRecord(value: unknown): value is Record<string, unknown> {
return typeof value === "object" && value !== null;
}
function parseCachedSession(value: unknown): CachedSession | null {
const parsed =
typeof value === "string" ? safeJSONParse<unknown>(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<Options extends BetterAuthOptions>(
validSessions.map(async ({ token }) => {
const cached = await secondaryStorage.get(token);
if (!cached) return;
const parsed = safeJSONParse<CachedSession>(cached);
const parsed = parseCachedSession(cached);
if (!parsed) return;
if (!(await isCurrentCachedSession(parsed))) {
return;
@@ -287,10 +311,8 @@ export function createSessionAdapterModule<Options extends BetterAuthOptions>(
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<Options extends BetterAuthOptions>(
return null;
}
if (sessionStringified) {
const s = safeJSONParse<CachedSession>(sessionStringified);
const s = parseCachedSession(sessionStringified);
if (!s) return null;
if (!(await isCurrentCachedSession(s))) {
return null;
@@ -539,12 +561,8 @@ export function createSessionAdapterModule<Options extends BetterAuthOptions>(
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<Options extends BetterAuthOptions>(
return null;
}
const parsedSession =
safeJSONParse<CachedSession>(currentSession);
const parsedSession = parseCachedSession(currentSession);
if (!parsedSession) return null;
if (!(await isCurrentCachedSession(parsedSession))) {
return null;
@@ -666,33 +683,34 @@ export function createSessionAdapterModule<Options extends BetterAuthOptions>(
}),
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;
@@ -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) => {
@@ -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<Verification>({
const existing = await currentAdapter.findOne<Verification>({
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,
);
});
}
}
@@ -47,24 +47,30 @@ export async function revokeUnprovenAccountAccess(
userId: string,
): Promise<User | null> {
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 {
@@ -284,6 +284,12 @@ export const oAuthProxy = <O extends OAuthProxyOptions>(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<ReturnType<typeof authenticateProviderUser>>;
try {
result = await authenticateProviderUser(ctx, {
@@ -292,6 +298,7 @@ export const oAuthProxy = <O extends OAuthProxyOptions>(opts?: O) => {
account: payload.account,
callbackURL: payload.callbackURL,
disableSignUp: payload.disableSignUp,
overrideUserInfo: provider?.options?.overrideUserInfoOnSignIn,
source: {
method: "oauth",
oauth: {
@@ -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(
{