Skip to content

Commit 1868f1e

Browse files
authored
Merge pull request #18312 from mozilla/FXA-11007
feat(sms): Add localized body text to SMS
2 parents 7116d0e + f353d00 commit 1868f1e

File tree

18 files changed

+325
-118
lines changed

18 files changed

+325
-118
lines changed

libs/accounts/recovery-phone/README.md

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -8,4 +8,8 @@ Run `nx build recovery-phone` to build the library.
88

99
## Running unit tests
1010

11-
Run `nx test recovery-phone` to execute the unit tests via [Jest](https://jestjs.io).
11+
Run `nx test-unit recovery-phone` to execute the unit tests via [Jest](https://jestjs.io).
12+
13+
## Running integration tests
14+
15+
Run `nx test-integration recovery-phone` to execute the integration tests via [Jest](https://jestjs.io).

libs/accounts/recovery-phone/src/lib/recovery-phone.manager.in.spec.ts

Lines changed: 13 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -204,20 +204,26 @@ describe('RecoveryPhoneManager', () => {
204204
mockLookUpData
205205
);
206206

207-
const expectedData = JSON.stringify({
208-
createdAt: 1739227529776,
209-
phoneNumber,
210-
isSetup,
211-
lookupData: JSON.stringify(mockLookUpData),
212-
});
213207
const redisKey = `sms-attempt:${uid.toString('hex')}:${code}`;
214208

215209
expect(mockRedis.set).toHaveBeenCalledWith(
216210
redisKey,
217-
expectedData,
211+
expect.any(String),
218212
'EX',
219213
600
220214
);
215+
216+
const expectedData = expect.objectContaining({
217+
createdAt: expect.any(Number),
218+
phoneNumber,
219+
isSetup,
220+
lookupData: JSON.stringify(mockLookUpData),
221+
});
222+
223+
const storedData = mockRedis.set.mock.calls[0][1];
224+
expect(() => JSON.parse(storedData)).not.toThrow();
225+
const parsedData = JSON.parse(mockRedis.set.mock.calls[0][1]);
226+
expect(parsedData).toEqual(expectedData);
221227
});
222228

223229
it('should return null if no unconfirmed phone number data is found in Redis', async () => {

libs/accounts/recovery-phone/src/lib/recovery-phone.service.spec.ts

Lines changed: 25 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -140,6 +140,31 @@ describe('RecoveryPhoneService', () => {
140140
expect(mockRecoveryPhoneManager.getAllUnconfirmed).toBeCalledWith(uid);
141141
});
142142

143+
it('handles message template when provided to setup phone number', async () => {
144+
mockOtpManager.generateCode.mockReturnValue(code);
145+
const getFormattedMessage = jest.fn().mockResolvedValue('message');
146+
147+
const result = await service.setupPhoneNumber(
148+
uid,
149+
phoneNumber,
150+
getFormattedMessage
151+
);
152+
153+
expect(result).toBeTruthy();
154+
expect(mockOtpManager.generateCode).toBeCalled();
155+
expect(mockSmsManager.sendSMS).toBeCalledWith({
156+
to: phoneNumber,
157+
body: 'message',
158+
});
159+
expect(mockRecoveryPhoneManager.storeUnconfirmed).toBeCalledWith(
160+
uid,
161+
code,
162+
phoneNumber,
163+
true
164+
);
165+
expect(mockRecoveryPhoneManager.getAllUnconfirmed).toBeCalledWith(uid);
166+
});
167+
143168
it('Will reject a phone number that is not part of launch', async () => {
144169
const to = '+16005551234';
145170
await expect(service.setupPhoneNumber(uid, to)).rejects.toEqual(

libs/accounts/recovery-phone/src/lib/recovery-phone.service.ts

Lines changed: 32 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -61,9 +61,14 @@ export class RecoveryPhoneService {
6161
* by sending the phone number provided an OTP code to verify.
6262
* @param uid The account id
6363
* @param phoneNumber The phone number to register
64+
* @param localizedMessageBody Optional localized message body
6465
* @returns True if code was sent and stored
6566
*/
66-
public async setupPhoneNumber(uid: string, phoneNumber: string) {
67+
public async setupPhoneNumber(
68+
uid: string,
69+
phoneNumber: string,
70+
getFormattedMessage?: (code: string) => Promise<string>
71+
) {
6772
if (!this.config.enabled) {
6873
throw new RecoveryPhoneNotEnabled();
6974
}
@@ -90,21 +95,26 @@ export class RecoveryPhoneService {
9095
}
9196

9297
const code = await this.otpCode.generateCode();
93-
const msg = await this.smsManager.sendSMS({
94-
to: phoneNumber,
95-
body: code,
96-
});
9798

98-
if (!this.isSuccessfulSmsSend(msg)) {
99-
return false;
100-
}
10199
await this.recoveryPhoneManager.storeUnconfirmed(
102100
uid,
103101
code,
104102
phoneNumber,
105103
true
106104
);
107-
return true;
105+
106+
const formattedSMSbody = getFormattedMessage
107+
? await getFormattedMessage(code)
108+
: undefined;
109+
110+
const smsBody = formattedSMSbody || `${code}`;
111+
112+
const msg = await this.smsManager.sendSMS({
113+
to: phoneNumber,
114+
body: smsBody,
115+
});
116+
117+
return this.isSuccessfulSmsSend(msg);
108118
}
109119

110120
/**
@@ -323,9 +333,13 @@ export class RecoveryPhoneService {
323333
/**
324334
* Sends an totp code to a user
325335
* @param uid Account id
336+
* @param getFormattedMessage Optional template function to format the message
326337
* @returns True if message didn't fail to send.
327338
*/
328-
public async sendCode(uid: string) {
339+
public async sendCode(
340+
uid: string,
341+
getFormattedMessage?: (code: string) => Promise<string>
342+
) {
329343
if (!this.config.enabled) {
330344
throw new RecoveryPhoneNotEnabled();
331345
}
@@ -339,9 +353,16 @@ export class RecoveryPhoneService {
339353
phoneNumber,
340354
false
341355
);
356+
357+
const formattedSMSbody = getFormattedMessage
358+
? await getFormattedMessage(code)
359+
: undefined;
360+
361+
const smsBody = formattedSMSbody || `${code}`;
362+
342363
const msg = await this.smsManager.sendSMS({
343364
to: phoneNumber,
344-
body: `${code}`, // TODO: Other text or translation around code?
365+
body: smsBody,
345366
});
346367

347368
return this.isSuccessfulSmsSend(msg);

packages/functional-tests/lib/email.ts

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -59,6 +59,10 @@ export enum EmailType {
5959
verificationReminderFinal,
6060
cadReminderFirst,
6161
cadReminderSecond,
62+
postAddRecoveryPhone,
63+
postChangeRecoveryPhone,
64+
postRemoveRecoveryPhone,
65+
postSigninRecoveryPhone,
6266
}
6367

6468
export enum EmailHeader {

packages/functional-tests/tests/settings/recoveryPhone.spec.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -103,7 +103,7 @@ test.describe('severity-1 #smoke', () => {
103103
await settings.disconnectTotp();
104104
});
105105

106-
test('can sign-in settings with recovery phone', async ({
106+
test('can sign-in to settings with recovery phone', async ({
107107
target,
108108
pages: {
109109
page,

packages/fxa-auth-server/config/index.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2196,7 +2196,7 @@ const convictConf = convict({
21962196
format: Array,
21972197
},
21982198
maxMessageLength: {
2199-
default: 60,
2199+
default: 160,
22002200
doc: 'Max allows sms message length',
22012201
env: 'RECOVERY_PHONE__SMS__MAX_MESSAGE_LENGTH',
22022202
format: Number,

packages/fxa-auth-server/lib/l10n/index.ts

Lines changed: 10 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -7,9 +7,16 @@ import { FluentBundle, FluentResource } from '@fluent/bundle';
77
import { determineLocale, parseAcceptLanguage } from '@fxa/shared/l10n';
88
import { ILocalizerBindings } from './interfaces/ILocalizerBindings';
99

10+
/**
11+
* Represents a Fluent (FTL) message
12+
* @param id - unique identifier for the message
13+
* @param message - a fallback message in case the localized string cannot be found
14+
* @param vars - optional arguments to be interpolated into the localized string
15+
*/
1016
export interface FtlIdMsg {
1117
id: string;
1218
message: string;
19+
vars?: Record<string, string>;
1320
}
1421

1522
interface LocalizedStrings {
@@ -118,11 +125,11 @@ class Localizer {
118125

119126
const localizedFtlIdMsgs = await Promise.all(
120127
ftlIdMsgs.map(async (ftlIdMsg) => {
121-
const { id, message } = ftlIdMsg;
128+
const { id, message, vars } = ftlIdMsg;
122129
let localizedMessage;
123130
try {
124-
localizedMessage = (await l10n.formatValue(id, message)) || message;
125-
} catch {
131+
localizedMessage = (await l10n.formatValue(id, vars)) || message;
132+
} catch (e) {
126133
localizedMessage = message;
127134
}
128135
return Promise.resolve({

packages/fxa-auth-server/lib/l10n/server.ftl

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2,3 +2,15 @@
22

33
session-verify-send-push-title-2 = Logging in to your { -product-mozilla-account }?
44
session-verify-send-push-body-2 = Click here to confirm it’s you
5+
6+
# Message sent by SMS with limited character length, please test translation with the messaging segment calculator
7+
# https://twiliodeved.github.io/message-segment-calculator/
8+
# Messages should be limited to one segment
9+
# $code - 6 digit code used to verify phone ownership when registering a recovery phone
10+
recovery-phone-setup-sms-body = { $code } is your { -brand-mozilla } verification code. Expires in 5 minutes.
11+
12+
# Message sent by SMS with limited character length, please test translation with the messaging segment calculator
13+
# https://twiliodeved.github.io/message-segment-calculator/
14+
# Messages should be limited to one segment
15+
# $code - 6 digit code used to sign in with a recovery phone as backup for two-step authentication
16+
recovery-phone-signin-sms-body = { $code } is your { -brand-mozilla } recovery code. Expires in 5 minutes.

0 commit comments

Comments
 (0)