fix(plugin-auth): 短信日配额拒发时 OTP / 邀请短信回 429,不再是 500 + 空响应体 (#6039)

.changeset/otp-sms-quota-429.md

@@ -0,0 +1,40 @@
+---
+"@objectstack/plugin-auth": patch
+---
+
+fix(plugin-auth): 短信日配额拒发时,OTP / 邀请短信按 429 TOO_MANY_REQUESTS 作答,不再是 500 (#6039)
+
+#2814 把短信总量成本闸落在 `SmsService.send()` —— 它是内核服务,不知道调用方是谁,
+所以超限时**返回**一条失败结果,把码写在服务层既有的 `CODE: message` 信封上:
+`TOO_MANY_REQUESTS: daily SMS quota exhausted`。把 HTTP 语义还原回去是 auth 端点的
+职责,而 `AuthManager` 此前没有做:`deliverPhoneOtp()` / `sendPhoneInviteSms()` 对
+任何 `status === 'failed'` 一律抛普通 `Error`。
+
+better-auth 的路由层 better-call 只把 `APIError` 映射成真实状态码
+(`isAPIError = err instanceof APIError || err?.name === 'APIError'`,
+better-call@1.3.7 `dist/utils.mjs:57`,消费点在 `dist/router.mjs:93`),其余一律走
+`console.error` + **500、响应体 `null`** 的分支。于是配额拒发对外是 500,
+`TOO_MANY_REQUESTS` 只留在服务端日志里;而**同一个端点**上按号码冷却闸
+(`assertPhoneOtpSendAllowed`,在 admission hook 里)抛的是
+`APIError('TOO_MANY_REQUESTS')`,正常回 429 —— 一个端点两种口径,正是 #2814
+「两道墙从外面看应当一样」的反面。
+
+现在两处失败分支都先识别信封上的 `TOO_MANY_REQUESTS:` **前缀**,改抛
+`APIError('TOO_MANY_REQUESTS')`:
+
+- **只有码跨包**。识别用的 `TOO_MANY_REQUESTS` 在 plugin-auth 本地写死并注明出处
+  (`SMS_QUOTA_EXCEEDED_CODE`,`packages/services/service-sms/src/sms-daily-quota.ts`)——
+  `@objectstack/service-sms` 已经依赖本包(它的日计数器从这里 import
+  `InProcessCounterStore`),反向 import 会成环;这与 service-sms 里
+  `normalizeSmsRecipient` 就地重述 plugin-auth 形状规则是同一个取舍的另一半。
+  跨包重述的只是一个 ADR-0112 闭集错误码,冒号后的措辞归服务层所有,可以自由改写。
+- **不泄露预算**。429 文案沿按号码闸的措辞形状,不含上限、剩余量与重置时刻
+  (按号码闸报自己的重试窗口,是因为它算得出;配额闸不承诺它给不出的时间)。
+- **不顺手收紧**。传输故障(provider 宕机等)仍抛普通 `Error`,500 语义原样不变;
+  仅仅在文中提到该码而不以之开头的 provider 报错同样保持 500。
+
+对外可见的变化:`POST /phone-number/send-otp`、
+`POST /phone-number/request-password-reset` 在部署日配额耗尽时,由
+**500 + 空响应体**变为 **429 TOO_MANY_REQUESTS**,与按号码冷却闸同形。
+邀请短信路径同样返回 `APIError`;仓内唯一调用方(admin import-users)按行捕获它并
+记为 `INVITE_SMS_FAILED`,该路径的变化是行内报错不再携带服务层原始信封。

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

@@ -1681,15 +1681,15 @@ describe('AuthManager', () => {
   describe('phone-number OTP over SMS (#2780)', () => {
     const PHONE = '+8613800000000';
 
-    const fakeSms = (opts: { failed?: boolean } = {}) => {
+    const fakeSms = (opts: { failed?: boolean; error?: string } = {}) => {
       const sent: any[] = [];
       return {
         sent,
         service: {
           async send(input: any) {
             sent.push(input);
             return opts.failed
-              ? { id: 'sms_1', status: 'failed', error: 'provider down' }
+              ? { id: 'sms_1', status: 'failed', error: opts.error ?? 'provider down' }
               : { id: 'sms_1', status: 'sent', messageId: 'prov_1' };
           },
           isConfigured: () => true,
@@ -1785,6 +1785,158 @@ describe('AuthManager', () => {
         .rejects.toSatisfy((e: Error) => /provider down/.test(e.message) && !e.message.includes('555555'));
     });
 
+    // ── #6039 / #2814 — the deployment-wide daily SMS quota wall ───────────
+    //
+    // `SmsService.send()` refuses a send past the deployment's daily cost
+    // ceiling by RETURNING a failed result whose `error` carries the
+    // `TOO_MANY_REQUESTS:` code prefix (#2814) — it cannot throw an HTTP-shaped
+    // error, because it is a kernel service with no idea who is calling.
+    // Rethrowing that envelope as a plain `Error` made better-call answer
+    // **500 with a null body**: its router maps only `APIError`
+    // (`isAPIError = err instanceof APIError || err?.name === 'APIError'`,
+    // better-call@1.3.7 `dist/utils.mjs:57`, consumed at `dist/router.mjs:93`),
+    // and everything else takes the `console.error` + 500 branch. Meanwhile the
+    // per-number wall on the SAME endpoint (`assertPhoneOtpSendAllowed`, in the
+    // admission hook) throws a real `APIError('TOO_MANY_REQUESTS')` and answers
+    // 429 — so one endpoint spoke with two voices, which is the reverse of what
+    // #2814 asked for.
+    describe('daily SMS quota refusal reaches the caller as 429 (#6039)', () => {
+      /**
+       * The refusal envelope an `SmsService` hands back on a quota refusal.
+       * Written out here rather than imported: `@objectstack/service-sms`
+       * already depends on THIS package (its day counter imports
+       * `InProcessCounterStore` / `incrementFixedWindow` from plugin-auth), so
+       * importing its `SMS_QUOTA_EXCEEDED_ERROR` back would close a dependency
+       * cycle. Source of truth: `SMS_QUOTA_EXCEEDED_CODE` /
+       * `SMS_QUOTA_EXCEEDED_ERROR` in
+       * `packages/services/service-sms/src/sms-daily-quota.ts`.
+       */
+      const QUOTA_REFUSAL = 'TOO_MANY_REQUESTS: daily SMS quota exhausted';
+
+      /**
+       * The outward shape a client — and better-call's router — actually
+       * branches on. Message text is deliberately NOT part of it: the two walls
+       * must be indistinguishable in code and status, while each may still say
+       * something true (only the per-number wall can name a retry window).
+       */
+      const outwardShape = (e: any) => ({
+        name: e?.name,
+        status: e?.status,
+        statusCode: e?.statusCode,
+        bodyCode: e?.body?.code,
+      });
+
+      const rejection = async (run: () => Promise<unknown>): Promise<any> => {
+        try {
+          await run();
+        } catch (e) {
+          return e;
+        }
+        throw new Error('expected the call to reject, but it resolved');
+      };
+
+      it('OTP send: rejects with an APIError carrying 429 / TOO_MANY_REQUESTS', async () => {
+        const { manager, opts } = await bootOtp();
+        manager.setSmsService(fakeSms({ failed: true, error: QUOTA_REFUSAL }).service);
+
+        const err = await rejection(() => opts.sendOTP({ phoneNumber: PHONE, code: '424242' }));
+        // Exactly what better-call reads to choose 429 over 500.
+        const { isAPIError } = await import('better-auth/api');
+        expect(isAPIError(err)).toBe(true);
+        expect(err.name).toBe('APIError');
+        expect(err.status).toBe('TOO_MANY_REQUESTS');
+        expect(err.statusCode).toBe(429);
+        // #2780 standing requirement: the code never travels in an error.
+        expect(String(err.message)).not.toContain('424242');
+      });
+
+      it('invitation SMS: same APIError / 429 at the AuthManager boundary', async () => {
+        const { manager } = await bootOtp();
+        manager.setSmsService(fakeSms({ failed: true, error: QUOTA_REFUSAL }).service);
+
+        const err = await rejection(() => manager.sendPhoneInviteSms(PHONE));
+        const { isAPIError } = await import('better-auth/api');
+        expect(isAPIError(err)).toBe(true);
+        expect(err.status).toBe('TOO_MANY_REQUESTS');
+        expect(err.statusCode).toBe(429);
+      });
+
+      it('both walls on the endpoint present the SAME outward shape', async () => {
+        const { manager, opts } = await bootOtp();
+        manager.setSmsService(fakeSms({ failed: true, error: QUOTA_REFUSAL }).service);
+
+        // Wall A — the per-number cooldown, refused in the admission hook (#2780).
+        await manager.assertPhoneOtpSendAllowed(PHONE);
+        const perNumber = await rejection(() => manager.assertPhoneOtpSendAllowed(PHONE));
+        // Wall B — the deployment's daily quota, refused inside the send (#2814).
+        const quota = await rejection(() => opts.sendOTP({ phoneNumber: PHONE, code: '111111' }));
+
+        expect(outwardShape(quota)).toEqual(outwardShape(perNumber));
+        expect(outwardShape(quota)).toEqual({
+          name: 'APIError',
+          status: 'TOO_MANY_REQUESTS',
+          statusCode: 429,
+          bodyCode: undefined,
+        });
+      });
+
+      it('matches the CODE prefix, so the service may reword the message half', async () => {
+        const { manager, opts } = await bootOtp();
+        // Only `TOO_MANY_REQUESTS` — an ADR-0112 error code — is restated across
+        // the package boundary; the prose after the colon is service-owned and
+        // free to change without breaking this mapping.
+        manager.setSmsService(
+          fakeSms({ failed: true, error: 'TOO_MANY_REQUESTS: budget spent for today' }).service,
+        );
+        const err = await rejection(() => opts.sendOTP({ phoneNumber: PHONE, code: '333333' }));
+        expect(err.statusCode).toBe(429);
+      });
+
+      it('does NOT over-tighten: a transport failure keeps its 500 semantics', async () => {
+        const { manager, opts } = await bootOtp();
+        manager.setSmsService(fakeSms({ failed: true }).service); // 'provider down'
+        const { isAPIError } = await import('better-auth/api');
+
+        const otpErr = await rejection(() => opts.sendOTP({ phoneNumber: PHONE, code: '555555' }));
+        expect(isAPIError(otpErr)).toBe(false);
+        expect(otpErr.name).toBe('Error');
+        expect(String(otpErr.message)).toContain('provider down');
+
+        const inviteErr = await rejection(() => manager.sendPhoneInviteSms(PHONE));
+        expect(isAPIError(inviteErr)).toBe(false);
+        expect(inviteErr.name).toBe('Error');
+        expect(String(inviteErr.message)).toContain('provider down');
+      });
+
+      it('the code must PREFIX the envelope — a provider merely mentioning it stays 500', async () => {
+        const { manager, opts } = await bootOtp();
+        manager.setSmsService(
+          fakeSms({ failed: true, error: 'upstream rejected: TOO_MANY_REQUESTS at carrier' }).service,
+        );
+        const { isAPIError } = await import('better-auth/api');
+        const err = await rejection(() => opts.sendOTP({ phoneNumber: PHONE, code: '777777' }));
+        expect(isAPIError(err)).toBe(false);
+        expect(err.name).toBe('Error');
+      });
+
+      it('the 429 message carries no quota ceiling, remaining count or reset clock', async () => {
+        const { manager, opts } = await bootOtp();
+        manager.setSmsService(fakeSms({ failed: true, error: QUOTA_REFUSAL }).service);
+
+        for (const run of [
+          () => opts.sendOTP({ phoneNumber: PHONE, code: '999999' }),
+          () => manager.sendPhoneInviteSms(PHONE),
+        ]) {
+          const message = String((await rejection(run)).message);
+          // No digits at all ⇒ no ceiling, no remaining count, no reset clock.
+          expect(message).not.toMatch(/\d/);
+          // …and not the raw service envelope, which names the budget that was hit.
+          expect(message).not.toContain(QUOTA_REFUSAL);
+          expect(message.toLowerCase()).not.toContain('quota');
+        }
+      });
+    });
+
     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);

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

@@ -728,6 +728,70 @@ export function ipMatchesRange(ip: string, range: string): boolean {
   return ip.trim() === r;
 }
 
+/**
+ * #6039 / #2814 — the error code an SMS service reports a QUOTA refusal with.
+ *
+ * `SmsService.send()` cannot throw an HTTP-shaped error: it is a kernel service
+ * with no idea who is calling. It reports the deployment's daily cost ceiling
+ * being hit as a normal failed result whose `error` carries the service's
+ * `CODE: message` envelope — here `TOO_MANY_REQUESTS: daily SMS quota
+ * exhausted`. Turning that back into an HTTP answer is the auth endpoint's job,
+ * which is why the mapping lives here (see {@link isSmsQuotaRefusal}).
+ *
+ * **Source of truth**: `SMS_QUOTA_EXCEEDED_CODE` in
+ * `packages/services/service-sms/src/sms-daily-quota.ts` (exported alongside
+ * `SMS_QUOTA_EXCEEDED_ERROR`).
+ *
+ * **Why it is restated instead of imported**: `@objectstack/service-sms` already
+ * depends on THIS package — its day counter imports `InProcessCounterStore` /
+ * `incrementFixedWindow` from `@objectstack/plugin-auth` — so importing the
+ * constant back would close a dependency cycle. This is the same call, in the
+ * other direction, that `normalizeSmsRecipient` makes in
+ * `packages/services/service-sms/src/sms-service.ts` ("Same shape rule as
+ * plugin-auth's `normalizePhoneNumber` … kept local: the two packages must not
+ * depend on each other").
+ *
+ * Only the CODE crosses the boundary — a closed-vocabulary ADR-0112 error code,
+ * not prose. The message half of the envelope is service-owned and may be
+ * reworded without touching this file.
+ */
+const SMS_QUOTA_EXCEEDED_CODE = 'TOO_MANY_REQUESTS';
+
+/**
+ * #6039 — is this `SendSmsResult.error` the quota wall's refusal?
+ *
+ * Matched as a PREFIX of the service's `CODE: message` envelope, never as a
+ * substring: a transport failure puts the provider's raw text in `error`
+ * (`SmsService` truncates it and reports it verbatim), and one that happens to
+ * mention the code mid-sentence is still a transport failure — 500, not 429.
+ */
+function isSmsQuotaRefusal(error: string | undefined): boolean {
+  return typeof error === 'string' && error.startsWith(`${SMS_QUOTA_EXCEEDED_CODE}:`);
+}
+
+/**
+ * #6039 — the 429 an SMS quota refusal must reach the caller as.
+ *
+ * better-call (better-auth's router) maps ONLY `APIError` to a real HTTP status:
+ * `isAPIError(err) = err instanceof APIError || err?.name === 'APIError'`
+ * (better-call@1.3.7 `dist/utils.mjs:57`), consumed at `dist/router.mjs:93`,
+ * where everything else takes the `console.error` + `500 / null body` branch.
+ * A plain `Error` therefore buried `TOO_MANY_REQUESTS` in a server log while the
+ * per-number wall on the same endpoint ({@link AuthManager.assertPhoneOtpSendAllowed})
+ * answered 429 — one endpoint, two voices.
+ *
+ * The message follows the per-number wall's shape and carries NO budget detail:
+ * no ceiling, no remaining count, no reset clock. #2814's requirement is that
+ * the two walls be indistinguishable from outside — an attacker must not learn
+ * which budget they hit, and a legitimate caller needs no more than "not now".
+ * (The per-number wall names its own retry window because it can compute one;
+ * this wall states no time it cannot honestly promise.)
+ */
+async function smsQuotaExceededApiError(message: string): Promise<Error> {
+  const { APIError } = await import('better-auth/api');
+  return new APIError(SMS_QUOTA_EXCEEDED_CODE, { message });
+}
+
 export class AuthManager {
   private auth: Auth<any> | null = null;
   private config: AuthManagerOptions;
@@ -2840,6 +2904,16 @@ export class AuthManager {
       templateParams: { code },
     });
     if (result.status === 'failed') {
+      // #6039 — the deployment's daily SMS quota refused this send. Answer it
+      // the way the per-number wall on this same endpoint answers: a real
+      // `APIError`, hence 429 TOO_MANY_REQUESTS instead of a 500 with a null
+      // body. Everything else stays a plain Error — a transport outage IS a
+      // server-side failure and 500 is the honest answer for it.
+      if (isSmsQuotaRefusal(result.error)) {
+        throw await smsQuotaExceededApiError(
+          'Too many verification codes requested. Please try again later.',
+        );
+      }
       // `result.error` is transport detail (never the code) — safe to surface.
       throw new Error(`Phone OTP could not be sent: ${result.error ?? 'SMS delivery failed'}`);
     }
@@ -2870,6 +2944,18 @@ export class AuthManager {
     });
     const result = await sms.send({ to: phone, body, templateParams: { content: body } });
     if (result.status === 'failed') {
+      // #6039 — same quota wall, same outward shape as the OTP path above.
+      // (The one in-repo caller, admin import-users, catches this per row and
+      // records INVITE_SMS_FAILED rather than failing the request — so what
+      // changes there is that the row's message no longer carries the raw
+      // service envelope. The 429 matters for any caller that surfaces this
+      // rejection directly, which is what a public AuthManager method must be
+      // correct for.)
+      if (isSmsQuotaRefusal(result.error)) {
+        throw await smsQuotaExceededApiError(
+          'Too many SMS messages requested. Please try again later.',
+        );
+      }
       throw new Error(`Invitation SMS failed: ${result.error ?? 'SMS delivery failed'}`);
     }
   }