From 20b30cd5380471ff8fbea6f2d7240bbda8185373 Mon Sep 17 00:00:00 2001 From: LinHuiqing Date: Fri, 15 Sep 2023 09:30:44 +0800 Subject: [PATCH] refactor: use getEnabledFlags for /submissions/storage - realised that getFeatureFlag is not suitable, repeated db calls --- .../encrypt-submission.controller.spec.ts | 1 + .../encrypt-submission.controller.ts | 2 +- .../encrypt-submission.middleware.ts | 125 ++++++++---------- .../public-forms.submissions.routes.spec.ts | 13 +- 4 files changed, 66 insertions(+), 75 deletions(-) diff --git a/src/app/modules/submission/encrypt-submission/__tests__/encrypt-submission.controller.spec.ts b/src/app/modules/submission/encrypt-submission/__tests__/encrypt-submission.controller.spec.ts index 7dea30b7ed..be2e789b08 100644 --- a/src/app/modules/submission/encrypt-submission/__tests__/encrypt-submission.controller.spec.ts +++ b/src/app/modules/submission/encrypt-submission/__tests__/encrypt-submission.controller.spec.ts @@ -919,6 +919,7 @@ describe('encrypt-submission.controller', () => { expect(mockRes.send).toHaveBeenCalledWith(MOCK_PRESIGNED_URLS) }) }) + describe('streamEncryptedResponses', () => { const MOCK_USER_ID = new ObjectId().toHexString() const MOCK_FORM_ID = new ObjectId().toHexString() diff --git a/src/app/modules/submission/encrypt-submission/encrypt-submission.controller.ts b/src/app/modules/submission/encrypt-submission/encrypt-submission.controller.ts index 6ff2d57f87..1b170c8c75 100644 --- a/src/app/modules/submission/encrypt-submission/encrypt-submission.controller.ts +++ b/src/app/modules/submission/encrypt-submission/encrypt-submission.controller.ts @@ -1001,7 +1001,7 @@ export const getS3PresignedPostData: ControllerHandler< AttachmentSizeMapType[] > = async (req, res) => { const logMeta = { - action: 'handleGetS3PresignedPostData', + action: 'getS3PresignedPostData', ...createReqMeta(req), } diff --git a/src/app/modules/submission/encrypt-submission/encrypt-submission.middleware.ts b/src/app/modules/submission/encrypt-submission/encrypt-submission.middleware.ts index 928af527b5..7cf3f57c48 100644 --- a/src/app/modules/submission/encrypt-submission/encrypt-submission.middleware.ts +++ b/src/app/modules/submission/encrypt-submission/encrypt-submission.middleware.ts @@ -158,32 +158,22 @@ export const checkNewBoundaryEnabled = async ( ...createReqMeta(req), } - return FeatureFlagService.getFeatureFlag(featureFlags.encryptionBoundaryShift) - .map((newBoundaryEnabled) => { - if (!newBoundaryEnabled) { - logger.warn({ - message: 'Encryption boundary shift is not enabled.', - meta: logMeta, - }) - - return res.status(StatusCodes.FORBIDDEN).json({ - message: 'This endpoint has not been enabled for this form.', - }) - } + const newBoundaryEnabled = req.formsg.featureFlags.includes( + featureFlags.encryptionBoundaryShift, + ) - return next() - }) - .mapErr((error) => { - logger.error({ - message: 'Error retrieving feature flags.', - meta: logMeta, - error, - }) - const { statusCode, errorMessage } = mapRouteError(error) - return res.status(statusCode).send({ - message: errorMessage, - }) + if (!newBoundaryEnabled) { + logger.warn({ + message: 'Encryption boundary shift is not enabled.', + meta: logMeta, }) + + return res + .status(StatusCodes.FORBIDDEN) + .json({ message: 'This endpoint has not been enabled for this form.' }) + } + + return next() } /** @@ -228,46 +218,32 @@ export const validateStorageSubmission = async ( req.formsg.filteredResponses = responses return next() }) - // TODO(FRM-1318): Set DB flag to true to harden submission validation after validation has similar error rates as email mode forms. - .mapErr((error) => - FeatureFlagService.getFeatureFlag( - featureFlags.encryptionBoundaryShiftHardValidation, - ) - .map((hardValidationEnabled) => { - if (hardValidationEnabled) { - logger.error({ - message: 'Error processing responses', - meta: logMeta, - error, - }) - const { statusCode, errorMessage } = mapRouteError(error) - return res.status(statusCode).json({ - message: errorMessage, - }) - } - - logger.warn({ - message: - 'Error processing responses, but proceeding with submission as submission have been validated client-side', - meta: logMeta, - error, - }) - - req.formsg.filteredResponses = req.body.responses - return next() + .mapErr((error) => { + // TODO(FRM-1318): Set DB flag to true to harden submission validation after validation has similar error rates as email mode forms. + if ( + req.formsg.featureFlags.includes( + featureFlags.encryptionBoundaryShiftHardValidation, + ) + ) { + logger.error({ + message: 'Error processing responses', + meta: logMeta, + error, }) - .mapErr((error) => { - logger.error({ - message: 'Error retrieving feature flags.', - meta: logMeta, - error, - }) - const { statusCode, errorMessage } = mapRouteError(error) - return res.status(statusCode).send({ - message: errorMessage, - }) - }), - ) + const { statusCode, errorMessage } = mapRouteError(error) + return res.status(statusCode).json({ + message: errorMessage, + }) + } + logger.warn({ + message: + 'Error processing responses, but proceeding with submission as submission have been validated client-side', + meta: logMeta, + error, + }) + req.formsg.filteredResponses = req.body.responses + return next() + }) } const encryptAttachment = async ( @@ -464,7 +440,20 @@ export const createFormsgAndRetrieveForm = async ( if (req.formsg) return res.send(new FormsgReqBodyExistsError()) const formsg = {} as FormLoadedDto - // Step 2a: Retrieve form + // Step 2: Retrieve feature flags + const featureFlagsListResult = await FeatureFlagService.getEnabledFlags() + + if (featureFlagsListResult.isErr()) { + logger.error({ + message: 'Error occurred whilst retrieving enabled feature flags', + meta: logMeta, + error: featureFlagsListResult.error, + }) + } else { + formsg.featureFlags = featureFlagsListResult.value + } + + // Step 3a: Retrieve form const formResult = await FormService.retrieveFullFormById(formId) if (formResult.isErr()) { logger.warn({ @@ -476,11 +465,11 @@ export const createFormsgAndRetrieveForm = async ( return res.status(statusCode).json({ message: errorMessage }) } - // Step 2b: Set formsg.formDef in req.body + // Step 3b: Set formsg.formDef in req.body const formDef = formResult.value formsg.formDef = formDef - // Step 3a: Check if form is encrypt mode + // Step 4a: Check if form is encrypt mode const checkFormIsEncryptModeResult = checkFormIsEncryptMode(formDef) if (checkFormIsEncryptModeResult.isErr()) { logger.error({ @@ -496,10 +485,10 @@ export const createFormsgAndRetrieveForm = async ( }) } - // Step 3b: Set formsg.encryptedFormDef in req.body + // Step 4b: Set formsg.encryptedFormDef in req.body formsg.encryptedFormDef = checkFormIsEncryptModeResult.value - // Step 4: Check if form has public key + // Step 5: Check if form has public key if (!formDef.publicKey) { logger.warn({ message: 'Form does not have a public key', diff --git a/src/app/routes/api/v3/forms/__tests__/public-forms.submissions.routes.spec.ts b/src/app/routes/api/v3/forms/__tests__/public-forms.submissions.routes.spec.ts index 036b904540..1b93b1d32f 100644 --- a/src/app/routes/api/v3/forms/__tests__/public-forms.submissions.routes.spec.ts +++ b/src/app/routes/api/v3/forms/__tests__/public-forms.submissions.routes.spec.ts @@ -4,6 +4,7 @@ import jwt from 'jsonwebtoken' import { omit } from 'lodash' import mongoose from 'mongoose' import { errAsync, okAsync } from 'neverthrow' +import { featureFlags } from 'shared/constants' import session, { Session } from 'supertest-session' import { aws } from 'src/app/config/config' @@ -1509,8 +1510,8 @@ describe('public-form.submissions.routes', () => { describe('Joi validation', () => { beforeEach(() => { jest - .spyOn(FeatureFlagsService, 'getFeatureFlag') - .mockReturnValue(okAsync(true)) + .spyOn(FeatureFlagsService, 'getEnabledFlags') + .mockReturnValue(okAsync([featureFlags.encryptionBoundaryShift])) }) it('should return 403 when feature flag has not been enabled', async () => { @@ -1524,8 +1525,8 @@ describe('public-form.submissions.routes', () => { }) jest - .spyOn(FeatureFlagsService, 'getFeatureFlag') - .mockReturnValueOnce(okAsync(false)) + .spyOn(FeatureFlagsService, 'getEnabledFlags') + .mockReturnValueOnce(okAsync([])) // Act const response = await request @@ -1945,8 +1946,8 @@ describe('public-form.submissions.routes', () => { } beforeEach(() => { jest - .spyOn(FeatureFlagsService, 'getFeatureFlag') - .mockReturnValue(okAsync(true)) + .spyOn(FeatureFlagsService, 'getEnabledFlags') + .mockReturnValue(okAsync([featureFlags.encryptionBoundaryShift])) }) describe('SingPass', () => {