From c2b1a33b90c2b06f1ce07722f7c9865a749b48c5 Mon Sep 17 00:00:00 2001 From: tshuli Date: Mon, 8 Jan 2024 22:11:36 +0800 Subject: [PATCH] chore: remove non encryption boundary fallback --- __tests__/e2e/encrypt-submission.spec.ts | 11 +- .../public-form/PublicFormProvider.tsx | 157 +++---- .../features/public-form/PublicFormService.ts | 68 +--- .../src/features/public-form/mutations.ts | 42 +- .../public-form/utils/createSubmission.ts | 1 - shared/constants/feature-flags.ts | 5 - shared/constants/form.ts | 1 - .../__tests__/submission.controller.spec.ts | 37 +- .../encrypt-submission.controller.ts | 12 - .../encrypt-submission.middleware.ts | 77 +--- .../submission/submission.controller.ts | 52 +-- .../public-forms.submissions.routes.spec.ts | 383 +----------------- .../forms/public-forms.submissions.routes.ts | 20 - 13 files changed, 95 insertions(+), 771 deletions(-) diff --git a/__tests__/e2e/encrypt-submission.spec.ts b/__tests__/e2e/encrypt-submission.spec.ts index 8b6a65fadc..13b6d0d4f3 100644 --- a/__tests__/e2e/encrypt-submission.spec.ts +++ b/__tests__/e2e/encrypt-submission.spec.ts @@ -1,5 +1,4 @@ import mongoose from 'mongoose' -import { featureFlags } from 'shared/constants/feature-flags' import { BasicField, FormAuthType, @@ -7,7 +6,7 @@ import { MyInfoAttribute, } from 'shared/types' -import { IFeatureFlagModel, IFormModel } from 'src/types' +import { IFormModel } from 'src/types' import { ALL_FIELDS as _ALL_FIELDS, @@ -43,20 +42,12 @@ const runEncryptSubmissionTest = createSubmissionTestRunnerForResponseMode( let db: mongoose.Connection let Form: IFormModel -let FeatureFlags: IFeatureFlagModel test.describe('Storage form submission', () => { test.beforeAll(async () => { // Create models db = await makeMongooseFixtures() Form = makeModel(db, 'form.server.model', 'Form') - // TODO(FRM-1232): Remove this once old storage submission endpoint (/submissions/encrypt) is removed - // Add feature flag model and set encryption boundary shift to true - FeatureFlags = makeModel(db, 'feature_flag.server.model', 'FeatureFlags') - await FeatureFlags.create({ - name: featureFlags.encryptionBoundaryShift, - enabled: true, - }) }) test.afterAll(async () => { // Clean up db diff --git a/frontend/src/features/public-form/PublicFormProvider.tsx b/frontend/src/features/public-form/PublicFormProvider.tsx index 5393d1e322..43f54fcacf 100644 --- a/frontend/src/features/public-form/PublicFormProvider.tsx +++ b/frontend/src/features/public-form/PublicFormProvider.tsx @@ -11,7 +11,7 @@ import { SubmitHandler } from 'react-hook-form' import { useNavigate } from 'react-router-dom' import { useDisclosure } from '@chakra-ui/react' import { datadogLogs } from '@datadog/browser-logs' -import { useFeatureIsOn, useGrowthBook } from '@growthbook/growthbook-react' +import { useGrowthBook } from '@growthbook/growthbook-react' import { differenceInMilliseconds, isPast } from 'date-fns' import get from 'lodash/get' import simplur from 'simplur' @@ -290,16 +290,11 @@ export const PublicFormProvider = ({ } }, [data?.form.form_fields, toast, vfnToastIdRef]) - const enableVirusScanner = useFeatureIsOn( - featureFlags.encryptionBoundaryShiftVirusScanner, - ) - const { submitEmailModeFormMutation, submitEmailModeFormFetchMutation, - submitStorageModeClearFormMutation, - submitStorageModeClearFormFetchMutation, - submitStorageModeClearFormWithVirusScanningMutation, + submitStorageModeFormMutation, + submitStorageModeFormFetchMutation, submitMultirespondentFormMutation, updateMultirespondentSubmissionMutation, } = usePublicFormMutations(formId, previousSubmissionId) @@ -514,7 +509,7 @@ export const PublicFormProvider = ({ }, }) - return submitStorageModeClearFormFetchMutation + return submitStorageModeFormFetchMutation .mutateAsync( { ...formData, @@ -570,111 +565,55 @@ export const PublicFormProvider = ({ }, }) - // TODO (FRM-1413): Move to main return statement once virus scanner has been fully rolled out - if (enableVirusScanner) { - return submitStorageModeClearFormWithVirusScanningMutation - .mutateAsync( - { - ...formData, - ...formPaymentData, - }, - { - onSuccess: ({ - submissionId, + return submitStorageModeFormMutation + .mutateAsync( + { + ...formData, + ...formPaymentData, + }, + { + onSuccess: ({ + submissionId, + timestamp, + // payment forms will have non-empty paymentData field + paymentData, + }) => { + trackSubmitForm(form) + + if (paymentData) { + navigate(getPaymentPageUrl(formId, paymentData.paymentId)) + storePaymentMemory(paymentData.paymentId) + return + } + setSubmissionData({ + id: submissionId, timestamp, - // payment forms will have non-empty paymentData field - paymentData, - }) => { - trackSubmitForm(form) - - if (paymentData) { - navigate(getPaymentPageUrl(formId, paymentData.paymentId)) - storePaymentMemory(paymentData.paymentId) - return - } - setSubmissionData({ - id: submissionId, - timestamp, - }) - }, - }, - ) - .catch(async (error) => { - // TODO(#5826): Remove when we have resolved the Network Error - datadogLogs.logger.warn( - `handleSubmitForm: submit with virus scan`, - { - meta: { - ...logMeta, - responseMode: 'storage', - method: 'axios', - error, - }, - }, - ) - - if (/Network Error/i.test(error.message)) { - axiosDebugFlow() - // defaults to the safest option of storage submission without virus scanning - return submitStorageFormWithFetch() - } else { - showErrorToast(error, form) - } - }) - } - - return ( - submitStorageModeClearFormMutation - .mutateAsync( - { - ...formData, - ...formPaymentData, + }) }, + }, + ) + .catch(async (error) => { + // TODO(#5826): Remove when we have resolved the Network Error + datadogLogs.logger.warn( + `handleSubmitForm: submit with virus scan`, { - onSuccess: ({ - submissionId, - timestamp, - // payment forms will have non-empty paymentData field - paymentData, - }) => { - trackSubmitForm(form) - - if (paymentData) { - navigate(getPaymentPageUrl(formId, paymentData.paymentId)) - storePaymentMemory(paymentData.paymentId) - return - } - setSubmissionData({ - id: submissionId, - timestamp, - }) - }, - }, - ) - // Using catch since we are using mutateAsync and react-hook-form will continue bubbling this up. - .catch(async (error) => { - // TODO(#5826): Remove when we have resolved the Network Error - datadogLogs.logger.warn(`handleSubmitForm: ${error.message}`, { meta: { ...logMeta, responseMode: 'storage', method: 'axios', - error: { - message: error.message, - stack: error.stack, - }, + error, }, - }) + }, + ) + + if (/Network Error/i.test(error.message)) { axiosDebugFlow() - if (/Network Error/i.test(error.message)) { - axiosDebugFlow() - // defaults to the safest option of storage submission without virus scanning - return submitStorageFormWithFetch() - } else { - showErrorToast(error, form) - } - }) - ) + // defaults to the safest option of storage submission without virus scanning + return submitStorageFormWithFetch() + } else { + showErrorToast(error, form) + } + }) } case FormResponseMode.Multirespondent: return ( @@ -704,17 +643,15 @@ export const PublicFormProvider = ({ showErrorToast, getCaptchaResponse, previousSubmissionId, - submitMultirespondentFormMutation, updateMultirespondentSubmissionMutation, + submitMultirespondentFormMutation, submitEmailModeFormFetchMutation, submitEmailModeFormMutation, - enableVirusScanner, - submitStorageModeClearFormMutation, - submitStorageModeClearFormFetchMutation, + submitStorageModeFormMutation, + submitStorageModeFormFetchMutation, navigate, formId, storePaymentMemory, - submitStorageModeClearFormWithVirusScanningMutation, ], ) diff --git a/frontend/src/features/public-form/PublicFormService.ts b/frontend/src/features/public-form/PublicFormService.ts index 0320cbfef7..7128dd8b9b 100644 --- a/frontend/src/features/public-form/PublicFormService.ts +++ b/frontend/src/features/public-form/PublicFormService.ts @@ -2,7 +2,6 @@ import { PresignedPost } from 'aws-sdk/clients/s3' import axios from 'axios' import { - ENCRYPTION_BOUNDARY_SHIFT_SUBMISSION_VERSION, MULTIRESPONDENT_FORM_SUBMISSION_VERSION, VIRUS_SCANNER_SUBMISSION_VERSION, } from '~shared/constants' @@ -181,7 +180,9 @@ export const submitEmailModeForm = async ({ ).then(({ data }) => data) } -export const submitStorageModeClearForm = async ({ +// TODO (#5826): Fallback mutation using Fetch. Remove once network error is resolved +// Submit storage mode form with virus scanning (storage v2.1+) +export const submitStorageModeFormWithFetch = async ({ formFields, formLogics, formInputs, @@ -192,63 +193,26 @@ export const submitStorageModeClearForm = async ({ responseMetadata, paymentProducts, payments, -}: SubmitStorageFormClearArgs) => { + fieldIdToQuarantineKeyMap, +}: SubmitStorageFormWithVirusScanningArgs) => { const filteredInputs = filterHiddenInputs({ formFields, formInputs, formLogics, }) - const formData = createClearSubmissionFormData({ - formFields, - formInputs: filteredInputs, - responseMetadata, - paymentReceiptEmail, - paymentProducts, - payments, - version: ENCRYPTION_BOUNDARY_SHIFT_SUBMISSION_VERSION, - }) - - return ApiService.post( - `${PUBLIC_FORMS_ENDPOINT}/${formId}/submissions/storage`, - formData, + const formData = createClearSubmissionWithVirusScanningFormData( { - params: { - captchaResponse: String(captchaResponse), - captchaType: captchaType, - }, + formFields, + formInputs: filteredInputs, + responseMetadata, + paymentReceiptEmail, + paymentProducts, + payments, + version: VIRUS_SCANNER_SUBMISSION_VERSION, }, - ).then(({ data }) => data) -} - -// TODO (#5826): Fallback mutation using Fetch. Remove once network error is resolved -export const submitStorageModeClearFormWithFetch = async ({ - formFields, - formLogics, - formInputs, - formId, - captchaResponse = null, - captchaType = '', - paymentReceiptEmail, - responseMetadata, - paymentProducts, - payments, -}: SubmitStorageFormClearArgs) => { - const filteredInputs = filterHiddenInputs({ - formFields, - formInputs, - formLogics, - }) - - const formData = createClearSubmissionFormData({ - formFields, - formInputs: filteredInputs, - responseMetadata, - paymentReceiptEmail, - paymentProducts, - payments, - version: ENCRYPTION_BOUNDARY_SHIFT_SUBMISSION_VERSION, - }) + fieldIdToQuarantineKeyMap, + ) // Add captcha response to query string const queryString = new URLSearchParams({ @@ -271,7 +235,7 @@ export const submitStorageModeClearFormWithFetch = async ({ } // Submit storage mode form with virus scanning (storage v2.1+) -export const submitStorageModeClearFormWithVirusScanning = async ({ +export const submitStorageModeForm = async ({ formFields, formLogics, formInputs, diff --git a/frontend/src/features/public-form/mutations.ts b/frontend/src/features/public-form/mutations.ts index 7565516b13..4a21068ca3 100644 --- a/frontend/src/features/public-form/mutations.ts +++ b/frontend/src/features/public-form/mutations.ts @@ -23,9 +23,8 @@ import { submitFormIssue, submitMultirespondentForm, SubmitStorageFormClearArgs, - submitStorageModeClearForm, - submitStorageModeClearFormWithFetch, - submitStorageModeClearFormWithVirusScanning, + submitStorageModeForm, + submitStorageModeFormWithFetch, updateMultirespondentSubmission, uploadAttachmentToQuarantine, } from './PublicFormService' @@ -80,12 +79,6 @@ export const usePublicFormMutations = ( }, ) - const submitStorageModeClearFormMutation = useMutation( - (args: Omit) => { - return submitStorageModeClearForm({ ...args, formId }) - }, - ) - // TODO (#5826): Fallback mutation using Fetch. Remove once network error is resolved const submitEmailModeFormFetchMutation = useMutation( (args: Omit) => { @@ -93,13 +86,7 @@ export const usePublicFormMutations = ( }, ) - const submitStorageModeClearFormFetchMutation = useMutation( - (args: Omit) => { - return submitStorageModeClearFormWithFetch({ ...args, formId }) - }, - ) - - const useSubmitClearFormWithVirusScanningMutation = ( + const useSubmitStorageModeFormMutation = ( f: ( args: SubmitStorageFormClearArgs & { fieldIdToQuarantineKeyMap: FieldIdToQuarantineKeyType[] @@ -176,23 +163,26 @@ export const usePublicFormMutations = ( ) }) - const submitStorageModeClearFormWithVirusScanningMutation = - useSubmitClearFormWithVirusScanningMutation( - submitStorageModeClearFormWithVirusScanning, - ) + const submitStorageModeFormMutation = useSubmitStorageModeFormMutation( + submitStorageModeForm, + ) - const submitMultirespondentFormMutation = - useSubmitClearFormWithVirusScanningMutation(submitMultirespondentForm) + const submitStorageModeFormFetchMutation = useSubmitStorageModeFormMutation( + submitStorageModeFormWithFetch, + ) + + const submitMultirespondentFormMutation = useSubmitStorageModeFormMutation( + submitMultirespondentForm, + ) const updateMultirespondentSubmissionMutation = - useSubmitClearFormWithVirusScanningMutation(updateMultirespondentSubmission) + useSubmitStorageModeFormMutation(updateMultirespondentSubmission) return { submitEmailModeFormMutation, submitEmailModeFormFetchMutation, - submitStorageModeClearFormMutation, - submitStorageModeClearFormFetchMutation, - submitStorageModeClearFormWithVirusScanningMutation, + submitStorageModeFormMutation, + submitStorageModeFormFetchMutation, submitMultirespondentFormMutation, updateMultirespondentSubmissionMutation, } diff --git a/frontend/src/features/public-form/utils/createSubmission.ts b/frontend/src/features/public-form/utils/createSubmission.ts index 80a7a0153e..506d6309aa 100644 --- a/frontend/src/features/public-form/utils/createSubmission.ts +++ b/frontend/src/features/public-form/utils/createSubmission.ts @@ -364,7 +364,6 @@ export const getAttachmentsMap = ( return attachmentsMap } -// TODO (FRM-1232): Remove once encryption boundary has been shifted. /** * Utility to filter out responses that should be sent to the server. This includes: * 1. Email fields that have an autoreply enabled. diff --git a/shared/constants/feature-flags.ts b/shared/constants/feature-flags.ts index 8c92165902..8cd072c212 100644 --- a/shared/constants/feature-flags.ts +++ b/shared/constants/feature-flags.ts @@ -3,11 +3,6 @@ export const featureFlags = { goLinks: 'goLinks' as const, turnstile: 'turnstile' as const, validateStripeEmailDomain: 'validateStripeEmailDomain' as const, - encryptionBoundaryShift: 'encryption-boundary-shift' as const, - encryptionBoundaryShiftHardValidation: - 'encryption-boundary-shift-hard-validation' as const, - encryptionBoundaryShiftVirusScanner: - 'encryption-boundary-shift-virus-scanner' as const, myinfoSgid: 'myinfo-sgid' as const, chartsMaxResponseCount: 'charts-max-response-count' as const, } diff --git a/shared/constants/form.ts b/shared/constants/form.ts index 6fedb83657..43470aa801 100644 --- a/shared/constants/form.ts +++ b/shared/constants/form.ts @@ -78,7 +78,6 @@ export const PAYMENT_VARIABLE_INPUT_AMOUNT_FIELD_ID = export const E2EE_SUBMISSION_VERSION = 1 // Encryption boundary shift RFC: https://docs.google.com/document/d/1VmNXS_xYY2Yg30AwVqzdndBp5yRJGSDsyjBnH51ktyc/edit?usp=sharing // Encryption boundary shift implementation PR: https://github.com/opengovsg/FormSG/pull/6587 -export const ENCRYPTION_BOUNDARY_SHIFT_SUBMISSION_VERSION = 2 export const VIRUS_SCANNER_SUBMISSION_VERSION = 2.1 // MRF RFC: https://www.notion.so/opengov/RFC-Multi-respondent-forms-8ab40a8c17674937b345450d9dd2c81d?pvs=4 export const MULTIRESPONDENT_FORM_SUBMISSION_VERSION = 3 diff --git a/src/app/modules/submission/__tests__/submission.controller.spec.ts b/src/app/modules/submission/__tests__/submission.controller.spec.ts index 600774c03f..9d8d6a835b 100644 --- a/src/app/modules/submission/__tests__/submission.controller.spec.ts +++ b/src/app/modules/submission/__tests__/submission.controller.spec.ts @@ -14,7 +14,6 @@ import { import * as AuthService from 'src/app/modules/auth/auth.service' import { DatabaseError } from 'src/app/modules/core/core.errors' -import * as FeatureFlagService from 'src/app/modules/feature-flags/feature-flags.service' import { PermissionLevel } from 'src/app/modules/form/admin-form/admin-form.types' import { ForbiddenFormError, @@ -58,7 +57,6 @@ const MockSubService = jest.mocked(SubmissionService) const MockEncryptSubService = jest.mocked(EncryptSubmissionService) const MockUserService = jest.mocked(UserService) const MockAuthService = jest.mocked(AuthService) -const MockFeatureFlagService = jest.mocked(FeatureFlagService) describe('submission.controller', () => { beforeEach(() => jest.clearAllMocks()) @@ -1069,37 +1067,8 @@ describe('submission.controller', () => { ] as unknown as AttachmentSizeMapType[], }) - it('should return 500 if getFeatureFlag returns errAsync(DatabaseError)', async () => { + it('should return 500 if getQuarantinePresignedPostData returns errAsync(CreatePresignedPostError)', async () => { // Arrange - MockFeatureFlagService.getFeatureFlag.mockReturnValueOnce( - errAsync(new DatabaseError()), - ) - const mockRes = expressHandler.mockResponse() - - // Act - await getS3PresignedPostData(MOCK_REQ, mockRes, jest.fn()) - - // Assert - expect(mockRes.status).toHaveBeenCalledWith( - StatusCodes.INTERNAL_SERVER_ERROR, - ) - }) - - it('should return 400 if getFeatureFlag returns okAsync(false)', async () => { - // Arrange - MockFeatureFlagService.getFeatureFlag.mockReturnValueOnce(okAsync(false)) - const mockRes = expressHandler.mockResponse() - - // Act - await getS3PresignedPostData(MOCK_REQ, mockRes, jest.fn()) - - // Assert - expect(mockRes.status).toHaveBeenCalledWith(StatusCodes.FORBIDDEN) - }) - - it('should return 500 if getFeatureFlag returns okAsync(true) but getQuarantinePresignedPostData returns errAsync(CreatePresignedPostError)', async () => { - // Arrange - MockFeatureFlagService.getFeatureFlag.mockReturnValueOnce(okAsync(true)) MockSubService.getQuarantinePresignedPostData.mockReturnValueOnce( errAsync(new CreatePresignedPostError()), ) @@ -1114,9 +1083,9 @@ describe('submission.controller', () => { ) }) - it('should return 200 if getFeatureFlag returns okAsync(true) and getQuarantinePresignedPostData returns okAsync with the presigned URLs', async () => { + it('should return 200 if getQuarantinePresignedPostData returns okAsync with the presigned URLs', async () => { // Arrange - MockFeatureFlagService.getFeatureFlag.mockReturnValueOnce(okAsync(true)) + const MOCK_PRESIGNED_URLS = [ { key: 'value' }, ] as unknown as AttachmentPresignedPostDataMapType[] 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 936bffb53a..084c0353f8 100644 --- a/src/app/modules/submission/encrypt-submission/encrypt-submission.controller.ts +++ b/src/app/modules/submission/encrypt-submission/encrypt-submission.controller.ts @@ -622,24 +622,12 @@ const _createSubmission = async ({ return await performEncryptPostSubmissionActions(submission, responses) } -// TODO (FRM-1232): remove endpoint after encryption boundary is shifted -export const handleEncryptedSubmission = [ - CaptchaMiddleware.validateCaptchaParams, - TurnstileMiddleware.validateTurnstileParams, - EncryptSubmissionMiddleware.validateEncryptSubmissionParams, - EncryptSubmissionMiddleware.createFormsgAndRetrieveForm, - EncryptSubmissionMiddleware.validateEncryptSubmission, - EncryptSubmissionMiddleware.moveEncryptedPayload, - submitEncryptModeForm, -] as ControllerHandler[] - export const handleStorageSubmission = [ CaptchaMiddleware.validateCaptchaParams, TurnstileMiddleware.validateTurnstileParams, ReceiverMiddleware.receiveStorageSubmission, EncryptSubmissionMiddleware.validateStorageSubmissionParams, EncryptSubmissionMiddleware.createFormsgAndRetrieveForm, - EncryptSubmissionMiddleware.checkNewBoundaryEnabled, EncryptSubmissionMiddleware.scanAndRetrieveAttachments, EncryptSubmissionMiddleware.validateStorageSubmission, EncryptSubmissionMiddleware.encryptSubmission, 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 d97da8f613..d9b9bacb93 100644 --- a/src/app/modules/submission/encrypt-submission/encrypt-submission.middleware.ts +++ b/src/app/modules/submission/encrypt-submission/encrypt-submission.middleware.ts @@ -3,10 +3,6 @@ import { NextFunction } from 'express' import { StatusCodes } from 'http-status-codes' import { ok, okAsync, Result, ResultAsync } from 'neverthrow' -import { - featureFlags, - VIRUS_SCANNER_SUBMISSION_VERSION, -} from '../../../../../shared/constants' import { BasicField, FormAuthType, @@ -161,38 +157,6 @@ export const validateStorageSubmissionParams = celebrate({ }), }) -/** - * Guardrail to prevent new endpoint from being used for regular storage mode forms. - * TODO (FRM-1232): remove this guardrail when encryption boundary is shifted. - */ -export const checkNewBoundaryEnabled = async ( - req: StorageSubmissionMiddlewareHandlerRequest, - res: Parameters[1], - next: NextFunction, -) => { - const logMeta = { - action: 'checkNewBoundaryEnabled', - ...createReqMeta(req), - } - - const newBoundaryEnabled = req.formsg.featureFlags.includes( - featureFlags.encryptionBoundaryShift, - ) - - 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() -} - /** * Asynchronous virus scanning for storage submissions v2.1+. This is used for non-dev environments. * @param responses all responses in the storage submissions v2.1+ request. @@ -268,44 +232,7 @@ export const scanAndRetrieveAttachments = async ( ...createReqMeta(req), } - // TODO (FRM-1413): remove this guardrail when virus scanning has completed rollout. - // Step 1: If virus scanner is not enabled, skip this middleware. - - const virusScannerEnabled = req.formsg.featureFlags.includes( - featureFlags.encryptionBoundaryShiftVirusScanner, - ) - - if (!virusScannerEnabled) { - logger.warn({ - message: 'Virus scanner is not enabled on BE.', - meta: logMeta, - }) - - return next() - } - - // TODO (FRM-1413): remove this guardrail when virus scanning has completed rollout. - // Step 2: If virus scanner is enabled, check if storage submission v2.1+. Storage submission v2.1 onwards - // should have virus scanning enabled. If not, skip this middleware. - // Note: Version number is sent by the frontend and should only be >=2.1 if virus scanning is enabled on the frontend. - - if (req.body.version < VIRUS_SCANNER_SUBMISSION_VERSION) { - logger.warn({ - message: 'Virus scanner is not enabled on FE.', - meta: logMeta, - }) - return next() - } - - logger.info({ - message: 'Virus scanner is enabled on both BE and FE.', - meta: logMeta, - }) - - // At this point, virus scanner is enabled and storage submission v2.1+. This means that both the FE and BE - // have virus scanning enabled. - - // Step 3 + 4: For each attachment, trigger lambda to scan and if it succeeds, retrieve attachment from clean bucket. Do this asynchronously. + // For each attachment, trigger lambda to scan and if it succeeds, retrieve attachment from clean bucket. Do this asynchronously. const scanAndRetrieveFilesResult: Result< ParsedClearFormFieldResponse[], // true for attachment fields, false for non-attachment fields. | VirusScanFailedError @@ -341,7 +268,7 @@ export const scanAndRetrieveAttachments = async ( meta: logMeta, }) - // Step 5: Replace req.body.responses with the new responses with populated attachments. + // Replace req.body.responses with the new responses with populated attachments. req.body.responses = scanAndRetrieveFilesResult.value return next() diff --git a/src/app/modules/submission/submission.controller.ts b/src/app/modules/submission/submission.controller.ts index 537d66fd67..9575926099 100644 --- a/src/app/modules/submission/submission.controller.ts +++ b/src/app/modules/submission/submission.controller.ts @@ -5,7 +5,6 @@ import { StatusCodes } from 'http-status-codes' import JSONStream from 'JSONStream' import { errAsync, okAsync, ResultAsync } from 'neverthrow' -import { featureFlags } from '../../../../shared/constants' import { AttachmentPresignedPostDataMapType, AttachmentSizeMapType, @@ -22,17 +21,13 @@ import { createReqMeta } from '../../utils/request' import { getFormAfterPermissionChecks } from '../auth/auth.service' import { DatabaseError } from '../core/core.errors' import { ControllerHandler } from '../core/core.types' -import { getFeatureFlag } from '../feature-flags/feature-flags.service' import { PermissionLevel } from '../form/admin-form/admin-form.types' import { PaymentNotFoundError } from '../payments/payments.errors' import { getPopulatedUserById } from '../user/user.service' import { createStorageModeSubmissionDto } from './encrypt-submission/encrypt-submission.utils' import { createMultirespondentSubmissionDto } from './multirespondent-submission/multirespondent-submission.utils' -import { - FeatureDisabledError, - InvalidSubmissionTypeError, -} from './submission.errors' +import { InvalidSubmissionTypeError } from './submission.errors' import { addPaymentDataStream, getEncryptedSubmissionData, @@ -445,46 +440,17 @@ export const getS3PresignedPostData: ControllerHandler< action: 'getS3PresignedPostData', ...createReqMeta(req), } - - return getFeatureFlag(featureFlags.encryptionBoundaryShiftVirusScanner) - .map((virusScannerEnabled) => { - if (!virusScannerEnabled) { - logger.warn({ - message: 'Virus scanning has not been enabled.', - meta: logMeta, - }) - - const { statusCode, errorMessage } = mapRouteError( - new FeatureDisabledError(), - ) - return res.status(statusCode).send({ - message: errorMessage, - }) - } - - return getQuarantinePresignedPostData(req.body) - .map((presignedUrls) => { - logger.info({ - message: 'Successfully retrieved quarantine presigned post data.', - meta: logMeta, - }) - return res.status(StatusCodes.OK).send(presignedUrls) - }) - .mapErr((error) => { - logger.error({ - message: 'Failure getting quarantine presigned post data.', - meta: logMeta, - error, - }) - const { statusCode, errorMessage } = mapRouteError(error) - return res.status(statusCode).send({ - message: errorMessage, - }) - }) + return getQuarantinePresignedPostData(req.body) + .map((presignedUrls) => { + logger.info({ + message: 'Successfully retrieved quarantine presigned post data.', + meta: logMeta, + }) + return res.status(StatusCodes.OK).send(presignedUrls) }) .mapErr((error) => { logger.error({ - message: 'Error retrieving feature flags.', + message: 'Failure getting quarantine presigned post data.', meta: logMeta, error, }) 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 a013174bde..056c74c977 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 @@ -3,12 +3,10 @@ import dbHandler from '__tests__/unit/backend/helpers/jest-db' import jwt from 'jsonwebtoken' import { omit } from 'lodash' import mongoose from 'mongoose' -import { errAsync, okAsync } from 'neverthrow' -import { featureFlags } from 'shared/constants' +import { okAsync } from 'neverthrow' import session, { Session } from 'supertest-session' import { aws } from 'src/app/config/config' -import { DatabaseError } from 'src/app/modules/core/core.errors' import * as FeatureFlagsService from 'src/app/modules/feature-flags/feature-flags.service' import { FormFieldSchema } from 'src/types' @@ -948,292 +946,6 @@ describe('public-form.submissions.routes', () => { }) }) - describe('POST /forms/:formId/submissions/encrypt', () => { - const MOCK_ENCRYPTED_CONTENT = `${'a'.repeat(44)};${'a'.repeat( - 32, - )}:${'a'.repeat(4)}` - const MOCK_SUBMISSION_BODY = { - responses: [], - encryptedContent: MOCK_ENCRYPTED_CONTENT, - - version: 1, - } - describe('SPCP authentication', () => { - describe('SingPass', () => { - it('should return 200 when submission is valid', async () => { - jest - .spyOn(SpOidcClient.prototype, 'verifyJwt') - .mockResolvedValueOnce({ - userName: 'S1234567A', - }) - const { form } = await dbHandler.insertEncryptForm({ - formOptions: { - esrvcId: 'mockEsrvcId', - authType: FormAuthType.SP, - hasCaptcha: false, - status: FormStatus.Public, - }, - }) - - const response = await request - .post(`/forms/${form._id}/submissions/encrypt`) - .send(MOCK_SUBMISSION_BODY) - .query({ captchaResponse: 'null', captchaType: '' }) - .set('Cookie', ['jwtSp=mockJwt']) - - expect(response.status).toBe(200) - expect(response.body).toEqual({ - message: 'Form submission successful.', - submissionId: expect.any(String), - timestamp: expect.any(Number), - }) - }) - - it('should return 401 when submission does not have JWT', async () => { - const { form } = await dbHandler.insertEncryptForm({ - formOptions: { - esrvcId: 'mockEsrvcId', - authType: FormAuthType.SP, - hasCaptcha: false, - status: FormStatus.Public, - }, - }) - - const response = await request - .post(`/forms/${form._id}/submissions/encrypt`) - .send(MOCK_SUBMISSION_BODY) - .query({ captchaResponse: 'null', captchaType: '' }) - // Note cookie is not set - - expect(response.status).toBe(401) - expect(response.body).toEqual({ - message: - 'Something went wrong with your login. Please try logging in and submitting again.', - spcpSubmissionFailure: true, - }) - }) - - it('should return 401 when submission has the wrong JWT type', async () => { - const { form } = await dbHandler.insertEncryptForm({ - formOptions: { - esrvcId: 'mockEsrvcId', - authType: FormAuthType.SP, - hasCaptcha: false, - status: FormStatus.Public, - }, - }) - - const response = await request - .post(`/forms/${form._id}/submissions/encrypt`) - .send(MOCK_SUBMISSION_BODY) - .query({ captchaResponse: 'null', captchaType: '' }) - .set('Cookie', ['jwtCp=mockJwt']) - // Note cookie is for CorpPass, not SingPass - - expect(response.status).toBe(401) - expect(response.body).toEqual({ - message: - 'Something went wrong with your login. Please try logging in and submitting again.', - spcpSubmissionFailure: true, - }) - }) - - it('should return 401 when submission has invalid JWT', async () => { - // Mock auth client to return error when decoding JWT - jest - .spyOn(SpOidcClient.prototype, 'verifyJwt') - .mockRejectedValueOnce(new Error()) - - const { form } = await dbHandler.insertEncryptForm({ - formOptions: { - esrvcId: 'mockEsrvcId', - authType: FormAuthType.SP, - hasCaptcha: false, - status: FormStatus.Public, - }, - }) - - const response = await request - .post(`/forms/${form._id}/submissions/encrypt`) - .send(MOCK_SUBMISSION_BODY) - .query({ captchaResponse: 'null', captchaType: '' }) - .set('Cookie', ['jwtSp=mockJwt']) - - expect(response.status).toBe(401) - expect(response.body).toEqual({ - message: - 'Something went wrong with your login. Please try logging in and submitting again.', - spcpSubmissionFailure: true, - }) - }) - - it('should return 401 when submission has JWT with the wrong shape', async () => { - // Mock auth client to return wrong decoded JWT shape - jest - .spyOn(SpOidcClient.prototype, 'verifyJwt') - .mockResolvedValueOnce({ - wrongKey: 'S1234567A', - }) - - const { form } = await dbHandler.insertEncryptForm({ - formOptions: { - esrvcId: 'mockEsrvcId', - authType: FormAuthType.SP, - hasCaptcha: false, - status: FormStatus.Public, - }, - }) - - const response = await request - .post(`/forms/${form._id}/submissions/encrypt`) - .send(MOCK_SUBMISSION_BODY) - .query({ captchaResponse: 'null', captchaType: '' }) - .set('Cookie', ['jwtSp=mockJwt']) - - expect(response.status).toBe(401) - expect(response.body).toEqual({ - message: - 'Something went wrong with your login. Please try logging in and submitting again.', - spcpSubmissionFailure: true, - }) - }) - }) - - describe('CorpPass', () => { - it('should return 200 when submission is valid', async () => { - mockCpClient.verifyJwt.mockResolvedValueOnce({ - userName: 'S1234567A', - userInfo: 'MyCorpPassUEN', - }) - const { form } = await dbHandler.insertEncryptForm({ - formOptions: { - esrvcId: 'mockEsrvcId', - authType: FormAuthType.CP, - hasCaptcha: false, - status: FormStatus.Public, - }, - }) - - const response = await request - .post(`/forms/${form._id}/submissions/encrypt`) - .send(MOCK_SUBMISSION_BODY) - .query({ captchaResponse: 'null', captchaType: '' }) - .set('Cookie', ['jwtCp=mockJwt']) - - expect(response.status).toBe(200) - expect(response.body).toEqual({ - message: 'Form submission successful.', - submissionId: expect.any(String), - timestamp: expect.any(Number), - }) - }) - - it('should return 401 when submission does not have JWT', async () => { - const { form } = await dbHandler.insertEncryptForm({ - formOptions: { - esrvcId: 'mockEsrvcId', - authType: FormAuthType.CP, - hasCaptcha: false, - status: FormStatus.Public, - }, - }) - - const response = await request - .post(`/forms/${form._id}/submissions/encrypt`) - .send(MOCK_SUBMISSION_BODY) - .query({ captchaResponse: 'null', captchaType: '' }) - // Note cookie is not set - - expect(response.status).toBe(401) - expect(response.body).toEqual({ - message: - 'Something went wrong with your login. Please try logging in and submitting again.', - spcpSubmissionFailure: true, - }) - }) - - it('should return 401 when submission has the wrong JWT type', async () => { - const { form } = await dbHandler.insertEncryptForm({ - formOptions: { - esrvcId: 'mockEsrvcId', - authType: FormAuthType.CP, - hasCaptcha: false, - status: FormStatus.Public, - }, - }) - - const response = await request - .post(`/forms/${form._id}/submissions/encrypt`) - .send(MOCK_SUBMISSION_BODY) - .query({ captchaResponse: 'null', captchaType: '' }) - // Note cookie is for SingPass, not CorpPass - .set('Cookie', ['jwtSp=mockJwt']) - - expect(response.status).toBe(401) - expect(response.body).toEqual({ - message: - 'Something went wrong with your login. Please try logging in and submitting again.', - spcpSubmissionFailure: true, - }) - }) - - it('should return 401 when submission has invalid JWT', async () => { - // Mock auth client to return error when decoding JWT - mockCpClient.verifyJwt.mockRejectedValueOnce(new Error()) - const { form } = await dbHandler.insertEncryptForm({ - formOptions: { - esrvcId: 'mockEsrvcId', - authType: FormAuthType.CP, - hasCaptcha: false, - status: FormStatus.Public, - }, - }) - - const response = await request - .post(`/forms/${form._id}/submissions/encrypt`) - .send(MOCK_SUBMISSION_BODY) - .query({ captchaResponse: 'null', captchaType: '' }) - .set('Cookie', ['jwtCp=mockJwt']) - - expect(response.status).toBe(401) - expect(response.body).toEqual({ - message: - 'Something went wrong with your login. Please try logging in and submitting again.', - spcpSubmissionFailure: true, - }) - }) - - it('should return 401 when submission has JWT with the wrong shape', async () => { - // Mock auth client to return wrong decoded JWT shape - mockCpClient.verifyJwt.mockResolvedValueOnce({ - wrongKey: 'S1234567A', - }) - const { form } = await dbHandler.insertEncryptForm({ - formOptions: { - esrvcId: 'mockEsrvcId', - authType: FormAuthType.CP, - hasCaptcha: false, - status: FormStatus.Public, - }, - }) - - const response = await request - .post(`/forms/${form._id}/submissions/encrypt`) - .send(MOCK_SUBMISSION_BODY) - .query({ captchaResponse: 'null', captchaType: '' }) - .set('Cookie', ['jwtCp=mockJwt']) - - expect(response.status).toBe(401) - expect(response.body).toEqual({ - message: - 'Something went wrong with your login. Please try logging in and submitting again.', - spcpSubmissionFailure: true, - }) - }) - }) - }) - }) - describe('POST /forms/:formId/submissions/get-s3-presigned-post-data', () => { const FILE_MAP_1 = { id: '64ed84955ac23100636a00a0', size: 1 } const FILE_MAP_2 = { id: '64ed84a35ac23100636a00af', size: 19999999 } @@ -1368,54 +1080,6 @@ describe('public-form.submissions.routes', () => { }) }) - it('should return 403 if virus scanning has not been enabled', async () => { - const { form } = await dbHandler.insertEncryptForm({ - formOptions: { - esrvcId: 'mockEsrvcId', - authType: FormAuthType.CP, - hasCaptcha: false, - status: FormStatus.Public, - }, - }) - - jest - .spyOn(FeatureFlagsService, 'getFeatureFlag') - .mockReturnValue(okAsync(false)) - - const response = await request - .post(`/forms/${form._id}/submissions/get-s3-presigned-post-data`) - .send(VALID_PAYLOAD) - - expect(response.status).toBe(403) - expect(response.body).toEqual({ - message: 'This feature is disabled.', - }) - }) - - it('should return 500 if feature flag retrieving fails', async () => { - const { form } = await dbHandler.insertEncryptForm({ - formOptions: { - esrvcId: 'mockEsrvcId', - authType: FormAuthType.CP, - hasCaptcha: false, - status: FormStatus.Public, - }, - }) - - jest - .spyOn(FeatureFlagsService, 'getFeatureFlag') - .mockReturnValue(errAsync(new DatabaseError())) - - const response = await request - .post(`/forms/${form._id}/submissions/get-s3-presigned-post-data`) - .send(VALID_PAYLOAD) - - expect(response.status).toBe(500) - expect(response.body).toEqual({ - message: 'Something went wrong. Please try again.', - }) - }) - it('should return 500 if creating of presigned post data fails', async () => { const { form } = await dbHandler.insertEncryptForm({ formOptions: { @@ -1492,46 +1156,6 @@ describe('public-form.submissions.routes', () => { describe('POST /forms/:formId/submissions/storage', () => { describe('Joi validation', () => { - beforeEach(() => { - jest - .spyOn(FeatureFlagsService, 'getEnabledFlags') - .mockReturnValue(okAsync([featureFlags.encryptionBoundaryShift])) - }) - - it('should return 403 when feature flag has not been enabled', async () => { - // Arrange - const { form } = await dbHandler.insertEncryptForm({ - formOptions: { - esrvcId: 'mockEsrvcId', - hasCaptcha: false, - status: FormStatus.Public, - }, - }) - - jest - .spyOn(FeatureFlagsService, 'getEnabledFlags') - .mockReturnValueOnce(okAsync([])) - - // Act - const response = await request - .post(`/forms/${form._id}/submissions/storage`) - // MOCK_RESPONSE contains all required keys - .field( - 'body', - JSON.stringify({ - responses: [MOCK_TEXTFIELD_RESPONSE], - version: 2, - }), - ) - .query({ captchaResponse: 'null', captchaType: '' }) - - // Assert - expect(response.status).toBe(403) - expect(response.body).toEqual({ - message: 'This endpoint has not been enabled for this form.', - }) - }) - it('should return 200 when submission is valid', async () => { // Arrange const { form } = await dbHandler.insertEncryptForm({ @@ -1928,11 +1552,6 @@ describe('public-form.submissions.routes', () => { ...MOCK_NO_RESPONSES_BODY, version: 2, } - beforeEach(() => { - jest - .spyOn(FeatureFlagsService, 'getEnabledFlags') - .mockReturnValue(okAsync([featureFlags.encryptionBoundaryShift])) - }) describe('SingPass', () => { it('should return 200 when submission is valid', async () => { diff --git a/src/app/routes/api/v3/forms/public-forms.submissions.routes.ts b/src/app/routes/api/v3/forms/public-forms.submissions.routes.ts index 56198b9930..947e9ed643 100644 --- a/src/app/routes/api/v3/forms/public-forms.submissions.routes.ts +++ b/src/app/routes/api/v3/forms/public-forms.submissions.routes.ts @@ -32,26 +32,6 @@ PublicFormsSubmissionsRouter.route( EmailSubmissionController.handleEmailSubmission, ) -// TODO (FRM-1232): remove endpoint after encryption boundary is shifted -/** - * Submit a form response, validate submission params and stores the encrypted - * contents. - * Optionally, an autoreply confirming submission is sent back to the user, if - * an email address was given. SMS autoreplies for mobile number fields are also - * sent if the feature is enabled. - * @route POST /forms/:formId/submissions/encrypt - * @param response.body.required - contains the entire form submission - * @param captchaResponse.query - contains the reCAPTCHA response artifact, if any - * @returns 200 - submission made - * @returns 400 - submission has bad data and could not be processed - */ -PublicFormsSubmissionsRouter.route( - '/:formId([a-fA-F0-9]{24})/submissions/encrypt', -).post( - limitRate({ max: rateLimitConfig.submissions }), - EncryptSubmissionController.handleEncryptedSubmission, -) - /** * Submit a form response before public key encryption, performs pre-encryption * steps (e.g. field validation, virus scanning) and stores the encrypted contents.