diff --git a/shared/utils/verification.ts b/shared/utils/verification.ts index e9d280dca1..d91fd5c13c 100644 --- a/shared/utils/verification.ts +++ b/shared/utils/verification.ts @@ -5,6 +5,8 @@ export const SALT_ROUNDS = 10 export const TRANSACTION_EXPIRE_AFTER_SECONDS = 14400 // 4 hours export const HASH_EXPIRE_AFTER_SECONDS = 60 * 30 // 30 minutes export const WAIT_FOR_OTP_SECONDS = 30 +export const MAX_OTP_REQUESTS = 10 + /** * WAIT_FOR_OTP_SECONDS tolerance. Server allows OTPs to be requested every * (WAIT_FOR_OTP_SECONDS - WAIT_FOR_OTP_TOLERANCE_SECONDS) seconds. diff --git a/src/app/modules/verification/__tests__/verification.model.spec.ts b/src/app/modules/verification/__tests__/verification.model.spec.ts index 0d73b6fe73..6c57aa8ba5 100644 --- a/src/app/modules/verification/__tests__/verification.model.spec.ts +++ b/src/app/modules/verification/__tests__/verification.model.spec.ts @@ -77,6 +77,7 @@ describe('Verification Model', () => { hashedOtp: 'hashedOtp', hashCreatedAt: new Date(), hashRetries: 5, + otpRequests: 6, } const vfnParams = merge({}, VFN_PARAMS, { fields: [field] }) const verification = new VerificationModel(vfnParams) @@ -457,6 +458,7 @@ describe('Verification Model', () => { hashedOtp: 'mockHashedOtp', hashCreatedAt: new Date(), hashRetries: 3, + otpRequests: 3, } const transaction = await VerificationModel.create({ ...VFN_PARAMS, @@ -477,6 +479,7 @@ describe('Verification Model', () => { expect(result!.fields[0].hashCreatedAt).not.toEqual(field.hashCreatedAt) expect(result!.fields[0].signedData).toEqual(updateParams.signedData) expect(result!.fields[0].hashedOtp).toEqual(updateParams.hashedOtp) + expect(result!.fields[0].otpRequests).toEqual(field.otpRequests + 1) expect(result!.formId).toEqual(transaction.formId) }) @@ -488,6 +491,7 @@ describe('Verification Model', () => { hashedOtp: 'mockHashedOtp', hashCreatedAt: new Date(), hashRetries: 3, + otpRequests: 1, } const field2 = { ...generateFieldParams(), @@ -495,6 +499,7 @@ describe('Verification Model', () => { hashedOtp: 'mockHashedOtp2', hashCreatedAt: new Date(), hashRetries: 2, + otpRequests: 3, } const transaction = await VerificationModel.create({ ...VFN_PARAMS, @@ -517,12 +522,14 @@ describe('Verification Model', () => { ) expect(result!.fields[0].signedData).toEqual(updateParams.signedData) expect(result!.fields[0].hashedOtp).toEqual(updateParams.hashedOtp) + expect(result!.fields[0].otpRequests).toEqual(field1.otpRequests + 1) // field2 should remain completely unchanged expect(result!.fields[1].hashRetries).toEqual(field2.hashRetries) expect(result!.fields[1].signedData).toEqual(field2.signedData) expect(result!.fields[1].hashedOtp).toEqual(field2.hashedOtp) expect(result!.fields[1].hashCreatedAt).toEqual(field2.hashCreatedAt) + expect(result!.fields[1].otpRequests).toEqual(field2.otpRequests) expect(result!.formId).toEqual(transaction.formId) }) diff --git a/src/app/modules/verification/__tests__/verification.service.spec.ts b/src/app/modules/verification/__tests__/verification.service.spec.ts index 3e19d3d01b..c88535d0a3 100644 --- a/src/app/modules/verification/__tests__/verification.service.spec.ts +++ b/src/app/modules/verification/__tests__/verification.service.spec.ts @@ -18,7 +18,10 @@ MockLoggerModule.createLoggerWithLabel.mockReturnValue(mockLogger) import { smsConfig } from 'src/app/config/features/sms.config' import formsgSdk from 'src/app/config/formsg-sdk' import * as FormService from 'src/app/modules/form/form.service' -import { OtpRequestError } from 'src/app/modules/verification/verification.errors' +import { + OtpRequestCountExceededError, + OtpRequestError, +} from 'src/app/modules/verification/verification.errors' import { MailGenerationError, MailSendError, @@ -442,6 +445,36 @@ describe('Verification service', () => { expect(result._unsafeUnwrapErr()).toEqual(new WaitForOtpError()) }) + it('should return OtpRequestCountExceededError when OTP max requests are exceeded', async () => { + const maxExceededOtpField = generateFieldParams({ + otpRequests: 11, + }) + const maxExceededOtpTransaction = await VerificationModel.create({ + formId: mockFormId, + // Expire 1 hour in future + expireAt: addHours(new Date(), 1), + fields: [maxExceededOtpField], + }) + + const result = await VerificationService.sendNewOtp({ + transactionId: maxExceededOtpTransaction._id, + fieldId: maxExceededOtpField._id, + hashedOtp: MOCK_HASHED_OTP, + otp: MOCK_OTP, + recipient: MOCK_RECIPIENT, + }) + + expect(MockMailService.sendVerificationOtp).not.toHaveBeenCalled() + expect(MockSmsFactory.sendVerificationOtp).not.toHaveBeenCalled() + expect( + MockFormsgSdk.verification.generateSignature, + ).not.toHaveBeenCalled() + expect(updateHashSpy).not.toHaveBeenCalled() + expect(result._unsafeUnwrapErr()).toEqual( + new OtpRequestCountExceededError(), + ) + }) + it('should forward errors returned by MailService.sendVerificationOtp', async () => { const error = new MailSendError() MockMailService.sendVerificationOtp.mockReturnValueOnce(errAsync(error)) diff --git a/src/app/modules/verification/__tests__/verification.test.helpers.ts b/src/app/modules/verification/__tests__/verification.test.helpers.ts index 968eeed489..3533d815b1 100644 --- a/src/app/modules/verification/__tests__/verification.test.helpers.ts +++ b/src/app/modules/verification/__tests__/verification.test.helpers.ts @@ -13,6 +13,7 @@ export const VFN_FIELD_DEFAULTS = { hashedOtp: null, hashCreatedAt: null, hashRetries: 0, + otpRequests: 0, } export const generateFieldParams = ( diff --git a/src/app/modules/verification/verification.errors.ts b/src/app/modules/verification/verification.errors.ts index 0061add700..137dd33078 100644 --- a/src/app/modules/verification/verification.errors.ts +++ b/src/app/modules/verification/verification.errors.ts @@ -72,6 +72,15 @@ export class WaitForOtpError extends ApplicationError { } } +/** + * Max OTP request count exceeded + */ +export class OtpRequestCountExceededError extends ApplicationError { + constructor(message = 'Max OTP request count exceeded') { + super(message) + } +} + /** * Unsupported field type for OTP verification */ diff --git a/src/app/modules/verification/verification.model.ts b/src/app/modules/verification/verification.model.ts index 4ce38ba1ed..aa08399e08 100644 --- a/src/app/modules/verification/verification.model.ts +++ b/src/app/modules/verification/verification.model.ts @@ -30,7 +30,10 @@ const VerificationFieldSchema = new Schema({ // No ttl index is applied on hashCreatedAt, as we do not want to delete the // entire document when a hash expires. hashCreatedAt: { type: Date, default: null }, + // Number of retries attempted for a given OTP hashRetries: { type: Number, default: 0 }, + // Number of OTPs requested for this field + otpRequests: { type: Number, default: 0 }, }) const compileVerificationModel = (db: Mongoose): IVerificationModel => { @@ -165,6 +168,9 @@ const compileVerificationModel = (db: Mongoose): IVerificationModel => { 'fields.$.signedData': updateData.signedData, 'fields.$.hashRetries': 0, }, + $inc: { + 'fields.$.otpRequests': 1, + }, }, { new: true, diff --git a/src/app/modules/verification/verification.service.ts b/src/app/modules/verification/verification.service.ts index 13b308dbdc..4f83cb0495 100644 --- a/src/app/modules/verification/verification.service.ts +++ b/src/app/modules/verification/verification.service.ts @@ -40,6 +40,7 @@ import { MissingHashDataError, NonVerifiedFieldTypeError, OtpExpiredError, + OtpRequestCountExceededError, OtpRequestError, OtpRetryExceededError, TransactionExpiredError, @@ -52,6 +53,7 @@ import { SendOtpParams } from './verification.types' import { hasAdminExceededFreeSmsLimit, isOtpExpired, + isOtpRequestCountExceeded, isOtpWaitTimeElapsed, isTransactionExpired, } from './verification.util' @@ -278,6 +280,7 @@ export const resetFieldForTransaction = ( * @returns err(TransactionExpiredError) when transaction is expired * @returns err(FieldNotFoundInTransactionError) when field does not exist * @returns err(WaitForOtpError) when waiting time for new OTP has not elapsed + * @returns err(OtpRequestCountExceededError) when max OTP requests has been exceeded * @returns err(MalformedParametersError) when form data to send SMS OTP cannot be retrieved * @returns err(SmsSendError) when attempt to send OTP SMS fails * @returns err(InvalidNumberError) when SMS recipient is invalid @@ -298,6 +301,7 @@ export const sendNewOtp = ({ | FieldNotFoundInTransactionError | TransactionExpiredError | WaitForOtpError + | OtpRequestCountExceededError | MalformedParametersError | SmsSendError | InvalidNumberError @@ -322,6 +326,14 @@ export const sendNewOtp = ({ return errAsync(new WaitForOtpError()) } + if (isOtpRequestCountExceeded(field.otpRequests)) { + logger.warn({ + message: 'Max OTP request count exceeded', + meta: logMeta, + }) + return errAsync(new OtpRequestCountExceededError()) + } + return sendOtpForField(transaction.formId, field, recipient, otp) }) .andThen(() => { diff --git a/src/app/modules/verification/verification.util.ts b/src/app/modules/verification/verification.util.ts index 10f0f81885..49d76cb030 100644 --- a/src/app/modules/verification/verification.util.ts +++ b/src/app/modules/verification/verification.util.ts @@ -2,6 +2,7 @@ import { StatusCodes } from 'http-status-codes' import { HASH_EXPIRE_AFTER_SECONDS, + MAX_OTP_REQUESTS, VERIFIED_FIELDTYPES, WAIT_FOR_OTP_SECONDS, WAIT_FOR_OTP_TOLERANCE_SECONDS, @@ -14,6 +15,7 @@ import { import { smsConfig } from '../../config/features/sms.config' import { createLoggerWithLabel } from '../../config/logger' import { + OtpRequestCountExceededError, OtpRequestError, SmsLimitExceededError, } from '../../modules/verification/verification.errors' @@ -124,6 +126,16 @@ export const isOtpWaitTimeElapsed = (hashCreatedAt: Date | null): boolean => { return currentDate > elapseAt } +/** + * Computes whether the number of OTPs requested has exceeded the maximum allowed. + * @param otpRequests Number of OTPs already requested + * @returns true if the number of OTPs has exceeded the maximum allowed + */ +export const isOtpRequestCountExceeded = (otpRequests: number): boolean => { + // Use >= because otpRequests is the number of OTPs previously requested + return otpRequests >= MAX_OTP_REQUESTS +} + export const mapRouteError: MapRouteError = ( error, coreErrorMsg = 'Sorry, something went wrong. Please refresh and try again.', @@ -155,6 +167,11 @@ export const mapRouteError: MapRouteError = ( errorMessage: `You must wait for ${WAIT_FOR_OTP_SECONDS} seconds between each OTP request.`, statusCode: StatusCodes.UNPROCESSABLE_ENTITY, } + case OtpRequestCountExceededError: + return { + errorMessage: `You have requested too many OTPs. Please refresh and try again.`, + statusCode: StatusCodes.BAD_REQUEST, + } case InvalidNumberError: return { errorMessage: diff --git a/src/types/verification.ts b/src/types/verification.ts index 52c851e7b6..cd2ae27fdb 100644 --- a/src/types/verification.ts +++ b/src/types/verification.ts @@ -11,6 +11,7 @@ export interface IVerificationField { hashedOtp: string | null hashCreatedAt: Date | null hashRetries: number + otpRequests: number } export interface IVerificationFieldSchema