Skip to content

Commit 5dab06b

Browse files
committed
fix: harden SMS transport and phone handling
1 parent 6531637 commit 5dab06b

5 files changed

Lines changed: 395 additions & 121 deletions

File tree

Lines changed: 189 additions & 46 deletions
Original file line numberDiff line numberDiff line change
@@ -1,33 +1,43 @@
1+
import type { FunctionHandler } from '@constructive-io/fn-runtime';
2+
13
import { createMockContext } from '../../../tests/helpers/mock-context';
4+
import handler, { SendSmsParams, SendSmsResult } from '../handler';
5+
6+
const configuredEnv = {
7+
SMS_SEND_PROVIDER: 'sms-dev',
8+
SMS_DEV_API_URL: 'http://localhost:4001',
9+
};
210

311
describe('send-sms handler', () => {
4-
let handler: any;
5-
let fetchMock: jest.Mock;
12+
const sendSmsHandler: FunctionHandler<SendSmsParams, SendSmsResult> = handler;
13+
const originalFetch = global.fetch;
14+
let fetchMock: jest.MockedFunction<typeof fetch>;
615

716
beforeEach(() => {
8-
jest.resetModules();
9-
fetchMock = jest.fn().mockResolvedValue({ ok: true });
17+
fetchMock = jest.fn().mockResolvedValue(
18+
new Response(null, { status: 201 }),
19+
);
1020
global.fetch = fetchMock;
11-
handler = require('../handler').default;
1221
});
1322

1423
afterEach(() => {
24+
global.fetch = originalFetch;
1525
jest.restoreAllMocks();
1626
});
1727

18-
it('sends sms_otp_code messages to the configured sms-dev API', async () => {
19-
const result = await handler(
28+
it('sends sms_otp_code messages through the configured sms-dev transport', async () => {
29+
const result = await sendSmsHandler(
2030
{
2131
sms_type: 'sms_otp_code',
22-
phone: '+15555550101',
32+
phone: '+12025550101',
2333
code: '123456',
2434
},
2535
createMockContext({
2636
env: {
27-
SMS_DEV_API_URL: 'http://localhost:4001',
37+
...configuredEnv,
2838
SMS_FROM: 'Constructive Test',
2939
},
30-
})
40+
}),
3141
);
3242

3343
expect(result).toEqual({ complete: true });
@@ -36,84 +46,217 @@ describe('send-sms handler', () => {
3646
expect.objectContaining({
3747
method: 'POST',
3848
body: JSON.stringify({
39-
to: '+15555550101',
49+
to: '+12025550101',
4050
from: 'Constructive Test',
4151
body: 'Your verification code is 123456',
4252
}),
43-
})
53+
signal: expect.any(AbortSignal),
54+
}),
4455
);
4556
});
4657

47-
it('sends mfa_verification_code messages with combined phone fields', async () => {
48-
await handler(
58+
it('uses a complete E.164 phone_number from the real DB MFA payload', async () => {
59+
await sendSmsHandler(
4960
{
5061
sms_type: 'mfa_verification_code',
51-
phone_cc: '+1',
52-
phone_number: '(555) 555-0102',
62+
phone_cc: '+',
63+
phone_number: '+12025550102',
5364
code: '234567',
54-
user_id: 'user-1',
65+
user_id: '00000000-0000-0000-0000-000000000001',
5566
},
56-
createMockContext()
67+
createMockContext({ env: configuredEnv }),
5768
);
5869

5970
expect(fetchMock).toHaveBeenCalledWith(
6071
'http://localhost:4001/v1/messages',
6172
expect.objectContaining({
6273
body: JSON.stringify({
63-
to: '+15555550102',
74+
to: '+12025550102',
6475
from: 'Constructive',
6576
body: 'Your verification code is 234567',
6677
}),
67-
})
78+
}),
6879
);
6980
});
7081

71-
it('skips capture API calls in dry-run mode', async () => {
72-
const result = await handler(
82+
it('combines a country code with a local formatted number', async () => {
83+
await sendSmsHandler(
7384
{
74-
sms_type: 'sms_otp_code',
75-
phone: '+15555550103',
85+
sms_type: 'mfa_verification_code',
86+
phone_cc: ' +1 ',
87+
phone_number: ' (202) 555-0103 ',
7688
code: '345678',
7789
},
78-
createMockContext({
79-
env: {
80-
SMS_SEND_DRY_RUN: 'true',
81-
},
82-
})
90+
createMockContext({ env: configuredEnv }),
91+
);
92+
93+
expect(fetchMock).toHaveBeenCalledWith(
94+
'http://localhost:4001/v1/messages',
95+
expect.objectContaining({
96+
body: expect.stringContaining('"to":"+12025550103"'),
97+
}),
98+
);
99+
});
100+
101+
it('normalizes whitespace, parentheses, and hyphens in a direct phone', async () => {
102+
await sendSmsHandler(
103+
{
104+
sms_type: 'sms_otp_code',
105+
phone: ' +1 (202) 555-0104 ',
106+
code: '456789',
107+
},
108+
createMockContext({ env: configuredEnv }),
109+
);
110+
111+
expect(fetchMock).toHaveBeenCalledWith(
112+
'http://localhost:4001/v1/messages',
113+
expect.objectContaining({
114+
body: expect.stringContaining('"to":"+12025550104"'),
115+
}),
116+
);
117+
});
118+
119+
it('allows dry-run without configuring a transport', async () => {
120+
const result = await sendSmsHandler(
121+
{
122+
sms_type: 'sms_otp_code',
123+
phone: '+12025550105',
124+
code: '567890',
125+
},
126+
createMockContext({ env: { SMS_SEND_DRY_RUN: 'true' } }),
83127
);
84128

85129
expect(result).toEqual({ complete: true, dryRun: true });
86130
expect(fetchMock).not.toHaveBeenCalled();
87131
});
88132

89-
it('throws for missing sms_type', async () => {
133+
it('fails when the provider is not configured', async () => {
90134
await expect(
91-
handler({ phone: '+15555550104', code: '456789' }, createMockContext())
92-
).rejects.toThrow('Missing required field: sms_type');
135+
sendSmsHandler(
136+
{
137+
sms_type: 'sms_otp_code',
138+
phone: '+12025550106',
139+
code: '678901',
140+
},
141+
createMockContext({ env: { SMS_DEV_API_URL: 'http://localhost:4001' } }),
142+
),
143+
).rejects.toThrow('Missing required field: SMS_SEND_PROVIDER');
93144
});
94145

95-
it('throws for unsupported sms_type', async () => {
146+
it('fails when the sms-dev URL is not configured', async () => {
96147
await expect(
97-
handler(
148+
sendSmsHandler(
98149
{
99-
sms_type: 'sms_invite',
100-
phone: '+15555550104',
101-
code: '456789',
150+
sms_type: 'sms_otp_code',
151+
phone: '+12025550107',
152+
code: '789012',
102153
},
103-
createMockContext()
104-
)
105-
).rejects.toThrow('Unsupported sms_type: sms_invite');
154+
createMockContext({ env: { SMS_SEND_PROVIDER: 'sms-dev' } }),
155+
),
156+
).rejects.toThrow('Missing required field: SMS_DEV_API_URL');
106157
});
107158

108-
it('throws for missing code', async () => {
159+
it('reports non-2xx status without exposing the response body', async () => {
160+
fetchMock.mockResolvedValueOnce(
161+
new Response('OTP 890123 for +12025550108', {
162+
status: 503,
163+
statusText: 'Service Unavailable',
164+
}),
165+
);
166+
167+
const request = sendSmsHandler(
168+
{
169+
sms_type: 'sms_otp_code',
170+
phone: '+12025550108',
171+
code: '890123',
172+
},
173+
createMockContext({ env: configuredEnv }),
174+
);
175+
176+
await expect(request).rejects.toThrow(
177+
'sms-dev API returned 503 Service Unavailable',
178+
);
179+
await expect(request).rejects.not.toThrow('890123');
180+
await expect(request).rejects.not.toThrow('+12025550108');
181+
});
182+
183+
it('aborts a request that exceeds the configured timeout', async () => {
184+
fetchMock.mockImplementationOnce((_input, init) =>
185+
new Promise((_resolve, reject) => {
186+
init?.signal?.addEventListener('abort', () => {
187+
reject(new DOMException('Aborted', 'AbortError'));
188+
});
189+
}),
190+
);
191+
192+
await expect(
193+
sendSmsHandler(
194+
{
195+
sms_type: 'sms_otp_code',
196+
phone: '+12025550109',
197+
code: '901234',
198+
},
199+
createMockContext({
200+
env: { ...configuredEnv, SMS_SEND_TIMEOUT_MS: '5' },
201+
}),
202+
),
203+
).rejects.toThrow('sms-dev request timed out after 5ms');
204+
});
205+
206+
it.each([
207+
[{ sms_type: ' ', phone: '+12025550110', code: '012345' }, 'sms_type'],
208+
[{ sms_type: 'sms_otp_code', phone: '+12025550110', code: ' ' }, 'code'],
209+
[{ sms_type: 'sms_otp_code', phone: ' ', code: '012345' }, 'phone'],
210+
])('rejects whitespace-only required fields', async (params, field) => {
211+
await expect(
212+
sendSmsHandler(params, createMockContext({ env: configuredEnv })),
213+
).rejects.toThrow(`Missing required field: ${field}`);
214+
});
215+
216+
it.each(['12345', '1234567', '12a456'])('rejects malformed OTP %s', async (code) => {
109217
await expect(
110-
handler({ sms_type: 'sms_otp_code', phone: '+15555550104' }, createMockContext())
111-
).rejects.toThrow('Missing required field: code');
218+
sendSmsHandler(
219+
{ sms_type: 'sms_otp_code', phone: '+12025550111', code },
220+
createMockContext({ env: configuredEnv }),
221+
),
222+
).rejects.toThrow('Invalid code: expected a 6-digit OTP');
112223
});
113224

114-
it('throws for missing phone', async () => {
225+
it.each([
226+
{ phone: '++12025550112' },
227+
{ phone: '+02025550112' },
228+
{ phone_cc: '+1', phone_number: '+1+2025550112' },
229+
{ phone_cc: '+1', phone_number: 'not-a-phone' },
230+
])('rejects malformed or duplicate-plus phone input', async (phoneInput) => {
115231
await expect(
116-
handler({ sms_type: 'sms_otp_code', code: '456789' }, createMockContext())
117-
).rejects.toThrow('Missing required field: phone');
232+
sendSmsHandler(
233+
{
234+
sms_type: 'sms_otp_code',
235+
code: '123456',
236+
...phoneInput,
237+
},
238+
createMockContext({ env: configuredEnv }),
239+
),
240+
).rejects.toThrow('Invalid phone: expected E.164 format');
241+
});
242+
243+
it('does not log the OTP or complete phone number', async () => {
244+
const context = createMockContext({ env: configuredEnv });
245+
246+
await sendSmsHandler(
247+
{
248+
sms_type: 'sms_otp_code',
249+
phone: '+12025550113',
250+
code: '234567',
251+
},
252+
context,
253+
);
254+
255+
const logOutput = JSON.stringify(
256+
(context.log.info as jest.Mock).mock.calls,
257+
);
258+
expect(logOutput).not.toContain('234567');
259+
expect(logOutput).not.toContain('+12025550113');
260+
expect(logOutput).toContain('***0113');
118261
});
119262
});

functions/send-sms/handler.json

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -4,5 +4,8 @@
44
"type": "node-graphql",
55
"port": 8092,
66
"taskIdentifier": "sms:send_verification_code",
7-
"description": "Sends SMS verification codes for auth flows"
7+
"description": "Sends SMS verification codes for auth flows",
8+
"dependencies": {
9+
"@pgpmjs/env": "^2.15.3"
10+
}
811
}

0 commit comments

Comments
 (0)