diff --git a/README.md b/README.md index bd5efb8e73..6ac6e23c99 100755 --- a/README.md +++ b/README.md @@ -63,7 +63,7 @@ npm run dev After the Docker image has finished building, the application can be accessed at [localhost:5000](localhost:5000). -If there have been no dependency changes in `package.json` or changes in the +If there are no dependency changes in `package.json` or changes in the `src/app/server.ts` file, you can run ```bash @@ -75,7 +75,7 @@ only takes ~15 seconds to finish starting up the image. ### Accessing email locally -We use [MailDev](https://github.com/maildev/maildev) to access emails in the development environment. The MailDev UI can be accessed at [localhost:1080](localhost:1080) when the Docker container is running. +We use [MailDev](https://github.com/maildev/maildev) to access emails in the development environment. The MailDev UI can be accessed at [localhost:1080](localhost:1080) when the Docker container runs. ### Environment variables @@ -87,8 +87,8 @@ The following is the order of priority: - Environment file - Dockerfile -FormSG requires some environment variables in order to function. -More information about the required environment variables can be seen in +FormSG requires some environment variables to function. +More information about the required environment variables are in [DEPLOYMENT_SETUP.md](/docs/DEPLOYMENT_SETUP.md). We provide a [`.template-env`](./.template-env) file with the secrets blanked out. You can copy and @@ -159,7 +159,7 @@ npm run test-e2e-ci ## Architecture -An overview of the architecture can be found [here](docs/ARCHITECTURE.md). +The architecture overview is [here](docs/ARCHITECTURE.md). ## MongoDB Scripts @@ -167,7 +167,7 @@ Scripts for common tasks in MongoDB can be found [here](docs/MONGODB.md). ## Contributing -We welcome all contributions, bug reports, bug fixes, documentation improvements, enhancements, and ideas to code open sourced by the Government Technology Agency of Singapore. Contributors should read [CONTRIBUTING.md](CONTRIBUTING.md) and will also be asked to sign a Contributor License Agreement (CLA) in order to ensure that everybody is free to use their contributions. +We welcome all contributions, bug reports, bug fixes, documentation improvements, enhancements, and ideas to code open sourced by the Government Technology Agency of Singapore. Contributors should read [CONTRIBUTING.md](CONTRIBUTING.md) and will also be asked to sign a Contributor License Agreement (CLA) to ensure that everybody is free to use their contributions. ## Support diff --git a/src/app/modules/auth/__tests__/auth.controller.spec.ts b/src/app/modules/auth/__tests__/auth.controller.spec.ts index f5dd798968..a142751310 100644 --- a/src/app/modules/auth/__tests__/auth.controller.spec.ts +++ b/src/app/modules/auth/__tests__/auth.controller.spec.ts @@ -42,7 +42,7 @@ describe('auth.controller', () => { ) // Act - await AuthController.handleCheckUser(MOCK_REQ, mockRes, jest.fn()) + await AuthController._handleCheckUser(MOCK_REQ, mockRes, jest.fn()) // Assert expect(mockRes.sendStatus).toBeCalledWith(200) @@ -57,7 +57,7 @@ describe('auth.controller', () => { ) // Act - await AuthController.handleCheckUser(MOCK_REQ, mockRes, jest.fn()) + await AuthController._handleCheckUser(MOCK_REQ, mockRes, jest.fn()) // Assert expect(mockRes.status).toBeCalledWith(401) @@ -82,7 +82,7 @@ describe('auth.controller', () => { MockMailService.sendLoginOtp.mockReturnValueOnce(okAsync(true)) // Act - await AuthController.handleLoginSendOtp(MOCK_REQ, mockRes, jest.fn()) + await AuthController._handleLoginSendOtp(MOCK_REQ, mockRes, jest.fn()) // Assert expect(mockRes.status).toBeCalledWith(200) @@ -101,7 +101,7 @@ describe('auth.controller', () => { ) // Act - await AuthController.handleLoginSendOtp(MOCK_REQ, mockRes, jest.fn()) + await AuthController._handleLoginSendOtp(MOCK_REQ, mockRes, jest.fn()) // Assert expect(mockRes.status).toBeCalledWith(401) @@ -120,7 +120,7 @@ describe('auth.controller', () => { ) // Act - await AuthController.handleLoginSendOtp(MOCK_REQ, mockRes, jest.fn()) + await AuthController._handleLoginSendOtp(MOCK_REQ, mockRes, jest.fn()) // Assert expect(mockRes.status).toBeCalledWith(500) @@ -146,7 +146,7 @@ describe('auth.controller', () => { ) // Act - await AuthController.handleLoginSendOtp(MOCK_REQ, mockRes, jest.fn()) + await AuthController._handleLoginSendOtp(MOCK_REQ, mockRes, jest.fn()) // Assert expect(mockRes.status).toBeCalledWith(500) @@ -183,7 +183,7 @@ describe('auth.controller', () => { MockUserService.retrieveUser.mockReturnValueOnce(okAsync(mockUser)) // Act - await AuthController.handleLoginVerifyOtp(MOCK_REQ, mockRes, jest.fn()) + await AuthController._handleLoginVerifyOtp(MOCK_REQ, mockRes, jest.fn()) // Assert expect(mockRes.status).toBeCalledWith(200) @@ -199,7 +199,7 @@ describe('auth.controller', () => { ) // Act - await AuthController.handleLoginVerifyOtp(MOCK_REQ, mockRes, jest.fn()) + await AuthController._handleLoginVerifyOtp(MOCK_REQ, mockRes, jest.fn()) // Assert expect(mockRes.status).toBeCalledWith(401) @@ -219,7 +219,7 @@ describe('auth.controller', () => { ) // Act - await AuthController.handleLoginVerifyOtp(MOCK_REQ, mockRes, jest.fn()) + await AuthController._handleLoginVerifyOtp(MOCK_REQ, mockRes, jest.fn()) // Assert expect(mockRes.status).toBeCalledWith(422) @@ -241,7 +241,7 @@ describe('auth.controller', () => { ) // Act - await AuthController.handleLoginVerifyOtp(MOCK_REQ, mockRes, jest.fn()) + await AuthController._handleLoginVerifyOtp(MOCK_REQ, mockRes, jest.fn()) // Assert expect(mockRes.status).toBeCalledWith(500) @@ -265,7 +265,7 @@ describe('auth.controller', () => { ) // Act - await AuthController.handleLoginVerifyOtp(MOCK_REQ, mockRes, jest.fn()) + await AuthController._handleLoginVerifyOtp(MOCK_REQ, mockRes, jest.fn()) // Assert expect(mockRes.status).toBeCalledWith(500) diff --git a/src/app/modules/auth/auth.controller.ts b/src/app/modules/auth/auth.controller.ts index 5d280baeba..188c8f3524 100644 --- a/src/app/modules/auth/auth.controller.ts +++ b/src/app/modules/auth/auth.controller.ts @@ -8,19 +8,18 @@ import { createReqMeta, getRequestIp } from '../../utils/request' import { ControllerHandler } from '../core/core.types' import * as UserService from '../user/user.service' +import { + validateCheckUserParams, + validateLoginSendOtpParams, + validateVerifyOtpParams, +} from './auth.middlewares' import * as AuthService from './auth.service' import { SessionUser } from './auth.types' import { mapRouteError } from './auth.utils' const logger = createLoggerWithLabel(module) -/** - * Handler for GET /auth/checkuser endpoint. - * @returns 500 when there was an error validating body.email - * @returns 401 when domain of body.email is invalid - * @returns 200 if domain of body.email is valid - */ -export const handleCheckUser: ControllerHandler< +export const _handleCheckUser: ControllerHandler< unknown, string, { email: string } @@ -46,12 +45,17 @@ export const handleCheckUser: ControllerHandler< } /** - * Handler for POST /auth/sendotp endpoint. - * @return 200 when OTP has been been successfully sent - * @return 401 when email domain is invalid - * @return 500 when unknown errors occurs during generate OTP, or create/send the email that delivers the OTP to the user's email address + * Handler for GET /auth/checkuser endpoint. + * @returns 500 when there was an error validating body.email + * @returns 401 when domain of body.email is invalid + * @returns 200 if domain of body.email is valid */ -export const handleLoginSendOtp: ControllerHandler< +export const handleCheckUser = [ + validateCheckUserParams, + _handleCheckUser, +] as ControllerHandler[] + +export const _handleLoginSendOtp: ControllerHandler< unknown, { message: string } | string, { email: string } @@ -103,6 +107,17 @@ export const handleLoginSendOtp: ControllerHandler< ) } +/** + * Handler for POST /auth/sendotp endpoint. + * @return 200 when OTP has been been successfully sent + * @return 401 when email domain is invalid + * @return 500 when unknown errors occurs during generate OTP, or create/send the email that delivers the OTP to the user's email address + */ +export const handleLoginSendOtp = [ + validateLoginSendOtpParams, + _handleLoginSendOtp, +] as ControllerHandler[] + /** * Handler for POST /auth/verifyotp endpoint. * @returns 200 when user has successfully logged in, with session cookie set @@ -110,7 +125,7 @@ export const handleLoginSendOtp: ControllerHandler< * @returns 422 when the OTP is invalid * @returns 500 when error occurred whilst verifying the OTP */ -export const handleLoginVerifyOtp: ControllerHandler< +export const _handleLoginVerifyOtp: ControllerHandler< unknown, string | SessionUser, { email: string; otp: string } @@ -185,6 +200,11 @@ export const handleLoginVerifyOtp: ControllerHandler< ) } +export const handleLoginVerifyOtp = [ + validateVerifyOtpParams, + _handleLoginVerifyOtp, +] as ControllerHandler[] + export const handleSignout: ControllerHandler = async (req, res) => { if (!req.session || isEmpty(req.session)) { logger.error({ diff --git a/src/app/modules/auth/auth.middlewares.ts b/src/app/modules/auth/auth.middlewares.ts index d61ad462aa..af232735b1 100644 --- a/src/app/modules/auth/auth.middlewares.ts +++ b/src/app/modules/auth/auth.middlewares.ts @@ -1,3 +1,4 @@ +import { celebrate, Joi, Segments } from 'celebrate' import { AuthedSessionData } from 'express-session' import { StatusCodes } from 'http-status-codes' @@ -57,3 +58,37 @@ export const logAdminAction: ControllerHandler<{ formId: string }> = async ( return next() } + +export const validateCheckUserParams = celebrate({ + [Segments.BODY]: Joi.object().keys({ + email: Joi.string() + .required() + .email() + .message('Please enter a valid email') + .lowercase(), + }), +}) + +export const validateLoginSendOtpParams = celebrate({ + [Segments.BODY]: Joi.object().keys({ + email: Joi.string() + .required() + .email() + .message('Please enter a valid email') + .lowercase(), + }), +}) + +export const validateVerifyOtpParams = celebrate({ + [Segments.BODY]: Joi.object().keys({ + email: Joi.string() + .required() + .email() + .message('Please enter a valid email') + .lowercase(), + otp: Joi.string() + .required() + .regex(/^\d{6}$/) + .message('Please enter a valid OTP'), + }), +}) diff --git a/src/app/modules/auth/auth.routes.ts b/src/app/modules/auth/auth.routes.ts index b47a532017..ffa645bbc3 100644 --- a/src/app/modules/auth/auth.routes.ts +++ b/src/app/modules/auth/auth.routes.ts @@ -1,4 +1,3 @@ -import { celebrate, Joi, Segments } from 'celebrate' import { Router } from 'express' import { rateLimitConfig } from '../../config/config' @@ -16,19 +15,7 @@ export const AuthRouter = Router() * @return 200 when email domain is valid * @return 401 when email domain is invalid */ -AuthRouter.post( - '/checkuser', - celebrate({ - [Segments.BODY]: Joi.object().keys({ - email: Joi.string() - .required() - .email() - .message('Please enter a valid email') - .lowercase(), - }), - }), - AuthController.handleCheckUser, -) +AuthRouter.post('/checkuser', AuthController.handleCheckUser) /** * Send a one-time password (OTP) to the specified email address @@ -45,15 +32,6 @@ AuthRouter.post( AuthRouter.post( '/sendotp', limitRate({ max: rateLimitConfig.sendAuthOtp }), - celebrate({ - [Segments.BODY]: Joi.object().keys({ - email: Joi.string() - .required() - .email() - .message('Please enter a valid email') - .lowercase(), - }), - }), AuthController.handleLoginSendOtp, ) @@ -70,23 +48,7 @@ AuthRouter.post( * @returns 422 when the OTP is invalid * @returns 500 when error occurred whilst verifying the OTP */ -AuthRouter.post( - '/verifyotp', - celebrate({ - [Segments.BODY]: Joi.object().keys({ - email: Joi.string() - .required() - .email() - .message('Please enter a valid email') - .lowercase(), - otp: Joi.string() - .required() - .regex(/^\d{6}$/) - .message('Please enter a valid OTP'), - }), - }), - AuthController.handleLoginVerifyOtp, -) +AuthRouter.post('/verifyotp', AuthController.handleLoginVerifyOtp) /** * Sign the user out of the session by clearing the relevant session cookie diff --git a/src/app/modules/billing/__tests__/billing.controller.spec.ts b/src/app/modules/billing/__tests__/billing.controller.spec.ts index dd11890b61..4dc500bb57 100644 --- a/src/app/modules/billing/__tests__/billing.controller.spec.ts +++ b/src/app/modules/billing/__tests__/billing.controller.spec.ts @@ -68,7 +68,7 @@ describe('billing.controller', () => { ) // Act - await BillingController.handleGetBillInfo(MOCK_REQ, mockRes, jest.fn()) + await BillingController._handleGetBillInfo(MOCK_REQ, mockRes, jest.fn()) // Assert expect(MockBillingService.getSpLoginStats).toHaveBeenCalledWith( @@ -86,7 +86,7 @@ describe('billing.controller', () => { ) // Act - await BillingController.handleGetBillInfo(MOCK_REQ, mockRes, jest.fn()) + await BillingController._handleGetBillInfo(MOCK_REQ, mockRes, jest.fn()) // Assert expect(MockBillingService.getSpLoginStats).toHaveBeenCalledWith( diff --git a/src/app/modules/billing/billing.controller.ts b/src/app/modules/billing/billing.controller.ts index 71f4af4fea..ade5f6bdcf 100644 --- a/src/app/modules/billing/billing.controller.ts +++ b/src/app/modules/billing/billing.controller.ts @@ -11,19 +11,12 @@ import { createLoggerWithLabel } from '../../config/logger' import { createReqMeta } from '../../utils/request' import { ControllerHandler } from '../core/core.types' +import { validateGetBillingInfoParams } from './billing.middlewares' import * as BillingService from './billing.service' const logger = createLoggerWithLabel(module) -/** - * Handler for GET /billing endpoint. - * @security session - * - * @return 200 with login statistics when query is valid - * @return 401 when request does not contain a user session - * @return 500 when error occurs whilst querying database - */ -export const handleGetBillInfo: ControllerHandler< +export const _handleGetBillInfo: ControllerHandler< unknown, ErrorDto | BillingInfoDto, unknown, @@ -72,3 +65,16 @@ export const handleGetBillInfo: ControllerHandler< loginStats: loginStatsResult.value, }) } + +/** + * Handler for GET /billing endpoint. + * @security session + * + * @return 200 with login statistics when query is valid + * @return 401 when request does not contain a user session + * @return 500 when error occurs whilst querying database + */ +export const handleGetBillInfo = [ + validateGetBillingInfoParams, + _handleGetBillInfo, +] as ControllerHandler[] diff --git a/src/app/modules/billing/billing.middlewares.ts b/src/app/modules/billing/billing.middlewares.ts new file mode 100644 index 0000000000..76c1e742c0 --- /dev/null +++ b/src/app/modules/billing/billing.middlewares.ts @@ -0,0 +1,9 @@ +import { celebrate, Joi, Segments } from 'celebrate' + +export const validateGetBillingInfoParams = celebrate({ + [Segments.QUERY]: Joi.object({ + esrvcId: Joi.string().required(), + yr: Joi.number().integer().min(2019).required(), + mth: Joi.number().integer().min(0).max(11).required(), + }), +}) diff --git a/src/app/modules/billing/billing.routes.ts b/src/app/modules/billing/billing.routes.ts index ca53f8b546..afa71877c2 100644 --- a/src/app/modules/billing/billing.routes.ts +++ b/src/app/modules/billing/billing.routes.ts @@ -1,4 +1,3 @@ -import { celebrate, Joi, Segments } from 'celebrate' import { Router } from 'express' import { withUserAuthentication } from '../auth/auth.middlewares' @@ -22,14 +21,4 @@ BillingRouter.use(withUserAuthentication) * @return 401 when request does not contain a user session * @return 500 when error occurs whilst querying database */ -BillingRouter.get( - '/', - celebrate({ - [Segments.QUERY]: Joi.object({ - esrvcId: Joi.string().required(), - yr: Joi.number().integer().min(2019).required(), - mth: Joi.number().integer().min(0).max(11).required(), - }), - }), - BillingController.handleGetBillInfo, -) +BillingRouter.get('/', BillingController.handleGetBillInfo) diff --git a/src/app/modules/examples/__tests__/examples.controller.spec.ts b/src/app/modules/examples/__tests__/examples.controller.spec.ts index dba66283d4..e7ea870910 100644 --- a/src/app/modules/examples/__tests__/examples.controller.spec.ts +++ b/src/app/modules/examples/__tests__/examples.controller.spec.ts @@ -42,7 +42,7 @@ describe('examples.controller', () => { ) // Act - await ExamplesController.handleGetExamples(MOCK_REQ, mockRes, jest.fn()) + await ExamplesController._handleGetExamples(MOCK_REQ, mockRes, jest.fn()) // Assert expect(MockExamplesService.getExampleForms).toHaveBeenCalledWith( @@ -61,7 +61,7 @@ describe('examples.controller', () => { ) // Act - await ExamplesController.handleGetExamples(MOCK_REQ, mockRes, jest.fn()) + await ExamplesController._handleGetExamples(MOCK_REQ, mockRes, jest.fn()) // Assert expect(MockExamplesService.getExampleForms).toHaveBeenCalledWith( diff --git a/src/app/modules/examples/examples.controller.ts b/src/app/modules/examples/examples.controller.ts index 533b1cdceb..43407637bd 100644 --- a/src/app/modules/examples/examples.controller.ts +++ b/src/app/modules/examples/examples.controller.ts @@ -10,6 +10,7 @@ import { createLoggerWithLabel } from '../../config/logger' import { createReqMeta } from '../../utils/request' import { ControllerHandler } from '../core/core.types' +import { validateGetExamplesParams } from './examples.middlewares' import * as ExamplesService from './examples.service' import { mapRouteError } from './examples.utils' @@ -23,7 +24,7 @@ const logger = createLoggerWithLabel(module) * @returns 401 when user does not exist in session * @returns 500 when error occurs whilst querying the database */ -export const handleGetExamples: ControllerHandler< +export const _handleGetExamples: ControllerHandler< unknown, ErrorDto | ExampleFormsResult, unknown, @@ -46,6 +47,11 @@ export const handleGetExamples: ControllerHandler< }) } +export const handleGetExamples = [ + validateGetExamplesParams, + _handleGetExamples, +] as ControllerHandler[] + /** * Handler for GET /examples/:formId endpoint. * @security session diff --git a/src/app/modules/examples/examples.middlewares.ts b/src/app/modules/examples/examples.middlewares.ts new file mode 100644 index 0000000000..4df50b44fb --- /dev/null +++ b/src/app/modules/examples/examples.middlewares.ts @@ -0,0 +1,12 @@ +import { celebrate, Joi, Segments } from 'celebrate' + +export const validateGetExamplesParams = celebrate({ + [Segments.QUERY]: Joi.object().keys({ + pageNo: Joi.number().min(0).required(), + agency: Joi.string() + .regex(/^[0-9a-fA-F]{24}$/) + .allow(''), + searchTerm: Joi.string().allow(''), + shouldGetTotalNumResults: Joi.boolean().default(false), + }), +}) diff --git a/src/app/modules/examples/examples.routes.ts b/src/app/modules/examples/examples.routes.ts index 611a1547b9..313cff72e6 100644 --- a/src/app/modules/examples/examples.routes.ts +++ b/src/app/modules/examples/examples.routes.ts @@ -1,4 +1,3 @@ -import { celebrate, Joi, Segments } from 'celebrate' import { Router } from 'express' import { withUserAuthentication } from '../auth/auth.middlewares' @@ -22,20 +21,7 @@ ExamplesRouter.use(withUserAuthentication) * @returns 401 when user does not exist in session * @returns 500 when error occurs whilst querying the database */ -ExamplesRouter.get( - '/', - celebrate({ - [Segments.QUERY]: Joi.object().keys({ - pageNo: Joi.number().min(0).required(), - agency: Joi.string() - .regex(/^[0-9a-fA-F]{24}$/) - .allow(''), - searchTerm: Joi.string().allow(''), - shouldGetTotalNumResults: Joi.boolean().default(false), - }), - }), - ExamplesController.handleGetExamples, -) +ExamplesRouter.get('/', ExamplesController.handleGetExamples) /** * Returns example information for the form that is referenced by the given diff --git a/src/app/modules/form/admin-form/__tests__/admin-form.controller.spec.ts b/src/app/modules/form/admin-form/__tests__/admin-form.controller.spec.ts index e922e14019..6ba7072fe1 100644 --- a/src/app/modules/form/admin-form/__tests__/admin-form.controller.spec.ts +++ b/src/app/modules/form/admin-form/__tests__/admin-form.controller.spec.ts @@ -4525,7 +4525,7 @@ describe('admin-form.controller', () => { ) // Act - await AdminFormController.handleUpdateSettings( + await AdminFormController._handleUpdateSettings( MOCK_REQ, mockRes, jest.fn(), @@ -4564,7 +4564,7 @@ describe('admin-form.controller', () => { ) // Act - await AdminFormController.handleUpdateSettings( + await AdminFormController._handleUpdateSettings( MOCK_REQ, mockRes, jest.fn(), @@ -4598,7 +4598,7 @@ describe('admin-form.controller', () => { ) // Act - await AdminFormController.handleUpdateSettings( + await AdminFormController._handleUpdateSettings( MOCK_REQ, mockRes, jest.fn(), @@ -4636,7 +4636,7 @@ describe('admin-form.controller', () => { ) // Act - await AdminFormController.handleUpdateSettings( + await AdminFormController._handleUpdateSettings( MOCK_REQ, mockRes, jest.fn(), @@ -4673,7 +4673,7 @@ describe('admin-form.controller', () => { ) // Act - await AdminFormController.handleUpdateSettings( + await AdminFormController._handleUpdateSettings( MOCK_REQ, mockRes, jest.fn(), @@ -4711,7 +4711,7 @@ describe('admin-form.controller', () => { ) // Act - await AdminFormController.handleUpdateSettings( + await AdminFormController._handleUpdateSettings( MOCK_REQ, mockRes, jest.fn(), @@ -4752,7 +4752,7 @@ describe('admin-form.controller', () => { ) // Act - await AdminFormController.handleUpdateSettings( + await AdminFormController._handleUpdateSettings( MOCK_REQ, mockRes, jest.fn(), @@ -4786,7 +4786,7 @@ describe('admin-form.controller', () => { ) // Act - await AdminFormController.handleUpdateSettings( + await AdminFormController._handleUpdateSettings( MOCK_REQ, mockRes, jest.fn(), @@ -4820,7 +4820,7 @@ describe('admin-form.controller', () => { ) // Act - await AdminFormController.handleUpdateSettings( + await AdminFormController._handleUpdateSettings( MOCK_REQ, mockRes, jest.fn(), diff --git a/src/app/modules/form/admin-form/admin-form.controller.ts b/src/app/modules/form/admin-form/admin-form.controller.ts index 35222f0f19..024154f7c5 100644 --- a/src/app/modules/form/admin-form/admin-form.controller.ts +++ b/src/app/modules/form/admin-form/admin-form.controller.ts @@ -88,6 +88,7 @@ import { PREVIEW_SINGPASS_UINFIN, } from './admin-form.constants' import { EditFieldError } from './admin-form.errors' +import { updateSettingsValidator } from './admin-form.middlewares' import * as AdminFormService from './admin-form.service' import { PermissionLevel } from './admin-form.types' import { mapRouteError } from './admin-form.utils' @@ -1247,22 +1248,7 @@ export const handleDuplicateFormField: ControllerHandler< }) } -/** - * Handler for PATCH /forms/:formId/settings. - * @security session - * - * @returns 200 with updated form settings - * @returns 400 when body is malformed; can happen when email parameter is passed for encrypt-mode forms - * @returns 403 when current user does not have permissions to update form settings - * @returns 404 when form to update settings for cannot be found - * @returns 409 when saving form settings incurs a conflict in the database - * @returns 410 when updating settings for archived form - * @returns 413 when updating settings causes form to be too large to be saved in the database - * @returns 422 when an invalid settings update is attempted on the form - * @returns 422 when user in session cannot be retrieved from the database - * @returns 500 when database error occurs - */ -export const handleUpdateSettings: ControllerHandler< +export const _handleUpdateSettings: ControllerHandler< { formId: string }, FormSettings | ErrorDto, SettingsUpdateDto @@ -1302,6 +1288,26 @@ export const handleUpdateSettings: ControllerHandler< }) } +/** + * Handler for PATCH /forms/:formId/settings. + * @security session + * + * @returns 200 with updated form settings + * @returns 400 when body is malformed; can happen when email parameter is passed for encrypt-mode forms + * @returns 403 when current user does not have permissions to update form settings + * @returns 404 when form to update settings for cannot be found + * @returns 409 when saving form settings incurs a conflict in the database + * @returns 410 when updating settings for archived form + * @returns 413 when updating settings causes form to be too large to be saved in the database + * @returns 422 when an invalid settings update is attempted on the form + * @returns 422 when user in session cannot be retrieved from the database + * @returns 500 when database error occurs + */ +export const handleUpdateSettings = [ + updateSettingsValidator, + _handleUpdateSettings, +] as ControllerHandler[] + /** * NOTE: Exported for testing. * Private handler for PUT /forms/:formId/fields/:fieldId diff --git a/src/app/modules/form/admin-form/admin-form.middlewares.ts b/src/app/modules/form/admin-form/admin-form.middlewares.ts new file mode 100644 index 0000000000..320b73b76c --- /dev/null +++ b/src/app/modules/form/admin-form/admin-form.middlewares.ts @@ -0,0 +1,30 @@ +import { celebrate, Joi, Segments } from 'celebrate' + +import { + FormAuthType, + FormStatus, + SettingsUpdateDto, +} from '../../../../../shared/types' + +/** + * Joi validator for PATCH /forms/:formId/settings route. + */ +export const updateSettingsValidator = celebrate({ + [Segments.BODY]: Joi.object({ + authType: Joi.string().valid(...Object.values(FormAuthType)), + emails: Joi.alternatives().try( + Joi.array().items(Joi.string().email()), + Joi.string().email({ multiple: true }), + ), + esrvcId: Joi.string().allow(''), + hasCaptcha: Joi.boolean(), + inactiveMessage: Joi.string(), + status: Joi.string().valid(...Object.values(FormStatus)), + submissionLimit: Joi.number().allow(null), + title: Joi.string(), + webhook: Joi.object({ + url: Joi.string().uri().allow(''), + isRetryEnabled: Joi.boolean(), + }).min(1), + }).min(1), +}) diff --git a/src/app/modules/frontend/__tests__/frontend.controller.spec.ts b/src/app/modules/frontend/__tests__/frontend.controller.spec.ts index da32eb2d63..a80076dc32 100644 --- a/src/app/modules/frontend/__tests__/frontend.controller.spec.ts +++ b/src/app/modules/frontend/__tests__/frontend.controller.spec.ts @@ -77,7 +77,7 @@ describe('frontend.server.controller', () => { 'window.location.hash = "#!/formId?fieldId1=abc&fieldId2=<>'"' // Note this is different from mockReqModified.query.redirectPath as // there are html-encoded characters - FrontendServerController.generateRedirectUrl( + FrontendServerController._generateRedirectUrl( mockReqModified, mockRes, jest.fn(), @@ -89,7 +89,7 @@ describe('frontend.server.controller', () => { expect(mockRes.status).toHaveBeenCalledWith(StatusCodes.OK) }) it('should return BAD_REQUEST if the request is not valid', () => { - FrontendServerController.generateRedirectUrl( + FrontendServerController._generateRedirectUrl( // @ts-ignore mockBadReq, mockRes, diff --git a/src/app/modules/frontend/frontend.controller.ts b/src/app/modules/frontend/frontend.controller.ts index 3268e9552d..7af36f345c 100644 --- a/src/app/modules/frontend/frontend.controller.ts +++ b/src/app/modules/frontend/frontend.controller.ts @@ -5,6 +5,8 @@ import { createLoggerWithLabel } from '../../config/logger' import { createReqMeta } from '../../utils/request' import { ControllerHandler } from '../core/core.types' +import { validateGenerateRedirectParams } from './frontend.middlewares' + const logger = createLoggerWithLabel(module) /** @@ -81,7 +83,7 @@ export const addEnvVarData: ControllerHandler = ( * @param res - Express response object * @returns Templated Javascript code for the frontend that redirects to specific form url */ -export const generateRedirectUrl: ControllerHandler< +export const _generateRedirectUrl: ControllerHandler< unknown, string | { message: string }, unknown, @@ -115,6 +117,11 @@ export const generateRedirectUrl: ControllerHandler< } } +export const generateRedirectUrl = [ + validateGenerateRedirectParams, + _generateRedirectUrl, +] as ControllerHandler[] + // Duplicated here since the feature manager is being deprecated. // TODO (#2147): delete this. enum FeatureNames { diff --git a/src/app/modules/frontend/frontend.middlewares.ts b/src/app/modules/frontend/frontend.middlewares.ts new file mode 100644 index 0000000000..f1a7699724 --- /dev/null +++ b/src/app/modules/frontend/frontend.middlewares.ts @@ -0,0 +1,9 @@ +import { celebrate, Joi, Segments } from 'celebrate' + +export const validateGenerateRedirectParams = celebrate({ + [Segments.QUERY]: { + redirectPath: Joi.string() + .regex(/^[a-fA-F0-9]{24}(\/(preview|template|use-template))?/) + .required(), + }, +}) diff --git a/src/app/modules/frontend/frontend.routes.ts b/src/app/modules/frontend/frontend.routes.ts index 0fc0bdfddd..d0f0879220 100644 --- a/src/app/modules/frontend/frontend.routes.ts +++ b/src/app/modules/frontend/frontend.routes.ts @@ -1,4 +1,3 @@ -import { celebrate, Joi, Segments } from 'celebrate' import { Router } from 'express' import * as FrontendServerController from './frontend.controller' @@ -40,14 +39,4 @@ FrontendRouter.get('/features', FrontendServerController.showFeaturesStates) * @return 200 when redirect code is successful * @return 400 when redirect code fails */ -FrontendRouter.get( - '/redirect', - celebrate({ - [Segments.QUERY]: { - redirectPath: Joi.string() - .regex(/^[a-fA-F0-9]{24}(\/(preview|template|use-template))?/) - .required(), - }, - }), - FrontendServerController.generateRedirectUrl, -) +FrontendRouter.get('/redirect', FrontendServerController.generateRedirectUrl) diff --git a/src/app/modules/user/__tests__/user.controller.spec.ts b/src/app/modules/user/__tests__/user.controller.spec.ts index e081c0bf6a..038486391f 100644 --- a/src/app/modules/user/__tests__/user.controller.spec.ts +++ b/src/app/modules/user/__tests__/user.controller.spec.ts @@ -51,7 +51,7 @@ describe('user.controller', () => { MockSmsFactory.sendAdminContactOtp.mockReturnValueOnce(okAsync(true)) // Act - await UserController.handleContactSendOtp(MOCK_REQ, mockRes, jest.fn()) + await UserController._handleContactSendOtp(MOCK_REQ, mockRes, jest.fn()) // Assert // Check passed in params. @@ -78,7 +78,7 @@ describe('user.controller', () => { const mockRes = expressHandler.mockResponse() // Act - await UserController.handleContactSendOtp( + await UserController._handleContactSendOtp( reqWithoutSession, mockRes, jest.fn(), @@ -109,7 +109,7 @@ describe('user.controller', () => { const mockRes = expressHandler.mockResponse() // Act - await UserController.handleContactSendOtp( + await UserController._handleContactSendOtp( reqWithDiffUserParam, mockRes, jest.fn(), @@ -137,7 +137,7 @@ describe('user.controller', () => { ) // Act - await UserController.handleContactSendOtp(MOCK_REQ, mockRes, jest.fn()) + await UserController._handleContactSendOtp(MOCK_REQ, mockRes, jest.fn()) // Assert expect(mockRes.status).toBeCalledWith(422) @@ -158,7 +158,7 @@ describe('user.controller', () => { ) // Act - await UserController.handleContactSendOtp(MOCK_REQ, mockRes, jest.fn()) + await UserController._handleContactSendOtp(MOCK_REQ, mockRes, jest.fn()) // Assert expect(mockRes.status).toBeCalledWith(500) @@ -200,7 +200,7 @@ describe('user.controller', () => { ) // Act - await UserController.handleContactVerifyOtp(MOCK_REQ, mockRes, jest.fn()) + await UserController._handleContactVerifyOtp(MOCK_REQ, mockRes, jest.fn()) // Assert // Expect services to be called with correct arguments. @@ -229,7 +229,7 @@ describe('user.controller', () => { const mockRes = expressHandler.mockResponse() // Act - await UserController.handleContactVerifyOtp( + await UserController._handleContactVerifyOtp( reqWithoutSession, mockRes, jest.fn(), @@ -261,7 +261,7 @@ describe('user.controller', () => { const mockRes = expressHandler.mockResponse() // Act - await UserController.handleContactVerifyOtp( + await UserController._handleContactVerifyOtp( reqWithDiffUserParam, mockRes, jest.fn(), @@ -289,7 +289,7 @@ describe('user.controller', () => { ) // Act - await UserController.handleContactVerifyOtp(MOCK_REQ, mockRes, jest.fn()) + await UserController._handleContactVerifyOtp(MOCK_REQ, mockRes, jest.fn()) // Assert expect(mockRes.status).toBeCalledWith(500) @@ -309,7 +309,7 @@ describe('user.controller', () => { ) // Act - await UserController.handleContactVerifyOtp(MOCK_REQ, mockRes, jest.fn()) + await UserController._handleContactVerifyOtp(MOCK_REQ, mockRes, jest.fn()) // Assert expect(mockRes.status).toBeCalledWith(401) @@ -329,7 +329,7 @@ describe('user.controller', () => { ) // Act - await UserController.handleContactVerifyOtp(MOCK_REQ, mockRes, jest.fn()) + await UserController._handleContactVerifyOtp(MOCK_REQ, mockRes, jest.fn()) // Assert expect(mockRes.status).toBeCalledWith(422) @@ -349,7 +349,7 @@ describe('user.controller', () => { ) // Act - await UserController.handleContactVerifyOtp(MOCK_REQ, mockRes, jest.fn()) + await UserController._handleContactVerifyOtp(MOCK_REQ, mockRes, jest.fn()) // Assert expect(mockRes.status).toBeCalledWith(500) diff --git a/src/app/modules/user/user.controller.ts b/src/app/modules/user/user.controller.ts index a598ee7747..8920fe906d 100644 --- a/src/app/modules/user/user.controller.ts +++ b/src/app/modules/user/user.controller.ts @@ -7,6 +7,10 @@ import { getRequestIp } from '../../utils/request' import { getUserIdFromSession } from '../auth/auth.utils' import { ControllerHandler } from '../core/core.types' +import { + validateContactOtpVerificationParams, + validateContactSendOtpParams, +} from './user.middleware' import { createContactOtp, getPopulatedUserById, @@ -17,16 +21,7 @@ import { mapRouteError } from './user.utils' const logger = createLoggerWithLabel(module) -/** - * Generates an OTP and sends the OTP to the given contact in request body. - * @route POST /contact/otp/generate - * @returns 200 if OTP was successfully sent - * @returns 401 if user id does not match current session user or if user is not currently logged in - * @returns 422 on OTP creation or SMS send failure - * @returns 422 if user id does not exist in the database - * @returns 500 if database errors occurs - */ -export const handleContactSendOtp: ControllerHandler< +export const _handleContactSendOtp: ControllerHandler< unknown, string, { contact: string; userId: string } @@ -92,15 +87,20 @@ export const handleContactSendOtp: ControllerHandler< } /** - * Verifies given OTP with the hashed OTP data, and updates the user's contact - * number if the hash matches. - * @route POST /contact/otp/verify - * @returns 200 when user contact update success + * Generates an OTP and sends the OTP to the given contact in request body. + * @route POST /contact/otp/generate + * @returns 200 if OTP was successfully sent * @returns 401 if user id does not match current session user or if user is not currently logged in - * @returns 422 when OTP is invalid - * @returns 500 when OTP is malformed or for unknown errors + * @returns 422 on OTP creation or SMS send failure + * @returns 422 if user id does not exist in the database + * @returns 500 if database errors occurs */ -export const handleContactVerifyOtp: ControllerHandler< +export const handleContactSendOtp = [ + validateContactSendOtpParams, + _handleContactSendOtp, +] as ControllerHandler[] + +export const _handleContactVerifyOtp: ControllerHandler< unknown, string | IPopulatedUser, { @@ -158,6 +158,20 @@ export const handleContactVerifyOtp: ControllerHandler< return res.status(StatusCodes.OK).json(updateResult.value) } +/** + * Verifies given OTP with the hashed OTP data, and updates the user's contact + * number if the hash matches. + * @route POST /contact/otp/verify + * @returns 200 when user contact update success + * @returns 401 if user id does not match current session user or if user is not currently logged in + * @returns 422 when OTP is invalid + * @returns 500 when OTP is malformed or for unknown errors + */ +export const handleContactVerifyOtp = [ + validateContactOtpVerificationParams, + _handleContactVerifyOtp, +] as ControllerHandler[] + /** * Retrieves and returns the session user from the database. * @route GET / diff --git a/src/app/modules/user/user.middleware.ts b/src/app/modules/user/user.middleware.ts new file mode 100644 index 0000000000..4824d73b16 --- /dev/null +++ b/src/app/modules/user/user.middleware.ts @@ -0,0 +1,24 @@ +import { celebrate, Joi, Segments } from 'celebrate' + +/** + * Celebrate validation for the contact OTP sending endpoint. + */ +export const validateContactSendOtpParams = celebrate({ + [Segments.BODY]: Joi.object().keys({ + contact: Joi.string().required(), + userId: Joi.string().required(), + }), +}) + +/** + * Celebrate validation for the contact OTP verification endpoint. + */ +export const validateContactOtpVerificationParams = celebrate({ + [Segments.BODY]: Joi.object({ + userId: Joi.string().required(), + otp: Joi.string() + .required() + .regex(/^\d{6}$/), + contact: Joi.string().required(), + }), +}) diff --git a/src/app/modules/user/user.routes.ts b/src/app/modules/user/user.routes.ts index 7566344d1b..6dc5cb5188 100644 --- a/src/app/modules/user/user.routes.ts +++ b/src/app/modules/user/user.routes.ts @@ -1,4 +1,3 @@ -import { celebrate, Joi, Segments } from 'celebrate' import { Router } from 'express' import * as UserController from './user.controller' @@ -28,16 +27,7 @@ UserRouter.get('/', UserController.handleFetchUser) * @returns 422 on OTP creation or SMS send failure, or if the user cannot be found * @returns 500 on application or database errors */ -UserRouter.post( - '/contact/sendotp', - celebrate({ - [Segments.BODY]: Joi.object().keys({ - contact: Joi.string().required(), - userId: Joi.string().required(), - }), - }), - UserController.handleContactSendOtp, -) +UserRouter.post('/contact/sendotp', UserController.handleContactSendOtp) /** * Verify the contact verification one-time password (OTP) for the user as part @@ -50,18 +40,6 @@ UserRouter.post( * @returns 422 when OTP is invalid * @returns 500 when OTP is malformed or for unknown errors */ -UserRouter.post( - '/contact/verifyotp', - celebrate({ - [Segments.BODY]: Joi.object().keys({ - userId: Joi.string().required(), - otp: Joi.string() - .required() - .regex(/^\d{6}$/), - contact: Joi.string().required(), - }), - }), - UserController.handleContactVerifyOtp, -) +UserRouter.post('/contact/verifyotp', UserController.handleContactVerifyOtp) export default UserRouter diff --git a/src/app/routes/api/v3/admin/forms/admin-forms.settings.routes.ts b/src/app/routes/api/v3/admin/forms/admin-forms.settings.routes.ts index 40acc921a0..0bfcfbfab3 100644 --- a/src/app/routes/api/v3/admin/forms/admin-forms.settings.routes.ts +++ b/src/app/routes/api/v3/admin/forms/admin-forms.settings.routes.ts @@ -1,38 +1,9 @@ -import { celebrate, Joi, Segments } from 'celebrate' import { Router } from 'express' -import { - FormAuthType, - FormStatus, - SettingsUpdateDto, -} from '../../../../../../../shared/types' import * as AdminFormController from '../../../../../modules/form/admin-form/admin-form.controller' export const AdminFormsSettingsRouter = Router() -/** - * Joi validator for PATCH /forms/:formId/settings route. - */ -const updateSettingsValidator = celebrate({ - [Segments.BODY]: Joi.object({ - authType: Joi.string().valid(...Object.values(FormAuthType)), - emails: Joi.alternatives().try( - Joi.array().items(Joi.string().email()), - Joi.string().email({ multiple: true }), - ), - esrvcId: Joi.string().allow(''), - hasCaptcha: Joi.boolean(), - inactiveMessage: Joi.string(), - status: Joi.string().valid(...Object.values(FormStatus)), - submissionLimit: Joi.number().allow(null), - title: Joi.string(), - webhook: Joi.object({ - url: Joi.string().uri().allow(''), - isRetryEnabled: Joi.boolean(), - }).min(1), - }).min(1), -}) - AdminFormsSettingsRouter.route('/:formId([a-fA-F0-9]{24})/settings') /** * Update form settings according to given subset of settings. @@ -53,7 +24,7 @@ AdminFormsSettingsRouter.route('/:formId([a-fA-F0-9]{24})/settings') * @returns 422 when user in session cannot be retrieved from the database * @returns 500 when database error occurs */ - .patch(updateSettingsValidator, AdminFormController.handleUpdateSettings) + .patch(AdminFormController.handleUpdateSettings) /** * Retrieve the settings of the specified form * @route GET /admin/forms/:formId/settings diff --git a/src/app/routes/api/v3/auth/auth.routes.ts b/src/app/routes/api/v3/auth/auth.routes.ts index 1bf1f9ecac..f34f29b2b7 100644 --- a/src/app/routes/api/v3/auth/auth.routes.ts +++ b/src/app/routes/api/v3/auth/auth.routes.ts @@ -1,4 +1,3 @@ -import { celebrate, Joi, Segments } from 'celebrate' import { Router } from 'express' import { rateLimitConfig } from '../../../../config/config' @@ -14,19 +13,7 @@ export const AuthRouter = Router() * @return 200 when email domain is valid * @return 401 when email domain is invalid */ -AuthRouter.post( - '/email/validate', - celebrate({ - [Segments.BODY]: Joi.object().keys({ - email: Joi.string() - .required() - .email() - .message('Please enter a valid email') - .lowercase(), - }), - }), - AuthController.handleCheckUser, -) +AuthRouter.post('/email/validate', AuthController.handleCheckUser) /** * Send a one-time password (OTP) to the specified email address @@ -43,15 +30,6 @@ AuthRouter.post( AuthRouter.post( '/otp/generate', limitRate({ max: rateLimitConfig.sendAuthOtp }), - celebrate({ - [Segments.BODY]: Joi.object().keys({ - email: Joi.string() - .required() - .email() - .message('Please enter a valid email') - .lowercase(), - }), - }), AuthController.handleLoginSendOtp, ) @@ -68,23 +46,7 @@ AuthRouter.post( * @returns 422 when the OTP is invalid * @returns 500 when error occurred whilst verifying the OTP */ -AuthRouter.post( - '/otp/verify', - celebrate({ - [Segments.BODY]: Joi.object().keys({ - email: Joi.string() - .required() - .email() - .message('Please enter a valid email') - .lowercase(), - otp: Joi.string() - .required() - .regex(/^\d{6}$/) - .message('Please enter a valid OTP'), - }), - }), - AuthController.handleLoginVerifyOtp, -) +AuthRouter.post('/otp/verify', AuthController.handleLoginVerifyOtp) /** * Sign the user out of the session by clearing the relevant session cookie diff --git a/src/app/routes/api/v3/billings/billings.routes.ts b/src/app/routes/api/v3/billings/billings.routes.ts index 91f3b2d5f2..6185a0d151 100644 --- a/src/app/routes/api/v3/billings/billings.routes.ts +++ b/src/app/routes/api/v3/billings/billings.routes.ts @@ -1,4 +1,3 @@ -import { celebrate, Joi, Segments } from 'celebrate' import { Router } from 'express' import { withUserAuthentication } from '../../../../modules/auth/auth.middlewares' @@ -21,14 +20,4 @@ BillingsRouter.use(withUserAuthentication) * @return 401 when request does not contain a user session * @return 500 when error occurs whilst querying database */ -BillingsRouter.get( - '/', - celebrate({ - [Segments.QUERY]: Joi.object({ - esrvcId: Joi.string().required(), - yr: Joi.number().integer().min(2019).required(), - mth: Joi.number().integer().min(0).max(11).required(), - }), - }), - BillingController.handleGetBillInfo, -) +BillingsRouter.get('/', BillingController.handleGetBillInfo) diff --git a/src/app/routes/api/v3/client/client.routes.ts b/src/app/routes/api/v3/client/client.routes.ts index 521506d91a..6d0bf5830c 100644 --- a/src/app/routes/api/v3/client/client.routes.ts +++ b/src/app/routes/api/v3/client/client.routes.ts @@ -1,4 +1,3 @@ -import { celebrate, Joi, Segments } from 'celebrate' import { Router } from 'express' import * as FrontendServerController from '../../../../modules/frontend/frontend.controller' @@ -40,14 +39,4 @@ ClientRouter.get('/features', FrontendServerController.showFeaturesStates) * @return 200 when redirect code is successful * @return 400 when redirect code fails */ -ClientRouter.get( - '/redirect', - celebrate({ - [Segments.QUERY]: { - redirectPath: Joi.string() - .regex(/^[a-fA-F0-9]{24}(\/(preview|template|use-template))?/) - .required(), - }, - }), - FrontendServerController.generateRedirectUrl, -) +ClientRouter.get('/redirect', FrontendServerController.generateRedirectUrl) diff --git a/src/app/routes/api/v3/user/user.routes.ts b/src/app/routes/api/v3/user/user.routes.ts index d08165f93c..48c6392b57 100644 --- a/src/app/routes/api/v3/user/user.routes.ts +++ b/src/app/routes/api/v3/user/user.routes.ts @@ -1,4 +1,3 @@ -import { celebrate, Joi, Segments } from 'celebrate' import { Router } from 'express' import * as UserController from '../../../../modules/user/user.controller' @@ -28,16 +27,7 @@ UserRouter.get('/', UserController.handleFetchUser) * @returns 422 on OTP creation or SMS send failure, or if the user cannot be found * @returns 500 on application or database errors */ -UserRouter.post( - '/contact/otp/generate', - celebrate({ - [Segments.BODY]: Joi.object().keys({ - contact: Joi.string().required(), - userId: Joi.string().required(), - }), - }), - UserController.handleContactSendOtp, -) +UserRouter.post('/contact/otp/generate', UserController.handleContactSendOtp) /** * Verify the contact verification one-time password (OTP) for the user as part @@ -50,18 +40,6 @@ UserRouter.post( * @returns 422 when OTP is invalid * @returns 500 when OTP is malformed or for unknown errors */ -UserRouter.post( - '/contact/otp/verify', - celebrate({ - [Segments.BODY]: Joi.object().keys({ - userId: Joi.string().required(), - otp: Joi.string() - .required() - .regex(/^\d{6}$/), - contact: Joi.string().required(), - }), - }), - UserController.handleContactVerifyOtp, -) +UserRouter.post('/contact/otp/verify', UserController.handleContactVerifyOtp) export default UserRouter