From 7d18175637a0b95a501fde0cf3db080879367a9d Mon Sep 17 00:00:00 2001 From: Maxwell <145994855+ping-maxwell@users.noreply.github.com> Date: Sat, 13 Jun 2026 07:49:10 +1000 Subject: [PATCH] fix: propagate `sendVerificationEmail` errors (#8863) Co-authored-by: Bereket Engida <86073083+Bekacru@users.noreply.github.com> --- .changeset/young-cups-laugh.md | 7 ++ .cspell/custom-words.txt | 1 + .../src/api/routes/email-verification.test.ts | 111 ++++++++++++++++++ .../src/api/routes/email-verification.ts | 43 ++++--- 4 files changed, 148 insertions(+), 14 deletions(-) create mode 100644 .changeset/young-cups-laugh.md diff --git a/.changeset/young-cups-laugh.md b/.changeset/young-cups-laugh.md new file mode 100644 index 0000000000..ae3667b767 --- /dev/null +++ b/.changeset/young-cups-laugh.md @@ -0,0 +1,7 @@ +--- +"better-auth": patch +--- + +`sendVerificationEmail` was invoked via `runInBackgroundOrAwait`, which could defer work when `advanced.backgroundTasks.handler` is configured (so the handler could return **200** before the email callback finished) and, in the default path, **caught and logged errors without rethrowing**. User callbacks that throw `APIError` (e.g. **429** from a rate limiter) were therefore not reliably reflected in the HTTP response ([better-auth/better-auth#8757](https://github.com/better-auth/better-auth/issues/8757)). + +Now we await `sendVerificationEmailFn` so failures surface to the client with the correct status. The unauthenticated `/send-verification-email` path enforces a constant-time floor (500 ms) so that the response duration does not reveal whether the email belongs to a real unverified user. diff --git a/.cspell/custom-words.txt b/.cspell/custom-words.txt index 1f4bd8ba32..3c99173a0c 100644 --- a/.cspell/custom-words.txt +++ b/.cspell/custom-words.txt @@ -22,3 +22,4 @@ ciba CIBA Suliman Abdulrazzaq +rethrowing \ No newline at end of file diff --git a/packages/better-auth/src/api/routes/email-verification.test.ts b/packages/better-auth/src/api/routes/email-verification.test.ts index ab980ae52a..53075c053f 100644 --- a/packages/better-auth/src/api/routes/email-verification.test.ts +++ b/packages/better-auth/src/api/routes/email-verification.test.ts @@ -1,3 +1,4 @@ +import { APIError } from "@better-auth/core/error"; import { afterEach, describe, expect, it, vi } from "vitest"; import { getTestInstance } from "../../test-utils/test-instance"; @@ -151,6 +152,45 @@ describe("Email Verification", async () => { ); }); + /** + * @see https://github.com/better-auth/better-auth/issues/8757 + */ + it("should return APIError status when sendVerificationEmail throws (e.g. rate limit)", async () => { + let sendCalls = 0; + const { auth, testUser } = await getTestInstance({ + emailAndPassword: { + enabled: true, + requireEmailVerification: true, + }, + emailVerification: { + async sendVerificationEmail() { + sendCalls += 1; + if (sendCalls >= 2) { + throw APIError.from("TOO_MANY_REQUESTS", { + code: "RATE_LIMIT_EXCEEDED", + message: "Too many requests. Please try again later.", + }); + } + }, + }, + }); + + try { + await auth.api.sendVerificationEmail({ + body: { + email: testUser.email, + }, + }); + expect.fail("Expected sendVerificationEmail to throw"); + } catch (error) { + expect(error).toBeInstanceOf(APIError); + if (error instanceof APIError) { + expect(error.status).toBe("TOO_MANY_REQUESTS"); + expect(error.body?.code).toBe("RATE_LIMIT_EXCEEDED"); + } + } + }); + it("should send a verification email if verification is required and user is not verified", async () => { await signInWithUser(testUser.email, testUser.password); @@ -468,6 +508,77 @@ describe("Email Verification", async () => { }); }); +describe("Email Verification - Timing-safe unauthenticated path", () => { + it("should enforce a constant-time floor for both existing and non-existing emails", async () => { + const mockSendEmail = vi.fn(); + const { auth, testUser } = await getTestInstance({ + emailAndPassword: { + enabled: true, + requireEmailVerification: true, + }, + emailVerification: { + async sendVerificationEmail({ user }) { + mockSendEmail(user.email); + }, + }, + }); + + const startExisting = Date.now(); + await auth.api.sendVerificationEmail({ + body: { email: testUser.email }, + }); + const elapsedExisting = Date.now() - startExisting; + + const startMissing = Date.now(); + await auth.api.sendVerificationEmail({ + body: { email: "nonexistent@example.com" }, + }); + const elapsedMissing = Date.now() - startMissing; + + expect(elapsedExisting).toBeGreaterThanOrEqual(450); + expect(elapsedMissing).toBeGreaterThanOrEqual(450); + + expect(mockSendEmail).toHaveBeenCalledWith(testUser.email); + expect(mockSendEmail).not.toHaveBeenCalledWith("nonexistent@example.com"); + }); + + it("should still surface errors from sendVerificationEmail after the minimum delay", async () => { + let calls = 0; + const { auth, testUser } = await getTestInstance({ + emailAndPassword: { + enabled: true, + requireEmailVerification: true, + }, + emailVerification: { + async sendVerificationEmail() { + calls += 1; + if (calls >= 2) { + throw APIError.from("TOO_MANY_REQUESTS", { + code: "RATE_LIMIT_EXCEEDED", + message: "Too many requests.", + }); + } + }, + }, + }); + + const start = Date.now(); + try { + await auth.api.sendVerificationEmail({ + body: { email: testUser.email }, + }); + expect.fail("Expected to throw"); + } catch (error) { + expect(error).toBeInstanceOf(APIError); + if (error instanceof APIError) { + expect(error.status).toBe("TOO_MANY_REQUESTS"); + } + } + const elapsed = Date.now() - start; + expect(elapsed).toBeGreaterThanOrEqual(450); + }); +}); + describe("Email Verification Secondary Storage", async () => { const store = new Map(); let token: string; diff --git a/packages/better-auth/src/api/routes/email-verification.ts b/packages/better-auth/src/api/routes/email-verification.ts index c69a86b5bd..fc47ea5bb3 100644 --- a/packages/better-auth/src/api/routes/email-verification.ts +++ b/packages/better-auth/src/api/routes/email-verification.ts @@ -64,15 +64,15 @@ export async function sendVerificationEmailFn( ? encodeURIComponent(ctx.body.callbackURL) : encodeURIComponent("/"); const url = `${ctx.context.baseURL}/verify-email?token=${token}&callbackURL=${callbackURL}`; - await ctx.context.runInBackgroundOrAwait( - ctx.context.options.emailVerification.sendVerificationEmail( - { - user: user, - url, - token, - }, - ctx.request?.clone(), - ), + // Await directly: `runInBackgroundOrAwait` may defer work or swallow errors (see #8757). + // This path only runs once a real unverified user is known, so timing here does not weaken the unauthenticated anti-enumeration behavior above. + await ctx.context.options.emailVerification.sendVerificationEmail( + { + user: user, + url, + token, + }, + ctx.request, ); } export const sendVerificationEmail = createAuthEndpoint( @@ -171,7 +171,16 @@ export const sendVerificationEmail = createAuthEndpoint( const { email } = ctx.body; const session = await getSessionFromCtx(ctx); if (!session) { + /** + * Enforce a constant-time floor so an attacker cannot distinguish + * "email not found / already verified" (fast local JWT sign) from + * "email found and unverified" (slow external email-send) by + * comparing response times. + */ + const MINIMUM_MS = 500; + const start = Date.now(); const user = await ctx.context.internalAdapter.findUserByEmail(email); + let error: unknown; if (!user || user.user.emailVerified) { await createEmailVerificationToken( ctx.context.secret, @@ -179,12 +188,18 @@ export const sendVerificationEmail = createAuthEndpoint( undefined, ctx.context.options.emailVerification?.expiresIn, ); - // We're returning true to avoid leaking information about the user - return ctx.json({ - status: true, - }); + } else { + try { + await sendVerificationEmailFn(ctx, user.user); + } catch (e) { + error = e; + } } - await sendVerificationEmailFn(ctx, user.user); + const remaining = MINIMUM_MS - (Date.now() - start); + if (remaining > 0) { + await new Promise((resolve) => setTimeout(resolve, remaining)); + } + if (error) throw error; return ctx.json({ status: true, });