fix(auth): move the OTP send guard to hooks.before (admission control)

Found dogfooding the live flow: better-auth's /phone-number/send-otp stores
the fresh code BEFORE invoking sendOTP, so a cooldown rejection thrown from
the callback still rotated - and thereby invalidated - the previously
delivered code. A user hitting resend during the cooldown (or an attacker
spamming the endpoint) voided the valid OTP every time.

The per-number cooldown/hourly-cap check now runs in the hooks.before
middleware (assertPhoneOtpSendAllowed) for /phone-number/send-otp and
/phone-number/request-password-reset - rejected requests never reach the
endpoint handler, so the stored code survives. The sendOTP callback only
delivers. Behaviour verified end-to-end against a live server.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_013LXUXU66dBaP3SSG4ZVtuH

Author: claude

packages/plugins/plugin-auth/src/auth-manager.test.ts

@@ -1143,29 +1143,42 @@ describe('AuthManager', () => {
       expect(sms.sent[0].templateParams).toEqual({ code: '123456' });
     });
 
-    it('enforces the per-number cooldown (TOO_MANY_REQUESTS, no second SMS)', async () => {
+    it('enforces the per-number cooldown at ADMISSION (before-hook), not in sendOTP', async () => {
       const { manager, opts } = await bootOtp();
       const sms = fakeSms();
       manager.setSmsService(sms.service);
 
-      await opts.sendOTP({ phoneNumber: PHONE, code: '111111' });
-      await expect(opts.sendOTP({ phoneNumber: PHONE, code: '222222' }))
+      // Admission guard: first request per number passes, immediate second 429s.
+      await manager.assertPhoneOtpSendAllowed(PHONE);
+      await expect(manager.assertPhoneOtpSendAllowed(PHONE))
         .rejects.toThrow(/Too many verification codes/);
-      expect(sms.sent).toHaveLength(1);
+
+      // The sendOTP callback itself must NOT re-guard: better-auth stores the
+      // fresh code BEFORE invoking it, so a rejection at that point would
+      // still rotate (void) the previously delivered code. Delivery always
+      // proceeds once a request was admitted.
+      await opts.sendOTP({ phoneNumber: PHONE, code: '111111' });
+      await opts.sendOTP({ phoneNumber: PHONE, code: '222222' });
+      expect(sms.sent).toHaveLength(2);
     });
 
-    it('sendPasswordResetOTP shares the same guarded SMS path (cross-flow budget)', async () => {
-      const { manager, opts } = await bootOtp();
-      const sms = fakeSms();
-      manager.setSmsService(sms.service);
+    it('the admission budget spans both flows (send-otp + request-password-reset)', async () => {
+      const { manager } = await bootOtp();
+      manager.setSmsService(fakeSms().service);
 
-      await opts.sendPasswordResetOTP({ phoneNumber: PHONE, code: '333333' });
-      expect(sms.sent[0].body).toContain('333333');
-      // The cooldown spans both flows — a reset send blocks an immediate sign-in send.
-      await expect(opts.sendOTP({ phoneNumber: PHONE, code: '444444' }))
+      // One admitted send (whatever the flow) blocks an immediate second one.
+      await manager.assertPhoneOtpSendAllowed(PHONE);
+      await expect(manager.assertPhoneOtpSendAllowed(PHONE))
         .rejects.toThrow(/Too many verification codes/);
     });
 
+    it('admission is a no-op while OTP is undeliverable (sendOTP fails loudly instead)', async () => {
+      const { manager } = await bootOtp();
+      // No SMS service wired — the guard must not consume budget or throw.
+      await manager.assertPhoneOtpSendAllowed(PHONE);
+      await manager.assertPhoneOtpSendAllowed(PHONE);
+    });
+
     it('surfaces a failed SMS delivery WITHOUT the code in the error', async () => {
       const { manager, opts } = await bootOtp();
       const sms = fakeSms({ failed: true });
@@ -1175,13 +1188,11 @@ describe('AuthManager', () => {
         .rejects.toSatisfy((e: Error) => /provider down/.test(e.message) && !e.message.includes('555555'));
     });
 
-    it('honours phoneOtp knobs (cooldown off ⇒ back-to-back sends allowed)', async () => {
-      const { manager, opts } = await bootOtp({ phoneOtp: { cooldownSeconds: 0, maxPerHour: 0 } });
-      const sms = fakeSms();
-      manager.setSmsService(sms.service);
-      await opts.sendOTP({ phoneNumber: PHONE, code: '111111' });
-      await opts.sendOTP({ phoneNumber: PHONE, code: '222222' });
-      expect(sms.sent).toHaveLength(2);
+    it('honours phoneOtp knobs (cooldown off ⇒ back-to-back admissions allowed)', async () => {
+      const { manager } = await bootOtp({ phoneOtp: { cooldownSeconds: 0, maxPerHour: 0 } });
+      manager.setSmsService(fakeSms().service);
+      await manager.assertPhoneOtpSendAllowed(PHONE);
+      await manager.assertPhoneOtpSendAllowed(PHONE);
     });
 
     it('features.phoneNumberOtp requires plugin + deliverable SMS', async () => {

packages/plugins/plugin-auth/src/auth-manager.ts

@@ -818,6 +818,23 @@ export class AuthManager {
       // sees `userCount > 0` and the toggle is enforced again.
       hooks: {
         before: createAuthMiddleware(async (ctx: any) => {
+          // ── #2780: per-number OTP send guard (admission control) ─────
+          // MUST run BEFORE the phone-number endpoints: better-auth's
+          // send-otp handler stores a fresh code and only THEN invokes
+          // `sendOTP` — a guard that throws inside the callback would
+          // still rotate (invalidate) the previously delivered code, so a
+          // blocked resend (or an attacker spamming the endpoint) could
+          // keep voiding the user's valid OTP. Rejecting here leaves the
+          // stored code untouched. Applies uniformly to registered and
+          // unregistered numbers (no account-existence oracle).
+          if (
+            ctx?.path === '/phone-number/send-otp' ||
+            ctx?.path === '/phone-number/request-password-reset'
+          ) {
+            const phone = typeof ctx?.body?.phoneNumber === 'string' ? ctx.body.phoneNumber : '';
+            await this.assertPhoneOtpSendAllowed(phone);
+          }
+
           // ── ADR-0069 D1: password complexity (validator) ────────────
           // better-auth enforces only min/max length; class-mix is custom.
           // Runs on the password-mutating endpoints; reads the candidate from
@@ -2084,15 +2101,36 @@ export class AuthManager {
     return this._otpSendGuard;
   }
 
+  /**
+   * #2780 — admission check for the OTP send endpoints, called from the
+   * `hooks.before` middleware (see the `/phone-number/*` branch there for
+   * why it cannot live in the `sendOTP` callback). Consumes one unit of the
+   * per-number budget and throws TOO_MANY_REQUESTS when the cooldown /
+   * hourly cap is exhausted. No-op while OTP is undeliverable — the send
+   * callback then fails loudly with NOT_SUPPORTED instead.
+   */
+  async assertPhoneOtpSendAllowed(phone: string): Promise<void> {
+    if (!phone || !this.isPhoneOtpDeliverable()) return;
+    const decision = await this.getOtpSendGuard().checkAndRecord(phone);
+    if (!decision.ok) {
+      const { APIError } = await import('better-auth/api');
+      throw new APIError('TOO_MANY_REQUESTS', {
+        message: `Too many verification codes requested for this phone number. Retry in ${decision.retryAfterSeconds ?? 60}s.`,
+      });
+    }
+  }
+
   /**
    * #2780 — deliver a phone OTP through the SMS service.
    *
    * Security posture (all named requirements of #2780):
    *  - No SMS service ⇒ throw NOT_SUPPORTED (loud, like the pre-SMS wiring).
-   *  - Per-number cooldown + hourly cap BEFORE the provider is called
-   *    (SMS-pumping cost abuse); violations throw TOO_MANY_REQUESTS, which
-   *    /phone-number/send-otp surfaces as an honest 429 while the
-   *    request-password-reset route logs-and-200s (no enumeration oracle).
+   *  - The per-number cooldown + hourly cap live in the `hooks.before`
+   *    admission check, NOT here: better-auth stores the fresh code before
+   *    invoking this callback, so a rejection at this point would still
+   *    rotate (invalidate) the previously delivered code — letting a
+   *    blocked resend or an endpoint-spamming attacker void a user's valid
+   *    OTP. See the `/phone-number/*` branch in `hooks.before`.
    *  - The code is embedded in the message body ONLY — it must never reach
    *    a log line or an error message (the SmsService logs masked numbers
    *    and statuses, never bodies).
@@ -2108,13 +2146,6 @@ export class AuthManager {
         'Phone sign-in is password-based (POST /sign-in/phone-number).',
       );
     }
-    const decision = await this.getOtpSendGuard().checkAndRecord(phone);
-    if (!decision.ok) {
-      const { APIError } = await import('better-auth/api');
-      throw new APIError('TOO_MANY_REQUESTS', {
-        message: `Too many verification codes requested for this phone number. Retry in ${decision.retryAfterSeconds ?? 60}s.`,
-      });
-    }
     const otpCfg = this.config.phoneOtp ?? {};
     const minutes = Math.max(1, Math.round((otpCfg.expiresIn ?? 300) / 60));
     const result = await sms.send({