diff --git a/src/app/modules/auth/__tests__/auth.controller.spec.ts b/src/app/modules/auth/__tests__/auth.controller.spec.ts index c622892805..844538b2a9 100644 --- a/src/app/modules/auth/__tests__/auth.controller.spec.ts +++ b/src/app/modules/auth/__tests__/auth.controller.spec.ts @@ -104,7 +104,7 @@ describe('auth.controller', () => { // Assert expect(mockRes.status).toBeCalledWith(expectedError.status) - expect(mockRes.json).toBeCalledWith(expectedError.message) + expect(mockRes.json).toBeCalledWith({ message: expectedError.message }) }) it('should return 500 when there is an error generating login OTP', async () => { @@ -123,9 +123,10 @@ describe('auth.controller', () => { // Assert expect(mockRes.status).toBeCalledWith(500) - expect(mockRes.json).toBeCalledWith( - 'Failed to send login OTP. Please try again later and if the problem persists, contact us.', - ) + expect(mockRes.json).toBeCalledWith({ + message: + 'Failed to send login OTP. Please try again later and if the problem persists, contact us.', + }) // Sending login OTP should not have been called. expect(MockAuthService.createLoginOtp).toHaveBeenCalledTimes(1) expect(MockMailService.sendLoginOtp).not.toHaveBeenCalled() @@ -148,9 +149,10 @@ describe('auth.controller', () => { // Assert expect(mockRes.status).toBeCalledWith(500) - expect(mockRes.json).toBeCalledWith( - 'Failed to send login OTP. Please try again later and if the problem persists, contact us.', - ) + expect(mockRes.json).toBeCalledWith({ + message: + 'Failed to send login OTP. Please try again later and if the problem persists, contact us.', + }) // Services should have been invoked. expect(MockAuthService.createLoginOtp).toHaveBeenCalledTimes(1) expect(MockMailService.sendLoginOtp).toHaveBeenCalledTimes(1) diff --git a/src/app/modules/auth/__tests__/auth.routes.spec.ts b/src/app/modules/auth/__tests__/auth.routes.spec.ts index 1d58bdf74f..e3ceaa14a3 100644 --- a/src/app/modules/auth/__tests__/auth.routes.spec.ts +++ b/src/app/modules/auth/__tests__/auth.routes.spec.ts @@ -167,9 +167,10 @@ describe('auth.routes', () => { // Assert expect(response.status).toEqual(401) - expect(response.body).toEqual( - 'This is not a whitelisted public service email domain. Please log in with your official government or government-linked email address.', - ) + expect(response.body).toEqual({ + message: + 'This is not a whitelisted public service email domain. Please log in with your official government or government-linked email address.', + }) }) it('should return 500 when error occurs whilst creating OTP', async () => { @@ -186,9 +187,10 @@ describe('auth.routes', () => { // Assert expect(createLoginOtpSpy).toHaveBeenCalled() expect(response.status).toEqual(500) - expect(response.body).toEqual( - 'Failed to send login OTP. Please try again later and if the problem persists, contact us.', - ) + expect(response.body).toEqual({ + message: + 'Failed to send login OTP. Please try again later and if the problem persists, contact us.', + }) }) it('should return 500 when error occurs whilst sending login OTP', async () => { @@ -205,9 +207,10 @@ describe('auth.routes', () => { // Assert expect(sendLoginOtpSpy).toHaveBeenCalled() expect(response.status).toEqual(500) - expect(response.body).toEqual( - 'Failed to send login OTP. Please try again later and if the problem persists, contact us.', - ) + expect(response.body).toEqual({ + message: + 'Failed to send login OTP. Please try again later and if the problem persists, contact us.', + }) }) it('should return 500 when validating domain returns a database error', async () => { @@ -224,9 +227,10 @@ describe('auth.routes', () => { // Assert expect(getAgencySpy).toBeCalled() expect(response.status).toEqual(500) - expect(response.body).toEqual( - 'Failed to send login OTP. Please try again later and if the problem persists, contact us.', - ) + expect(response.body).toEqual({ + message: + 'Failed to send login OTP. Please try again later and if the problem persists, contact us.', + }) }) it('should return 200 when otp is sent successfully', async () => { diff --git a/src/app/modules/auth/auth.controller.ts b/src/app/modules/auth/auth.controller.ts index cc1ba0da1a..19b4dc231e 100644 --- a/src/app/modules/auth/auth.controller.ts +++ b/src/app/modules/auth/auth.controller.ts @@ -54,7 +54,7 @@ export const handleCheckUser: RequestHandler< */ export const handleLoginSendOtp: RequestHandler< ParamsDictionary, - string, + { message: string } | string, { email: string } > = async (req, res) => { // Joi validation ensures existence. @@ -99,7 +99,7 @@ export const handleLoginSendOtp: RequestHandler< error, /* coreErrorMessage=*/ 'Failed to send login OTP. Please try again later and if the problem persists, contact us.', ) - return res.status(statusCode).json(errorMessage) + return res.status(statusCode).json({ message: errorMessage }) }) ) } diff --git a/src/app/utils/__tests__/limit-rate.spec.ts b/src/app/utils/__tests__/limit-rate.spec.ts index 77e04a8a7f..97b53f38a4 100644 --- a/src/app/utils/__tests__/limit-rate.spec.ts +++ b/src/app/utils/__tests__/limit-rate.spec.ts @@ -30,15 +30,17 @@ describe('limitRate', () => { ) }) - it('should call next() in the handler', () => { - limitRate() + it('should return 429 when the rate limit is exceeded', () => { + limitRate({ max: 0 }) const handler = MockRateLimit.mock.calls[0][0]!.handler! const mockNext = jest.fn() - handler( - expressHandler.mockRequest(), - expressHandler.mockResponse(), - mockNext, - ) - expect(mockNext).toHaveBeenCalled() + const mockRes = expressHandler.mockResponse() + handler(expressHandler.mockRequest(), mockRes, mockNext) + expect(mockNext).not.toHaveBeenCalled() + expect(mockRes.status).toHaveBeenCalledWith(429) + expect(mockRes.json).toHaveBeenCalledWith({ + message: + 'We are experiencing a temporary issue. Please try again in one minute.', + }) }) }) diff --git a/src/app/utils/limit-rate.ts b/src/app/utils/limit-rate.ts index 7d584797df..073b45a863 100644 --- a/src/app/utils/limit-rate.ts +++ b/src/app/utils/limit-rate.ts @@ -2,6 +2,7 @@ import RateLimit, { Options as RateLimitOptions, RateLimit as RateLimitFn, } from 'express-rate-limit' +import { StatusCodes } from 'http-status-codes' import { merge } from 'lodash' import { createLoggerWithLabel } from '../../config/logger' @@ -21,7 +22,7 @@ export const limitRate = (options: RateLimitOptions = {}): RateLimitFn => { const defaultOptions: RateLimitOptions = { windowMs: 60 * 1000, // Apply rate per-minute max: 1200, - handler: (req, _res, next) => { + handler: (req, res) => { logger.warn({ message: 'Rate limit exceeded', meta: { @@ -31,8 +32,10 @@ export const limitRate = (options: RateLimitOptions = {}): RateLimitFn => { rateLimitInfo: req.rateLimit, }, }) - // TODO (private #49): terminate the request with HTTP 429 - return next() + return res.status(StatusCodes.TOO_MANY_REQUESTS).json({ + message: + 'We are experiencing a temporary issue. Please try again in one minute.', + }) }, } return RateLimit(merge(defaultOptions, options)) diff --git a/src/config/schema.ts b/src/config/schema.ts index f7953860aa..c1c7cba7f2 100644 --- a/src/config/schema.ts +++ b/src/config/schema.ts @@ -268,9 +268,10 @@ export const optionalVarsSchema: Schema = { }, rateLimit: { submissions: { - doc: 'Per-minute, per-IP request limit for submissions endpoints', + doc: + 'Per-minute, per-IP, per-instance request limit for submissions endpoints', format: 'int', - default: 200, + default: 80, env: 'SUBMISSIONS_RATE_LIMIT', }, sendAuthOtp: { diff --git a/src/public/modules/users/controllers/authentication.client.controller.js b/src/public/modules/users/controllers/authentication.client.controller.js index 5fde795d4d..c35a825202 100755 --- a/src/public/modules/users/controllers/authentication.client.controller.js +++ b/src/public/modules/users/controllers/authentication.client.controller.js @@ -180,11 +180,14 @@ function AuthenticationController($state, $timeout, $window, Auth, GTag) { function (error) { vm.isOtpSending = false vm.buttonClicked = false - // Configure message to be show + // Configure message to be shown + const msg = + (error && error.data && error.data.message) || + 'Failed to send login OTP. Please try again later and if the problem persists, contact us.' vm.signInMsg = { isMsg: true, isError: true, - msg: error.data, + msg, } $timeout(function () { angular.element('#otp-input').focus()