Skip to content

Commit 794c4f0

Browse files
committed
feat(customs): Add custom rate limiting rules for sending and confirming sms
1 parent 3e3fe88 commit 794c4f0

File tree

3 files changed

+56
-3
lines changed

3 files changed

+56
-3
lines changed

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

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -9,7 +9,6 @@ import {
99
RecoveryPhoneFactory,
1010
} from '@fxa/shared/db/mysql/account';
1111
import { Test } from '@nestjs/testing';
12-
import { RecoveryPhoneFactory } from '@fxa/shared/db/mysql/account';
1312

1413
describe('RecoveryPhoneManager', () => {
1514
let recoveryPhoneManager: RecoveryPhoneManager;

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

Lines changed: 43 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -104,7 +104,7 @@ describe('RecoveryPhoneService', () => {
104104
expect(result).toBeTruthy();
105105
});
106106

107-
it('Should send new code for setup phone number ', async () => {
107+
it('Should send new code for setup phone number', async () => {
108108
mockOtpManager.generateCode.mockReturnValue(code);
109109
mockRecoveryPhoneManager.getAllUnconfirmed.mockResolvedValue([
110110
'this:is:the:code123',
@@ -384,6 +384,48 @@ describe('RecoveryPhoneService', () => {
384384
mockRecoveryPhoneManager.getConfirmedPhoneNumber.mockRejectedValue(error);
385385
expect(service.sendCode(uid)).rejects.toThrow(error);
386386
});
387+
388+
it('Should send new code for setup phone number', async () => {
389+
mockOtpManager.generateCode.mockReturnValue(code);
390+
mockRecoveryPhoneManager.getAllUnconfirmed.mockResolvedValue([
391+
'this:is:the:code123',
392+
'this:is:the:code456',
393+
]);
394+
395+
mockRecoveryPhoneManager.getConfirmedPhoneNumber.mockResolvedValueOnce({
396+
phoneNumber,
397+
});
398+
mockOtpManager.generateCode.mockResolvedValueOnce(code);
399+
mockSmsManager.sendSMS.mockResolvedValue({ status: 'success' });
400+
401+
const result = await service.sendCode(uid);
402+
403+
expect(result).toBeTruthy();
404+
expect(mockRecoveryPhoneManager.getConfirmedPhoneNumber).toBeCalledWith(
405+
uid
406+
);
407+
expect(mockRecoveryPhoneManager.storeUnconfirmed).toBeCalledWith(
408+
uid,
409+
code,
410+
phoneNumber,
411+
false
412+
);
413+
expect(mockOtpManager.generateCode).toBeCalled();
414+
expect(mockSmsManager.sendSMS).toBeCalledWith({
415+
to: phoneNumber,
416+
body: code,
417+
});
418+
419+
expect(mockRecoveryPhoneManager.removeCode).toBeCalledWith(
420+
uid,
421+
'code123'
422+
);
423+
expect(mockRecoveryPhoneManager.removeCode).toBeCalledWith(
424+
uid,
425+
'code456'
426+
);
427+
expect(mockRecoveryPhoneManager.getAllUnconfirmed).toBeCalledWith(uid);
428+
});
387429
});
388430

389431
describe('available', () => {

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

Lines changed: 13 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -292,7 +292,8 @@ export class RecoveryPhoneService {
292292
}
293293

294294
/**
295-
* Sends an totp code to a user
295+
* Sends a new totp code to a user and revokes any previous unconfirmed codes.
296+
*
296297
* @param uid Account id
297298
* @returns True if message didn't fail to send.
298299
*/
@@ -301,6 +302,17 @@ export class RecoveryPhoneService {
301302
throw new RecoveryPhoneNotEnabled();
302303
}
303304

305+
// Invalidate and remove any or all previous unconfirmed code entries
306+
const unconfirmedKeys = await this.recoveryPhoneManager.getAllUnconfirmed(
307+
uid
308+
);
309+
for (const key of unconfirmedKeys) {
310+
const oldCode = key.split(':').pop();
311+
if (oldCode) {
312+
await this.recoveryPhoneManager.removeCode(uid, oldCode);
313+
}
314+
}
315+
304316
const { phoneNumber } =
305317
await this.recoveryPhoneManager.getConfirmedPhoneNumber(uid);
306318
const code = await this.otpCode.generateCode();

0 commit comments

Comments
 (0)