diff --git a/.changeset/24767-settings-test-send-recipient.md b/.changeset/24767-settings-test-send-recipient.md new file mode 100644 index 00000000000..7e63ea7c421 --- /dev/null +++ b/.changeset/24767-settings-test-send-recipient.md @@ -0,0 +1,20 @@ +--- +'@objectstack/plugin-email': patch +'@objectstack/service-sms': patch +--- + +fix(plugin-email, service-sms): Setup's "Send test email" and "Send test SMS" send to the signed-in admin when the request names no recipient (#24767) + +Clause-②: no + +The Mail and SMS settings pages offer a Send test button and no recipient input, so the console's request carries no `to`. "Send test email" fell back to the From address — often a no-reply mailbox nobody reads — and with From empty it refused, asking for a "to" address the page has no field for. "Send test SMS" refused every press the same way. + +With no `to` in the request, the test now goes to the person who pressed the button: `mail/test` sends to the caller's own account email and `sms/test` to the caller's own phone number, each read from the caller's own `sys_user` row. A `to` in the request body still wins, as before. When the caller's account has no address or number, or the account cannot be read, the action refuses and says that nothing was sent. A phone number is on an account only where phone sign-in is enabled, so on other deployments "Send test SMS" still needs a `to` in the request body. + +`sms/test` no longer reports a send through the log-only transport (the namespace's default `None (log only)` provider) as a success. It answers `ok: false` with severity `warning`, as `mail/test` already does for its log transport. When the form names no delivering provider and the transport the deployment configured at boot carries the test, the success message names that transport instead of saying the message went "via log". + +**Unchanged.** + +- The request and result shapes of `POST /api/settings/mail/test` and `POST /api/settings/sms/test`: no key, route or setting is added. +- Which provider and credentials a test uses: the saved values overlaid with the form's unsaved edits, as before. +- Saving the Mail form still judges a required field only when the save writes it, or writes a key its visibility depends on. diff --git a/packages/plugins/plugin-email/src/email-plugin.queue-delivery.test.ts b/packages/plugins/plugin-email/src/email-plugin.queue-delivery.test.ts index 85dc95cc181..f8d6ed63b31 100644 --- a/packages/plugins/plugin-email/src/email-plugin.queue-delivery.test.ts +++ b/packages/plugins/plugin-email/src/email-plugin.queue-delivery.test.ts @@ -517,8 +517,7 @@ describe('mail/test is never queued', () => { const result = await h.settings.action('test')!({ values: { provider: 'log', from_email: 'no-reply@example.test' }, - payload: {}, - ctx: { body: { to: 'ops@example.test' } }, + payload: { to: 'ops@example.test' }, }); // The answer came from the TRANSPORT — it either sent or it did not — @@ -542,8 +541,7 @@ describe('mail/test is never queued', () => { const result = await h.settings.action('test')!({ values: { provider: 'log', from_email: 'no-reply@example.test' }, - payload: {}, - ctx: { body: { to: 'ops@example.test' } }, + payload: { to: 'ops@example.test' }, }); expect(result.ok).toBe(false); diff --git a/packages/plugins/plugin-email/src/email-plugin.ts b/packages/plugins/plugin-email/src/email-plugin.ts index e7545d5d86e..5ba4260f224 100644 --- a/packages/plugins/plugin-email/src/email-plugin.ts +++ b/packages/plugins/plugin-email/src/email-plugin.ts @@ -148,12 +148,77 @@ interface MailSettingsSurface { action: string, handler: (input: { values?: Record; + /** The request body the action was invoked with — the console posts its unsaved form edits here. */ payload?: Record; - ctx?: { body?: Record }; + /** The caller's settings context; `userId` is the signed-in admin who pressed the button. */ + ctx?: { userId?: string }; }) => Promise, ): void; } +/** The identity object a `mail/test` press reads the caller's own address from. */ +const ACCOUNT_OBJECT = 'sys_user'; + +/** + * Who a `mail/test` press sends to: the `to` the request names, else the + * signed-in admin's own account address. + * + * The served form has no recipient input — a settings manifest cannot declare + * a value an action posts without storing it — so the console's request + * carries no `to`. This used to fall back to `from_email`, the SENDER address + * (usually a no-reply mailbox nobody reads), so "Sent test email to no-reply@…" + * reported a delivery the operator could not observe, and with From empty the + * button refused by asking for a "to" field the page does not have. The person + * who pressed the button is the one recipient who can confirm the delivery, + * and a signed-in admin has an account address. + * + * The read is pinned to the CALLER's own row — `where.id` is the request's + * own `userId`, never a value the body names — and projects `email` alone. It + * runs as the system because resolving who the caller is must not depend on + * the caller holding read permission on the identity table (the reason the + * shared authz resolver's own `sys_user` read gives); the one value it yields + * reaches the caller in the result message, and it is their own address, so + * nothing the caller could not already read is disclosed. + * + * Every refusal says that nothing was sent and what to supply. A read that + * fails is reported as a failed read, never as "you have no address". + */ +async function resolveMailTestRecipient( + payload: unknown, + userId: unknown, + engine: IDataEngine, +): Promise<{ to: string } | { refusal: string }> { + const named = payload && typeof payload === 'object' ? (payload as { to?: unknown }).to : undefined; + if (typeof named === 'string' && named.trim() !== '') return { to: named.trim() }; + if (typeof userId !== 'string' || userId === '') { + return { + refusal: 'No recipient — nothing was sent. This call names no "to" address and carries no signed-in ' + + 'user whose own address the test could go to; pass "to" in the request body.', + }; + } + let row: Record | null; + try { + row = await engine.findOne( + ACCOUNT_OBJECT, + { where: { id: userId }, fields: ['email'] }, + { context: { isSystem: true } }, + ); + } catch (err: any) { + return { + refusal: `Could not read your account's email address to send the test to (${err?.message ?? String(err)}) ` + + '— nothing was sent. Pass "to" in the request body to name a recipient.', + }; + } + const email = row?.email; + if (typeof email !== 'string' || email.trim() === '') { + return { + refusal: 'Your account has no email address, and the test is sent to the signed-in admin\'s own address ' + + '— nothing was sent. Add an email address to your account, or pass "to" in the request body.', + }; + } + return { to: email.trim() }; +} + /** * Resolve a queue service that can actually carry a durable email job, or * `undefined`. @@ -562,17 +627,20 @@ export class EmailServicePlugin implements Plugin { // provider/credentials differ from what the live `svc` is bound // to, a one-shot temporary `EmailService` is built so the // operator can validate edits before hitting "Save". + // + // The recipient is the request's `to`, else the signed-in admin's + // own address — see `resolveMailTestRecipient` for why it is not + // `from_email`. if (typeof settings.registerAction === 'function') { const svc = this.service; settings.registerAction('mail', 'test', async ({ values, payload, ctx: actionCtx }: any) => { const overrides = extractOverrides(payload); const merged: Record = { ...(values ?? {}), ...overrides }; - const to = (actionCtx?.body?.to as string | undefined) - ?? (payload?.to as string | undefined) - ?? (merged.from_email as string | undefined); - if (!to) { - return { ok: false, severity: 'error', message: 'Provide a "to" address (or set from_email).' }; + const recipient = await resolveMailTestRecipient(payload, actionCtx?.userId, engine); + if ('refusal' in recipient) { + return { ok: false, severity: 'error', message: recipient.refusal }; } + const { to } = recipient; // Build a temporary service from the merged values when // the form differs from the live svc — covers the diff --git a/packages/plugins/plugin-email/src/mail-test-recipient.pin.test.ts b/packages/plugins/plugin-email/src/mail-test-recipient.pin.test.ts new file mode 100644 index 00000000000..89485686455 --- /dev/null +++ b/packages/plugins/plugin-email/src/mail-test-recipient.pin.test.ts @@ -0,0 +1,196 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. +// +// Who "Send test email" sends to, driven through the SERVED door: a real +// `SettingsService` holding the real `mail` manifest, its real REST routes, and +// the real `mail/test` handler that `EmailServicePlugin` registers at +// `kernel:ready`. +// +// The served form has no recipient input, so the console's request body is its +// unsaved edits — `{}` on a page nobody has edited — and carries no `to`. The +// handler used to fall back to `from_email`, the sender address: a "success" +// mailed to a no-reply box, or, with From empty, a refusal asking for a field +// the page does not have. The pins: with no `to`, the test goes to the signed-in +// admin's own address, read from their own `sys_user` row; an explicit `to` +// still wins; and every refusal that remains carries the ADR-0112 envelope. + +import { describe, it, expect, vi, beforeEach } from 'vitest'; +import type { IHttpServer, IHttpRequest, IHttpResponse, RouteHandler } from '@objectstack/spec/contracts'; +import { assertEngineFindOnePredicate, assertEngineUpdateDispatch } from '@objectstack/objectql'; +import { SettingsService, mailSettingsManifest, registerSettingsRoutes } from '@objectstack/service-settings'; +import { EmailServicePlugin } from './email-plugin.js'; + +const nm = vi.hoisted(() => ({ createTransport: vi.fn(), sendMail: vi.fn() })); +vi.mock('nodemailer', () => ({ + default: { createTransport: nm.createTransport }, + createTransport: nm.createTransport, +})); + +class MockHttp implements IHttpServer { + routes = new Map(); + private add(method: string, path: string, handler: RouteHandler) { + this.routes.set(`${method} ${path}`, handler); + } + get(path: string, h: RouteHandler) { this.add('GET', path, h); return this as any; } + post(path: string, h: RouteHandler) { this.add('POST', path, h); return this as any; } + put(path: string, h: RouteHandler) { this.add('PUT', path, h); return this as any; } + delete(path: string, h: RouteHandler) { this.add('DELETE', path, h); return this as any; } + patch(path: string, h: RouteHandler) { this.add('PATCH', path, h); return this as any; } + use() { return this as any; } + listen() { return Promise.resolve(); } + close() { return Promise.resolve(); } + getInstance() { return null; } +} + +const ADMIN_ID = 'usr_admin'; +const ADMIN_EMAIL = 'admin@acme.test'; +const FROM_EMAIL = 'no-reply@acme.test'; + +type UserRow = { id: string; email?: string | null }; + +/** The engine double: `update` and `findOne` held to ObjectQL's own dispatch contract. */ +function fakeEngine(users: UserRow[], opts: { userReadFails?: boolean } = {}) { + return { + insert: vi.fn(async (_object: string, row: any) => ({ id: row?.id })), + update: vi.fn(async (_object: string, data: any, options?: any) => { + assertEngineUpdateDispatch(data, options); + return undefined; + }), + find: vi.fn(async () => []), + findOne: vi.fn(async (object: string, query: any) => { + assertEngineFindOnePredicate(object, query); + if (object !== 'sys_user') return null; + if (opts.userReadFails) throw new Error('sys_user is unavailable'); + return users.find((u) => u.id === query?.where?.id) ?? null; + }), + }; +} + +function fakeCtx(services: Record) { + const hooks: Record Promise | void>> = {}; + return { + logger: { info: vi.fn(), warn: vi.fn(), error: vi.fn(), debug: vi.fn() }, + getService: (name: string): T => { + if (!(name in services)) throw new Error(`service '${name}' not registered`); + return services[name] as T; + }, + registerService: (name: string, svc: unknown) => { services[name] = svc; }, + hook: (name: string, fn: () => Promise | void) => { (hooks[name] ??= []).push(fn); }, + fire: async (name: string) => { for (const fn of hooks[name] ?? []) await fn(); }, + }; +} + +/** Boot the served `mail/test` door: real settings service + routes, the plugin's real action. */ +async function serve(users: UserRow[], opts: { userReadFails?: boolean } = {}) { + const settings = new SettingsService({ env: {} }); + settings.registerManifest(mailSettingsManifest); + await settings.setMany('mail', { provider: 'smtp', smtp_host: 'smtp.acme.test', from_email: FROM_EMAIL }); + const engine = fakeEngine(users, opts); + const services: Record = { manifest: { register: () => {} }, objectql: engine, settings }; + const ctx = fakeCtx(services); + const plugin = new EmailServicePlugin(); + await plugin.init(ctx as never); + await plugin.start(ctx as never); + await ctx.fire('kernel:ready'); + + const http = new MockHttp(); + registerSettingsRoutes(http, settings, { + contextFromRequest: () => ({ + enforced: true, + permissions: ['manage_platform_settings'], + userId: ADMIN_ID, + }), + }); + + /** POST /api/settings/mail/test with the body the console sends. */ + const pressSendTest = async (body: unknown) => { + const handler = http.routes.get('POST /api/settings/:namespace/:actionId'); + if (!handler) throw new Error('the settings action route is not mounted'); + const captured: { status: number; body: any } = { status: 200, body: undefined }; + const res: IHttpResponse = { + json: vi.fn((data: any) => { captured.body = data; }) as any, + send: vi.fn() as any, + status: vi.fn((code: number) => { captured.status = code; return res; }) as any, + header: vi.fn(() => res) as any, + }; + const req: IHttpRequest = { + params: { namespace: 'mail', actionId: 'test' }, + query: {}, + body, + headers: {}, + method: 'POST', + path: '/api/settings/mail/test', + }; + await handler(req, res); + return captured; + }; + return { engine, settings, pressSendTest }; +} + +/** The `to` of the one message nodemailer was handed, as text. */ +const sentTo = () => JSON.stringify(nm.sendMail.mock.calls[0]?.[0]?.to); + +beforeEach(() => { + nm.createTransport.mockReset(); + nm.sendMail.mockReset(); + nm.createTransport.mockImplementation(() => ({ sendMail: nm.sendMail, close: vi.fn() })); + nm.sendMail.mockResolvedValue({ messageId: '', response: '250 2.0.0 Ok' }); +}); + +describe('served mail/test — the recipient', () => { + it('with no "to", sends to the signed-in admin\'s own address, not to From', async () => { + const { engine, pressSendTest } = await serve([{ id: ADMIN_ID, email: ADMIN_EMAIL }]); + + const answer = await pressSendTest({}); + + expect(answer.status).toBe(200); + expect(answer.body?.data).toMatchObject({ ok: true }); + expect(nm.sendMail).toHaveBeenCalledTimes(1); + expect(sentTo()).toContain(ADMIN_EMAIL); + expect(sentTo()).not.toContain(FROM_EMAIL); + // The read is the caller's OWN row, projected to the address alone. + expect(engine.findOne).toHaveBeenCalledWith( + 'sys_user', + { where: { id: ADMIN_ID }, fields: ['email'] }, + { context: expect.objectContaining({ isSystem: true }) }, + ); + }); + + it('control: an explicit "to" still wins, and no account is read', async () => { + const { engine, pressSendTest } = await serve([{ id: ADMIN_ID, email: ADMIN_EMAIL }]); + + const answer = await pressSendTest({ to: 'ops@acme.test' }); + + expect(answer.status).toBe(200); + expect(nm.sendMail).toHaveBeenCalledTimes(1); + expect(sentTo()).toContain('ops@acme.test'); + expect(sentTo()).not.toContain(ADMIN_EMAIL); + expect(engine.findOne).not.toHaveBeenCalledWith('sys_user', expect.anything(), expect.anything()); + }); + + it.each([ + ['an account with no email address', [{ id: ADMIN_ID, email: null }], {}], + ['an account row that is missing', [], {}], + ['a failed account read', [{ id: ADMIN_ID, email: ADMIN_EMAIL }], { userReadFails: true }], + ] as const)('refuses, sending nothing, for %s — ADR-0112 envelope', async (_name, users, opts) => { + const { pressSendTest } = await serve([...users], opts); + + const answer = await pressSendTest({}); + + expect(answer.status).toBe(400); + expect(answer.body?.success).toBe(false); + expect(answer.body?.error?.code).toBe('SETTINGS_ACTION_FAILED'); + expect(answer.body?.error?.details).toMatchObject({ ok: false, severity: 'error' }); + expect(nm.sendMail).not.toHaveBeenCalled(); + }); + + it('an in-process call with no "to" and no user refuses rather than mailing From', async () => { + // Not a public door — the routes refuse an anonymous caller before any + // action runs — but the trusted in-process path reaches the same handler. + const { settings } = await serve([{ id: ADMIN_ID, email: ADMIN_EMAIL }]); + + const result = await settings.runAction('mail', 'test', {}, {}); + + expect(result).toMatchObject({ ok: false, severity: 'error' }); + expect(nm.sendMail).not.toHaveBeenCalled(); + }); +}); diff --git a/packages/services/service-sms/src/sms-plugin.test.ts b/packages/services/service-sms/src/sms-plugin.test.ts index 3067bf3559b..b88a4eb4b60 100644 --- a/packages/services/service-sms/src/sms-plugin.test.ts +++ b/packages/services/service-sms/src/sms-plugin.test.ts @@ -161,11 +161,58 @@ describe('SmsServicePlugin', () => { const test = harness.actions.get('sms/test'); expect(test).toBeDefined(); + const send = vi.spyOn((harness.services.get('sms') as SmsService).options.transport, 'send'); const bad = await test!({ values: { provider: 'log' }, payload: { to: 'not-a-phone' }, ctx: {} }); - expect(bad.ok).toBe(false); - const good = await test!({ values: { provider: 'log' }, payload: { to: '+15005550006' }, ctx: {} }); - expect(good.ok).toBe(true); - expect(good.message).not.toContain('5550006'); // masked + expect(bad).toMatchObject({ ok: false, severity: 'error' }); + expect(send).not.toHaveBeenCalled(); + }); + + it('never reports success while only the LogSmsTransport is active', async () => { + // The mail/test rule: a log "send" is not a delivery. `log` is this + // namespace's default, so this is the answer a fresh install's button gives. + const harness = fakeCtx({ settingsValues: { provider: 'log' } }); + const plugin = new SmsServicePlugin(); + await plugin.init(harness.ctx); + await plugin.start(harness.ctx); + await harness.fireReady(); + + const svc = harness.services.get('sms') as SmsService; + expect(svc.options.transport).toBeInstanceOf(LogSmsTransport); + const send = vi.spyOn(svc.options.transport, 'send'); + const result = await harness.actions.get('sms/test')!({ + values: { provider: 'log' }, payload: { to: '+15005550006' }, ctx: {}, + }); + expect(result).toMatchObject({ ok: false, severity: 'warning' }); + expect(send).not.toHaveBeenCalled(); + expect(result.message).not.toContain('5550006'); + }); + + it('names the deployment\'s transport, not `log`, when the boot-configured one carried the test', async () => { + // A deployment that configured Twilio at boot, with the namespace's + // `provider` still at its `log` default: the live service delivers, so the + // answer must not say the message went "via log". + const fetchImpl = vi.fn(async () => new Response( + JSON.stringify({ sid: 'SM1', status: 'queued' }), + { status: 201, headers: { 'Content-Type': 'application/json' } }, + )); + const harness = fakeCtx({ + settingsValues: { provider: 'log' }, + settingsSources: { provider: 'default' }, + }); + const plugin = new SmsServicePlugin({ + provider: 'twilio', + providerOptions: { accountSid: 'AC1', authToken: 't', from: '+15005550006', fetchImpl }, + }); + await plugin.init(harness.ctx); + await plugin.start(harness.ctx); + await harness.fireReady(); + + const result = await harness.actions.get('sms/test')!({ + values: { provider: 'log' }, payload: { to: '+15005550009' }, ctx: {}, + }); + expect(result).toMatchObject({ ok: true, severity: 'info' }); + expect(fetchImpl).toHaveBeenCalledTimes(1); + expect(result.message).not.toMatch(/via log\b/); }); }); diff --git a/packages/services/service-sms/src/sms-plugin.ts b/packages/services/service-sms/src/sms-plugin.ts index 15ebabe636b..2c802fec503 100644 --- a/packages/services/service-sms/src/sms-plugin.ts +++ b/packages/services/service-sms/src/sms-plugin.ts @@ -1,7 +1,7 @@ // Copyright (c) 2025 ObjectStack. Licensed under the Apache-2.0 license. import type { Plugin, PluginContext } from '@objectstack/core'; -import type { ISmsTransport } from '@objectstack/spec/contracts'; +import type { IDataEngine, ISmsTransport } from '@objectstack/spec/contracts'; import { createLazyCounterStore, type CounterStore } from '@objectstack/plugin-auth/rate-limit-storage'; import { SmsService, LogSmsTransport, maskPhoneNumber, normalizeSmsRecipient } from './sms-service.js'; import { SmsDailyQuota } from './sms-daily-quota.js'; @@ -51,6 +51,88 @@ function providerFromSettings(values: Record): { return { provider: 'log', options: {} }; } +/** The identity object an `sms/test` press reads the caller's own number from. */ +const ACCOUNT_OBJECT = 'sys_user'; + +/** + * The system opt-in for the one `sys_user` read below: resolving who the + * caller is must not depend on the caller holding read permission on the + * identity table. + */ +const SYSTEM_CTX = { isSystem: true } as const; + +/** + * Who an `sms/test` press sends to: the `to` the request names, else the + * signed-in admin's own number (`sys_user.phone_number`, the column the + * messaging `sms` channel delivers to). + * + * The served form has no recipient input — a settings manifest cannot declare + * a value an action posts without storing it — so the console's request + * carries no `to`, and this button used to refuse every press by asking for a + * "to" field the page does not have. The person who pressed it is the one + * recipient who can confirm the delivery. + * + * The read is pinned to the CALLER's own row — `where.id` is the request's own + * `userId`, never a value the body names — and projects `phone_number` alone; + * the success message masks it. A user carries a number only where phone + * sign-in is enabled, so the refusal for a caller without one says exactly + * that, and that nothing was sent. A read that fails is reported as a failed + * read, never as "you have no number". + */ +async function resolveSmsTestRecipient( + payload: unknown, + userId: unknown, + getEngine: () => IDataEngine | undefined, +): Promise<{ to: string } | { refusal: string }> { + const named = payload && typeof payload === 'object' ? (payload as { to?: unknown }).to : undefined; + if (typeof named === 'string' && named.trim() !== '') { + const to = normalizeSmsRecipient(named); + return to ? { to } : { refusal: 'Provide a valid "to" phone number (E.164 recommended).' }; + } + if (typeof userId !== 'string' || userId === '') { + return { + refusal: 'No recipient — nothing was sent. This call names no "to" number and carries no signed-in ' + + 'user whose own number the test could go to; pass "to" in the request body.', + }; + } + const engine = getEngine(); + if (!engine) { + return { + refusal: 'No data engine is available to read your account\'s phone number — nothing was sent. ' + + 'Pass "to" in the request body to name a recipient.', + }; + } + let row: Record | null; + try { + row = await engine.findOne( + ACCOUNT_OBJECT, + { where: { id: userId }, fields: ['phone_number'] }, + { context: SYSTEM_CTX }, + ); + } catch (err: any) { + return { + refusal: `Could not read your account's phone number to send the test SMS to (${err?.message ?? String(err)}) ` + + '— nothing was sent. Pass "to" in the request body to name a recipient.', + }; + } + const raw = row?.phone_number; + const to = typeof raw === 'string' ? normalizeSmsRecipient(raw) : undefined; + if (!to) { + return { + refusal: 'Your account has no phone number, and the test SMS is sent to the signed-in admin\'s own number ' + + '— nothing was sent. A number is on an account only where phone sign-in is enabled; otherwise pass ' + + '"to" in the request body.', + }; + } + return { to }; +} + +/** The data engine, looked up when an action runs — `objectql`, else `data`. */ +function resolveDataEngine(ctx: PluginContext): IDataEngine | undefined { + try { return ctx.getService('objectql') ?? undefined; } catch { /* fall through */ } + try { return ctx.getService('data') ?? undefined; } catch { return undefined; } +} + /** * SmsServicePlugin — registers the `sms` service (#2780). * @@ -236,6 +318,9 @@ export class SmsServicePlugin implements Plugin { // the `mail/test` handler in EmailServicePlugin. Still skipped when the // host injected its own transport: the form's provider fields describe // nothing that composition uses, so testing them would be theatre. + // + // The recipient is the request's `to`, else the signed-in admin's own + // number — see `resolveSmsTestRecipient`. if (!this.options.transport && typeof settings.registerAction === 'function') { const svc = this.service; settings.registerAction('sms', 'test', async ({ values, payload, ctx: actionCtx }: any) => { @@ -243,11 +328,11 @@ export class SmsServicePlugin implements Plugin { ? payload.values : (payload ?? {}); const merged: Record = { ...(values ?? {}), ...overrides }; - const rawTo = (actionCtx?.body?.to as string | undefined) ?? (payload?.to as string | undefined); - const to = rawTo ? normalizeSmsRecipient(rawTo) : undefined; - if (!to) { - return { ok: false, severity: 'error', message: 'Provide a valid "to" phone number (E.164 recommended).' }; + const recipient = await resolveSmsTestRecipient(payload, actionCtx?.userId, () => resolveDataEngine(ctx)); + if ('refusal' in recipient) { + return { ok: false, severity: 'error', message: recipient.refusal }; } + const { to } = recipient; const resolved = providerFromSettings(merged); if (resolved.missing) { @@ -263,6 +348,18 @@ export class SmsServicePlugin implements Plugin { return { ok: false, severity: 'error', message: `Failed to build ${resolved.provider} transport: ${err?.message ?? String(err)}` }; } } + // A LogSmsTransport "send" is not a delivery — say so instead of + // reporting a success it never had (the mail/test rule). Reachable + // from the served form now that the recipient defaults to the + // admin's own number, and `log` is this namespace's default. + if (target === svc && svc.options.transport instanceof LogSmsTransport) { + return { + ok: false, + severity: 'warning', + message: 'No SMS delivery transport is active (provider: None — log only), so the test SMS was NOT sent. ' + + 'Pick a provider, fill in its credentials and save before testing.', + }; + } try { const result = await target.send({ @@ -273,10 +370,14 @@ export class SmsServicePlugin implements Plugin { if (result.status === 'failed') { return { ok: false, severity: 'error', message: result.error ?? 'Send failed.' }; } + // Name the transport that actually carried it. When the form + // names no delivering provider, the live service did — the one + // the deployment configured at boot, never `log` (refused above). + const via = target === svc ? 'the deployment\'s configured transport' : resolved.provider; return { ok: true, severity: 'info', - message: `Sent test SMS to ${maskPhoneNumber(to)} via ${resolved.provider} (id=${result.messageId ?? result.id}).`, + message: `Sent test SMS to ${maskPhoneNumber(to)} via ${via} (id=${result.messageId ?? result.id}).`, }; } catch (err: any) { return { ok: false, severity: 'error', message: err?.message ?? String(err) }; diff --git a/packages/services/service-sms/src/sms-test-recipient.pin.test.ts b/packages/services/service-sms/src/sms-test-recipient.pin.test.ts new file mode 100644 index 00000000000..f4ff87115e0 --- /dev/null +++ b/packages/services/service-sms/src/sms-test-recipient.pin.test.ts @@ -0,0 +1,181 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. +// +// Who "Send test SMS" sends to, driven through the SERVED door: a real +// `SettingsService` holding the real `sms` manifest, its real REST routes, and +// the real `sms/test` handler that `SmsServicePlugin` registers at +// `kernel:ready`. +// +// The served form has no recipient input, so the console's request body is its +// unsaved edits and carries no `to` — and the handler refused every press by +// asking for one. The pins: with no `to`, the test goes to the signed-in admin's +// own number, read from their own `sys_user` row; an explicit `to` still wins; +// and every refusal that remains carries the ADR-0112 envelope. + +import { describe, it, expect, vi, afterEach } from 'vitest'; +import type { IHttpServer, IHttpRequest, IHttpResponse, RouteHandler } from '@objectstack/spec/contracts'; +import { SettingsService, smsSettingsManifest, registerSettingsRoutes } from '@objectstack/service-settings'; +import { SmsServicePlugin } from './sms-plugin.js'; + +class MockHttp implements IHttpServer { + routes = new Map(); + private add(method: string, path: string, handler: RouteHandler) { + this.routes.set(`${method} ${path}`, handler); + } + get(path: string, h: RouteHandler) { this.add('GET', path, h); return this as any; } + post(path: string, h: RouteHandler) { this.add('POST', path, h); return this as any; } + put(path: string, h: RouteHandler) { this.add('PUT', path, h); return this as any; } + delete(path: string, h: RouteHandler) { this.add('DELETE', path, h); return this as any; } + patch(path: string, h: RouteHandler) { this.add('PATCH', path, h); return this as any; } + use() { return this as any; } + listen() { return Promise.resolve(); } + close() { return Promise.resolve(); } + getInstance() { return null; } +} + +const ADMIN_ID = 'usr_admin'; +const ADMIN_PHONE = '+8613800000001'; + +/** The console's draft after an admin typed Twilio credentials and did not save yet. */ +const TWILIO_DRAFT = { + provider: 'twilio', + twilio_account_sid: 'AC_test', + twilio_auth_token: 'tok', + twilio_from_number: '+15005550006', +}; + +type UserRow = { id: string; phone_number?: string | null }; + +function fakeEngine(users: UserRow[], opts: { userReadFails?: boolean } = {}) { + return { + findOne: vi.fn(async (object: string, query: any) => { + if (object !== 'sys_user') return null; + if (opts.userReadFails) throw new Error('sys_user is unavailable'); + return users.find((u) => u.id === query?.where?.id) ?? null; + }), + }; +} + +async function serve(users: UserRow[], opts: { userReadFails?: boolean } = {}) { + const settings = new SettingsService({ env: {} }); + settings.registerManifest(smsSettingsManifest); + const engine = fakeEngine(users, opts); + const services = new Map([['settings', settings], ['objectql', engine]]); + const readyHooks: Array<() => Promise | void> = []; + const ctx = { + logger: { info: vi.fn(), warn: vi.fn(), error: vi.fn(), debug: vi.fn() }, + registerService(name: string, svc: unknown) { services.set(name, svc); }, + getService(name: string) { + if (!services.has(name)) throw new Error(`service not found: ${name}`); + return services.get(name); + }, + hook(event: string, fn: () => Promise | void) { + if (event === 'kernel:ready') readyHooks.push(fn); + }, + } as any; + const plugin = new SmsServicePlugin(); + await plugin.init(ctx); + await plugin.start(ctx); + for (const fn of readyHooks) await fn(); + + const http = new MockHttp(); + registerSettingsRoutes(http, settings, { + contextFromRequest: () => ({ enforced: true, permissions: ['manage_platform_settings'], userId: ADMIN_ID }), + }); + + /** Twilio's REST endpoint, as the one-shot test transport reaches it. */ + const twilio = vi.fn(async (_url: string, _init: any) => new Response( + JSON.stringify({ sid: 'SM_test', status: 'queued' }), + { status: 201, headers: { 'Content-Type': 'application/json' } }, + )); + vi.stubGlobal('fetch', twilio); + + /** POST /api/settings/sms/test with the body the console sends. */ + const pressSendTest = async (body: unknown) => { + const handler = http.routes.get('POST /api/settings/:namespace/:actionId'); + if (!handler) throw new Error('the settings action route is not mounted'); + const captured: { status: number; body: any } = { status: 200, body: undefined }; + const res: IHttpResponse = { + json: vi.fn((data: any) => { captured.body = data; }) as any, + send: vi.fn() as any, + status: vi.fn((code: number) => { captured.status = code; return res; }) as any, + header: vi.fn(() => res) as any, + }; + const req: IHttpRequest = { + params: { namespace: 'sms', actionId: 'test' }, + query: {}, + body, + headers: {}, + method: 'POST', + path: '/api/settings/sms/test', + }; + await handler(req, res); + return captured; + }; + /** The `To` of the one message Twilio was handed. */ + const sentTo = () => new URLSearchParams(String(twilio.mock.calls[0]?.[1]?.body ?? '')).get('To'); + return { engine, settings, twilio, sentTo, pressSendTest }; +} + +afterEach(() => { vi.unstubAllGlobals(); }); + +describe('served sms/test — the recipient', () => { + it('with no "to", sends to the signed-in admin\'s own number', async () => { + const { engine, twilio, sentTo, pressSendTest } = await serve([{ id: ADMIN_ID, phone_number: ADMIN_PHONE }]); + + const answer = await pressSendTest(TWILIO_DRAFT); + + expect(answer.status).toBe(200); + expect(answer.body?.data).toMatchObject({ ok: true }); + expect(twilio).toHaveBeenCalledTimes(1); + expect(sentTo()).toBe(ADMIN_PHONE); + // The number never travels back in the answer unmasked. + expect(JSON.stringify(answer.body)).not.toContain(ADMIN_PHONE); + // The read is the caller's OWN row, projected to the number alone. + expect(engine.findOne).toHaveBeenCalledWith( + 'sys_user', + { where: { id: ADMIN_ID }, fields: ['phone_number'] }, + { context: expect.objectContaining({ isSystem: true }) }, + ); + }); + + it('control: an explicit "to" still wins, and no account is read', async () => { + const { engine, twilio, sentTo, pressSendTest } = await serve([{ id: ADMIN_ID, phone_number: ADMIN_PHONE }]); + + const answer = await pressSendTest({ ...TWILIO_DRAFT, to: '+15005550009' }); + + expect(answer.status).toBe(200); + expect(twilio).toHaveBeenCalledTimes(1); + expect(sentTo()).toBe('+15005550009'); + expect(engine.findOne).not.toHaveBeenCalled(); + }); + + it.each([ + ['an account with no phone number', [{ id: ADMIN_ID, phone_number: null }], {}, TWILIO_DRAFT, 'error'], + ['an account row that is missing', [], {}, TWILIO_DRAFT, 'error'], + ['a failed account read', [{ id: ADMIN_ID, phone_number: ADMIN_PHONE }], { userReadFails: true }, TWILIO_DRAFT, 'error'], + // `log` is the namespace's default: the press on a fresh install. A log + // "send" is not a delivery, so it is not reported as one. + ['the log-only provider', [{ id: ADMIN_ID, phone_number: ADMIN_PHONE }], {}, {}, 'warning'], + ] as const)('refuses, sending nothing, for %s — ADR-0112 envelope', async (_name, users, opts, draft, severity) => { + const { twilio, pressSendTest } = await serve([...users], opts); + + const answer = await pressSendTest({ ...draft }); + + expect(answer.status).toBe(400); + expect(answer.body?.success).toBe(false); + expect(answer.body?.error?.code).toBe('SETTINGS_ACTION_FAILED'); + expect(answer.body?.error?.details).toMatchObject({ ok: false, severity }); + expect(twilio).not.toHaveBeenCalled(); + }); + + it('an in-process call with no "to" and no user refuses', async () => { + // Not a public door — the routes refuse an anonymous caller before any + // action runs — but the trusted in-process path reaches the same handler. + const { settings, twilio } = await serve([{ id: ADMIN_ID, phone_number: ADMIN_PHONE }]); + + const result = await settings.runAction('sms', 'test', { ...TWILIO_DRAFT }, {}); + + expect(result).toMatchObject({ ok: false, severity: 'error' }); + expect(twilio).not.toHaveBeenCalled(); + }); +}); diff --git a/scripts/engine-double-contract.pinned.json b/scripts/engine-double-contract.pinned.json index f38069d31f2..80609a29c0e 100644 --- a/scripts/engine-double-contract.pinned.json +++ b/scripts/engine-double-contract.pinned.json @@ -3256,6 +3256,16 @@ "verb": "update", "pinned": 1 }, + { + "file": "packages/plugins/plugin-email/src/mail-test-recipient.pin.test.ts", + "verb": "findOne", + "pinned": 1 + }, + { + "file": "packages/plugins/plugin-email/src/mail-test-recipient.pin.test.ts", + "verb": "update", + "pinned": 1 + }, { "file": "packages/plugins/plugin-email/src/plugin-shutdown-detaches-template-bridge.test.ts", "verb": "update",