Skip to content

Commit 1c19139

Browse files
os-zhuangclaude
andauthored
refactor(sms): plugin-sms 更名为 service-sms + OTP 准入限流修复 + OS_AUTH_PHONE_NUMBER_ENABLED (#2797)
* refactor(sms): rename @objectstack/plugin-sms to @objectstack/service-sms Infrastructure services follow the service-* convention (service-messaging, service-settings, ...) - the plugin-* prefix was a misfit for a package whose whole job is registering the `sms` kernel service (plugin-email is legacy debt, not precedent). Same exports, same SmsServicePlugin class, same `sms` service id and settings namespace - only the package name and its home (packages/services/service-sms) change. The one published @objectstack/plugin-sms@14.3.0 release should be npm-deprecated in favour of @objectstack/service-sms. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_013LXUXU66dBaP3SSG4ZVtuH * feat(cli): OS_AUTH_PHONE_NUMBER_ENABLED switch for the phoneNumber plugin plugins.phoneNumber existed in AuthPluginConfig (#2766) but `objectstack serve` never set it - there was no way to turn phone sign-in/OTP on from the CLI. Follows the OS_{DOMAIN}_{FEATURE}_ENABLED convention; default off. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_013LXUXU66dBaP3SSG4ZVtuH * 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 --------- Co-authored-by: Claude <noreply@anthropic.com>
1 parent 7449476 commit 1c19139

26 files changed

Lines changed: 140 additions & 77 deletions

File tree

.changeset/config.json

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -44,7 +44,7 @@
4444
"@objectstack/plugin-reports",
4545
"@objectstack/plugin-security",
4646
"@objectstack/plugin-sharing",
47-
"@objectstack/plugin-sms",
47+
"@objectstack/service-sms",
4848
"@objectstack/plugin-webhooks",
4949
"@objectstack/trigger-record-change",
5050
"@objectstack/trigger-api",
Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,15 @@
1+
---
2+
"@objectstack/service-sms": patch
3+
"@objectstack/cli": patch
4+
---
5+
6+
refactor(sms): rename `@objectstack/plugin-sms` to `@objectstack/service-sms`
7+
8+
Infrastructure services follow the `service-*` convention
9+
(`service-messaging`, `service-settings`, …) — the `plugin-*` prefix was a
10+
misfit for a package whose whole job is registering the `sms` kernel
11+
service (`plugin-email` is legacy debt, not precedent). Same exports, same
12+
`SmsServicePlugin` class, same `sms` service id and settings namespace —
13+
only the package name and its home (`packages/services/service-sms`)
14+
change. The one published `@objectstack/plugin-sms@14.3.0` release should
15+
be npm-deprecated in favour of `@objectstack/service-sms`.

content/docs/plugins/packages.mdx

Lines changed: 9 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -15,8 +15,8 @@ ObjectStack is organized into **71 package manifests** across multiple categorie
1515
| **Client / DX** | 5 | `client`, `client-react`, `cli`, `create-objectstack`, `vscode-objectstack` |
1616
| **Framework adapters** | 1 | `hono` (other frameworks: build a thin adapter on `HttpDispatcher` — see below) |
1717
| **Drivers** | 4 | `driver-memory`, `driver-sql`, `driver-sqlite-wasm`, `driver-mongodb` |
18-
| **Plugins** | 19 | `plugin-auth`, `plugin-security`, `plugin-org-scoping`, `plugin-audit`, `plugin-approvals`, `plugin-sharing`, `plugin-email`, `plugin-sms`, `plugin-webhooks`, `plugin-reports`, `plugin-hono-server`, `plugin-dev`, `mcp`, trigger plugins (`trigger-api`, `trigger-record-change`, `trigger-schedule`), and knowledge/embedder plugins (`knowledge-memory`, `knowledge-ragflow`, `embedder-openai`) |
19-
| **Platform services** | 15 | `service-analytics`, `service-automation`, `service-cache`, `service-cluster`, `service-cluster-redis`, `service-datasource`, `service-i18n`, `service-job`, `service-knowledge`, `service-messaging`, `service-package`, `service-queue`, `service-realtime`, `service-settings`, `service-storage` |
18+
| **Plugins** | 18 | `plugin-auth`, `plugin-security`, `plugin-org-scoping`, `plugin-audit`, `plugin-approvals`, `plugin-sharing`, `plugin-email`, `plugin-webhooks`, `plugin-reports`, `plugin-hono-server`, `plugin-dev`, `mcp`, trigger plugins (`trigger-api`, `trigger-record-change`, `trigger-schedule`), and knowledge/embedder plugins (`knowledge-memory`, `knowledge-ragflow`, `embedder-openai`) |
19+
| **Platform services** | 16 | `service-analytics`, `service-automation`, `service-cache`, `service-cluster`, `service-cluster-redis`, `service-datasource`, `service-i18n`, `service-job`, `service-knowledge`, `service-messaging`, `service-package`, `service-queue`, `service-realtime`, `service-settings`, `service-sms`, `service-storage` |
2020

2121

2222
## Core Packages
@@ -226,6 +226,13 @@ All services implement contracts from `@objectstack/spec/contracts` and are kern
226226
- **Features**: `MessagingChannel` registry, `emit()` fan-out, always-on inbox channel; email/webhook/push/IM channels plug in
227227
- **When to use**: Notifying users across channels from flows, hooks, and plugins
228228

229+
### @objectstack/service-sms
230+
231+
**SMS Service** — Outbound SMS delivery (`sms` service).
232+
233+
- **Features**: Provider adapters (Aliyun SMS, Twilio), `sms` settings namespace with live rebind, backs phone-number OTP sign-in/reset and the messaging `sms` channel
234+
- **When to use**: Phone OTP first-login / self-service reset, SMS notifications
235+
229236
### @objectstack/service-i18n
230237

231238
**I18n Service** — Internationalization with file-based locales.
@@ -363,13 +370,6 @@ All services implement contracts from `@objectstack/spec/contracts` and are kern
363370
- **Features**: Provider adapters, MJML templates, delivery tracking
364371
- **When to use**: Transactional and workflow-driven email
365372

366-
### @objectstack/plugin-sms
367-
368-
**SMS Plugin** — Outbound SMS delivery (`sms` service).
369-
370-
- **Features**: Provider adapters (Aliyun SMS, Twilio), `sms` settings namespace with live rebind, backs phone-number OTP sign-in/reset and the messaging `sms` channel
371-
- **When to use**: Phone OTP first-login / self-service reset, SMS notifications
372-
373373
### @objectstack/plugin-webhooks
374374

375375
**Webhooks Plugin** — Outbound HTTP webhook delivery.

packages/cli/package.json

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -62,7 +62,7 @@
6262
"@objectstack/plugin-reports": "workspace:*",
6363
"@objectstack/plugin-security": "workspace:*",
6464
"@objectstack/plugin-sharing": "workspace:*",
65-
"@objectstack/plugin-sms": "workspace:*",
65+
"@objectstack/service-sms": "workspace:*",
6666
"@objectstack/plugin-webhooks": "workspace:*",
6767
"@objectstack/rest": "workspace:*",
6868
"@objectstack/runtime": "workspace:^",

packages/cli/src/commands/serve.ts

Lines changed: 8 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1259,6 +1259,12 @@ export default class Serve extends Command {
12591259
// operator override (env wins in buildPluginList()).
12601260
passwordRejectBreached:
12611261
String(process.env.OS_AUTH_PASSWORD_REJECT_BREACHED ?? 'false').toLowerCase() === 'true',
1262+
// #2766/#2780 — phone-number sign-in (phone+password always;
1263+
// OTP sign-in/reset once the sms capability has a deliverable
1264+
// provider). Opt-in: without this env the config flag had no
1265+
// `objectstack serve` switch at all.
1266+
phoneNumber:
1267+
String(process.env.OS_AUTH_PHONE_NUMBER_ENABLED ?? 'false').toLowerCase() === 'true',
12621268
},
12631269
advanced: process.env.OS_COOKIE_DOMAIN
12641270
? ({
@@ -1666,9 +1672,9 @@ export default class Serve extends Command {
16661672
// the messaging `sms` channel. Provider config lives in the `sms`
16671673
// settings namespace (OS_SMS_* env keys win at the resolver);
16681674
// unconfigured ⇒ dev LogSmsTransport (no real send).
1669-
pkg: '@objectstack/plugin-sms',
1675+
pkg: '@objectstack/service-sms',
16701676
export: 'SmsServicePlugin',
1671-
nameMatch: ['plugin-sms', 'SmsServicePlugin'],
1677+
nameMatch: ['service-sms', 'SmsServicePlugin'],
16721678
},
16731679
sharing: {
16741680
pkg: '@objectstack/plugin-sharing',

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

Lines changed: 30 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -1143,29 +1143,42 @@ describe('AuthManager', () => {
11431143
expect(sms.sent[0].templateParams).toEqual({ code: '123456' });
11441144
});
11451145

1146-
it('enforces the per-number cooldown (TOO_MANY_REQUESTS, no second SMS)', async () => {
1146+
it('enforces the per-number cooldown at ADMISSION (before-hook), not in sendOTP', async () => {
11471147
const { manager, opts } = await bootOtp();
11481148
const sms = fakeSms();
11491149
manager.setSmsService(sms.service);
11501150

1151-
await opts.sendOTP({ phoneNumber: PHONE, code: '111111' });
1152-
await expect(opts.sendOTP({ phoneNumber: PHONE, code: '222222' }))
1151+
// Admission guard: first request per number passes, immediate second 429s.
1152+
await manager.assertPhoneOtpSendAllowed(PHONE);
1153+
await expect(manager.assertPhoneOtpSendAllowed(PHONE))
11531154
.rejects.toThrow(/Too many verification codes/);
1154-
expect(sms.sent).toHaveLength(1);
1155+
1156+
// The sendOTP callback itself must NOT re-guard: better-auth stores the
1157+
// fresh code BEFORE invoking it, so a rejection at that point would
1158+
// still rotate (void) the previously delivered code. Delivery always
1159+
// proceeds once a request was admitted.
1160+
await opts.sendOTP({ phoneNumber: PHONE, code: '111111' });
1161+
await opts.sendOTP({ phoneNumber: PHONE, code: '222222' });
1162+
expect(sms.sent).toHaveLength(2);
11551163
});
11561164

1157-
it('sendPasswordResetOTP shares the same guarded SMS path (cross-flow budget)', async () => {
1158-
const { manager, opts } = await bootOtp();
1159-
const sms = fakeSms();
1160-
manager.setSmsService(sms.service);
1165+
it('the admission budget spans both flows (send-otp + request-password-reset)', async () => {
1166+
const { manager } = await bootOtp();
1167+
manager.setSmsService(fakeSms().service);
11611168

1162-
await opts.sendPasswordResetOTP({ phoneNumber: PHONE, code: '333333' });
1163-
expect(sms.sent[0].body).toContain('333333');
1164-
// The cooldown spans both flows — a reset send blocks an immediate sign-in send.
1165-
await expect(opts.sendOTP({ phoneNumber: PHONE, code: '444444' }))
1169+
// One admitted send (whatever the flow) blocks an immediate second one.
1170+
await manager.assertPhoneOtpSendAllowed(PHONE);
1171+
await expect(manager.assertPhoneOtpSendAllowed(PHONE))
11661172
.rejects.toThrow(/Too many verification codes/);
11671173
});
11681174

1175+
it('admission is a no-op while OTP is undeliverable (sendOTP fails loudly instead)', async () => {
1176+
const { manager } = await bootOtp();
1177+
// No SMS service wired — the guard must not consume budget or throw.
1178+
await manager.assertPhoneOtpSendAllowed(PHONE);
1179+
await manager.assertPhoneOtpSendAllowed(PHONE);
1180+
});
1181+
11691182
it('surfaces a failed SMS delivery WITHOUT the code in the error', async () => {
11701183
const { manager, opts } = await bootOtp();
11711184
const sms = fakeSms({ failed: true });
@@ -1175,13 +1188,11 @@ describe('AuthManager', () => {
11751188
.rejects.toSatisfy((e: Error) => /provider down/.test(e.message) && !e.message.includes('555555'));
11761189
});
11771190

1178-
it('honours phoneOtp knobs (cooldown off ⇒ back-to-back sends allowed)', async () => {
1179-
const { manager, opts } = await bootOtp({ phoneOtp: { cooldownSeconds: 0, maxPerHour: 0 } });
1180-
const sms = fakeSms();
1181-
manager.setSmsService(sms.service);
1182-
await opts.sendOTP({ phoneNumber: PHONE, code: '111111' });
1183-
await opts.sendOTP({ phoneNumber: PHONE, code: '222222' });
1184-
expect(sms.sent).toHaveLength(2);
1191+
it('honours phoneOtp knobs (cooldown off ⇒ back-to-back admissions allowed)', async () => {
1192+
const { manager } = await bootOtp({ phoneOtp: { cooldownSeconds: 0, maxPerHour: 0 } });
1193+
manager.setSmsService(fakeSms().service);
1194+
await manager.assertPhoneOtpSendAllowed(PHONE);
1195+
await manager.assertPhoneOtpSendAllowed(PHONE);
11851196
});
11861197

11871198
it('features.phoneNumberOtp requires plugin + deliverable SMS', async () => {

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

Lines changed: 44 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -327,7 +327,7 @@ export interface AuthManagerOptions extends Partial<AuthConfig> {
327327
*
328328
* Resolved lazily through {@link AuthManager.getSmsService}; safe to set
329329
* after construction. AuthPlugin wires this from the kernel service
330-
* registry (`sms`, see `@objectstack/plugin-sms`) on `kernel:ready`.
330+
* registry (`sms`, see `@objectstack/service-sms`) on `kernel:ready`.
331331
*/
332332
smsService?: ISmsService;
333333

@@ -818,6 +818,23 @@ export class AuthManager {
818818
// sees `userCount > 0` and the toggle is enforced again.
819819
hooks: {
820820
before: createAuthMiddleware(async (ctx: any) => {
821+
// ── #2780: per-number OTP send guard (admission control) ─────
822+
// MUST run BEFORE the phone-number endpoints: better-auth's
823+
// send-otp handler stores a fresh code and only THEN invokes
824+
// `sendOTP` — a guard that throws inside the callback would
825+
// still rotate (invalidate) the previously delivered code, so a
826+
// blocked resend (or an attacker spamming the endpoint) could
827+
// keep voiding the user's valid OTP. Rejecting here leaves the
828+
// stored code untouched. Applies uniformly to registered and
829+
// unregistered numbers (no account-existence oracle).
830+
if (
831+
ctx?.path === '/phone-number/send-otp' ||
832+
ctx?.path === '/phone-number/request-password-reset'
833+
) {
834+
const phone = typeof ctx?.body?.phoneNumber === 'string' ? ctx.body.phoneNumber : '';
835+
await this.assertPhoneOtpSendAllowed(phone);
836+
}
837+
821838
// ── ADR-0069 D1: password complexity (validator) ────────────
822839
// better-auth enforces only min/max length; class-mix is custom.
823840
// Runs on the password-mutating endpoints; reads the candidate from
@@ -2057,7 +2074,7 @@ export class AuthManager {
20572074
* OTP callbacks and the SMS invite path (#2780). Mirrors
20582075
* {@link setEmailService}: resolved lazily per send, so it is safe to set
20592076
* after construction — AuthPlugin wires it on `kernel:ready` once
2060-
* `ctx.getService('sms')` (plugin-sms) resolves.
2077+
* `ctx.getService('sms')` (service-sms) resolves.
20612078
*/
20622079
setSmsService(sms: ISmsService | undefined): void {
20632080
this.config.smsService = sms;
@@ -2084,15 +2101,36 @@ export class AuthManager {
20842101
return this._otpSendGuard;
20852102
}
20862103

2104+
/**
2105+
* #2780 — admission check for the OTP send endpoints, called from the
2106+
* `hooks.before` middleware (see the `/phone-number/*` branch there for
2107+
* why it cannot live in the `sendOTP` callback). Consumes one unit of the
2108+
* per-number budget and throws TOO_MANY_REQUESTS when the cooldown /
2109+
* hourly cap is exhausted. No-op while OTP is undeliverable — the send
2110+
* callback then fails loudly with NOT_SUPPORTED instead.
2111+
*/
2112+
async assertPhoneOtpSendAllowed(phone: string): Promise<void> {
2113+
if (!phone || !this.isPhoneOtpDeliverable()) return;
2114+
const decision = await this.getOtpSendGuard().checkAndRecord(phone);
2115+
if (!decision.ok) {
2116+
const { APIError } = await import('better-auth/api');
2117+
throw new APIError('TOO_MANY_REQUESTS', {
2118+
message: `Too many verification codes requested for this phone number. Retry in ${decision.retryAfterSeconds ?? 60}s.`,
2119+
});
2120+
}
2121+
}
2122+
20872123
/**
20882124
* #2780 — deliver a phone OTP through the SMS service.
20892125
*
20902126
* Security posture (all named requirements of #2780):
20912127
* - No SMS service ⇒ throw NOT_SUPPORTED (loud, like the pre-SMS wiring).
2092-
* - Per-number cooldown + hourly cap BEFORE the provider is called
2093-
* (SMS-pumping cost abuse); violations throw TOO_MANY_REQUESTS, which
2094-
* /phone-number/send-otp surfaces as an honest 429 while the
2095-
* request-password-reset route logs-and-200s (no enumeration oracle).
2128+
* - The per-number cooldown + hourly cap live in the `hooks.before`
2129+
* admission check, NOT here: better-auth stores the fresh code before
2130+
* invoking this callback, so a rejection at this point would still
2131+
* rotate (invalidate) the previously delivered code — letting a
2132+
* blocked resend or an endpoint-spamming attacker void a user's valid
2133+
* OTP. See the `/phone-number/*` branch in `hooks.before`.
20962134
* - The code is embedded in the message body ONLY — it must never reach
20972135
* a log line or an error message (the SmsService logs masked numbers
20982136
* and statuses, never bodies).
@@ -2108,13 +2146,6 @@ export class AuthManager {
21082146
'Phone sign-in is password-based (POST /sign-in/phone-number).',
21092147
);
21102148
}
2111-
const decision = await this.getOtpSendGuard().checkAndRecord(phone);
2112-
if (!decision.ok) {
2113-
const { APIError } = await import('better-auth/api');
2114-
throw new APIError('TOO_MANY_REQUESTS', {
2115-
message: `Too many verification codes requested for this phone number. Retry in ${decision.retryAfterSeconds ?? 60}s.`,
2116-
});
2117-
}
21182149
const otpCfg = this.config.phoneOtp ?? {};
21192150
const minutes = Math.max(1, Math.round((otpCfg.expiresIn ?? 300) / 60));
21202151
const result = await sms.send({

packages/services/service-messaging/src/messaging-service-plugin.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -214,7 +214,7 @@ export class MessagingServicePlugin implements Plugin {
214214
});
215215

216216
// SMS channel (#2780): same pattern as email — register when an
217-
// `sms` service (plugin-sms) is present at kernel:ready; absent
217+
// `sms` service (service-sms) is present at kernel:ready; absent
218218
// sms ⇒ no channel, so a notify(channels:['sms']) reports "not
219219
// registered" rather than silently no-opping.
220220
const getSms = () => {

packages/services/service-messaging/src/sms-channel.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -16,7 +16,7 @@ import {
1616
import { USER_OBJECT } from './email-channel.js';
1717

1818
/**
19-
* Structural view of the SMS service (`@objectstack/plugin-sms`'s
19+
* Structural view of the SMS service (`@objectstack/service-sms`'s
2020
* `SmsService`), declared locally so service-messaging takes no runtime
2121
* dependency on it — the channel resolves whatever is registered under the
2222
* `sms` service and sends through this shape (mirrors `EmailSenderSurface`

packages/services/service-settings/src/manifests/sms.manifest.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -81,6 +81,6 @@ export const smsTestActionHandler: SettingsActionHandler = async ({ values }) =>
8181
return {
8282
ok: true,
8383
severity: 'info',
84-
message: `Configuration looks valid (provider=${provider}). Wire @objectstack/plugin-sms for actual delivery.`,
84+
message: `Configuration looks valid (provider=${provider}). Wire @objectstack/service-sms for actual delivery.`,
8585
};
8686
};

0 commit comments

Comments
 (0)