From d5bbb4d30454a4d6050c0bda9a041bac2928ec3e Mon Sep 17 00:00:00 2001 From: tshuli <63710093+tshuli@users.noreply.github.com> Date: Tue, 14 Nov 2023 23:46:20 +0800 Subject: [PATCH] chore: remove eb shift frontend feature flags (#6869) * chore: remove virus scanner feature flag * chore: remove enable encryption boundary shift feature flag * chore: remove growthbook in public form provider * chore: remove unused storage submission modes * chore: fetch fallback should also use virus scanning * chore: update mutation * chore: move fallback to catch block * chore: add back network error check * chore: remove unused import and const * chore: remove clear word --- .../public-form/PublicFormProvider.tsx | 145 +++------------- .../features/public-form/PublicFormService.ts | 162 ++---------------- .../src/features/public-form/mutations.ts | 107 ++++++++---- shared/constants/form.ts | 1 - 4 files changed, 112 insertions(+), 303 deletions(-) diff --git a/frontend/src/features/public-form/PublicFormProvider.tsx b/frontend/src/features/public-form/PublicFormProvider.tsx index fc3feeaffb..ecc8b777ff 100644 --- a/frontend/src/features/public-form/PublicFormProvider.tsx +++ b/frontend/src/features/public-form/PublicFormProvider.tsx @@ -11,11 +11,6 @@ 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, - useFeatureValue, - useGrowthBook, -} from '@growthbook/growthbook-react' import { differenceInMilliseconds, isPast } from 'date-fns' import get from 'lodash/get' import simplur from 'simplur' @@ -135,23 +130,6 @@ export const PublicFormProvider = ({ /* enabled= */ !submissionData, ) - const growthbook = useGrowthBook() - - useEffect(() => { - if (growthbook) { - growthbook.setAttributes({ - // Only update the `formId` attribute, keep the rest the same - ...growthbook.getAttributes(), - formId, - }) - } - }, [growthbook, formId]) - - const enableEncryptionBoundaryShift = useFeatureValue( - featureFlags.encryptionBoundaryShift, - true, - ) - // Scroll to top of page when user has finished their submission. useLayoutEffect(() => { if (submissionData) { @@ -269,18 +247,11 @@ export const PublicFormProvider = ({ } }, [data?.form.form_fields, toast, vfnToastIdRef]) - const enableVirusScanner = useFeatureIsOn( - featureFlags.encryptionBoundaryShiftVirusScanner, - ) - const { submitEmailModeFormMutation, - submitStorageModeFormMutation, submitEmailModeFormFetchMutation, - submitStorageModeFormFetchMutation, - submitStorageModeClearFormMutation, - submitStorageModeClearFormFetchMutation, - submitStorageModeClearFormWithVirusScanningMutation, + submitStorageModeFormWithVirusScanningFetchMutation, + submitStorageModeFormWithVirusScanningMutation, } = usePublicFormMutations(formId, submissionData?.id ?? '') const { handleLogoutMutation } = usePublicAuthMutations(formId) @@ -484,9 +455,7 @@ export const PublicFormProvider = ({ : {}), } - const submitStorageFormWithFetch = function ( - routeToNewStorageModeSubmission: boolean, - ) { + const submitStorageFormWithFetch = function () { datadogLogs.logger.info(`handleSubmitForm: submitting via fetch`, { meta: { ...logMeta, @@ -495,16 +464,11 @@ export const PublicFormProvider = ({ }, }) - return ( - routeToNewStorageModeSubmission - ? submitStorageModeClearFormFetchMutation - : submitStorageModeFormFetchMutation - ) + return submitStorageModeFormWithVirusScanningFetchMutation .mutateAsync( { ...formData, ...formPaymentData, - publicKey: form.publicKey, }, { onSuccess: ({ @@ -546,7 +510,7 @@ export const PublicFormProvider = ({ // TODO (#5826): Toggle to use fetch for submissions instead of axios. If enabled, this is used for testing and to use fetch instead of axios by default if testing shows fetch is more stable. Remove once network error is resolved if (useFetchForSubmissions) { - return submitStorageFormWithFetch(enableEncryptionBoundaryShift) + return submitStorageFormWithFetch() } datadogLogs.logger.info(`handleSubmitForm: submitting via axios`, { meta: { @@ -556,9 +520,8 @@ export const PublicFormProvider = ({ }, }) - // TODO (FRM-1413): Move to main return statement once virus scanner has been fully rolled out - if (enableEncryptionBoundaryShift && enableVirusScanner) { - return submitStorageModeClearFormWithVirusScanningMutation.mutateAsync( + return submitStorageModeFormWithVirusScanningMutation + .mutateAsync( { ...formData, ...formPaymentData, @@ -582,86 +545,27 @@ export const PublicFormProvider = ({ timestamp, }) }, - onError: (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, - }, - }, - ) - - // defaults to the safest option of storage submission without virus scanning - return submitStorageFormWithFetch( - enableEncryptionBoundaryShift, - ) - }, }, ) - } - - return ( - ( - enableEncryptionBoundaryShift - ? submitStorageModeClearFormMutation - : submitStorageModeFormMutation - ) - .mutateAsync( - { - ...formData, - ...formPaymentData, - publicKey: form.publicKey, - }, + .catch(async (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() - return submitStorageFormWithFetch( - enableEncryptionBoundaryShift, - ) - } - showErrorToast(error, form) - }) - ) + }, + ) + if (/Network Error/i.test(error.message)) { + axiosDebugFlow() + // fallback to fetch + return submitStorageFormWithFetch() + } + showErrorToast(error, form) + }) } } }, @@ -678,16 +582,11 @@ export const PublicFormProvider = ({ getCaptchaResponse, submitEmailModeFormFetchMutation, submitEmailModeFormMutation, - enableEncryptionBoundaryShift, - enableVirusScanner, - submitStorageModeClearFormMutation, - submitStorageModeFormMutation, - submitStorageModeClearFormFetchMutation, - submitStorageModeFormFetchMutation, + submitStorageModeFormWithVirusScanningMutation, + submitStorageModeFormWithVirusScanningFetchMutation, navigate, formId, storePaymentMemory, - submitStorageModeClearFormWithVirusScanningMutation, ], ) diff --git a/frontend/src/features/public-form/PublicFormService.ts b/frontend/src/features/public-form/PublicFormService.ts index 1ae78819d0..9cab540bee 100644 --- a/frontend/src/features/public-form/PublicFormService.ts +++ b/frontend/src/features/public-form/PublicFormService.ts @@ -1,10 +1,7 @@ import { PresignedPost } from 'aws-sdk/clients/s3' import axios from 'axios' -import { - ENCRYPTION_BOUNDARY_SHIFT_SUBMISSION_VERSION, - VIRUS_SCANNER_SUBMISSION_VERSION, -} from '~shared/constants' +import { VIRUS_SCANNER_SUBMISSION_VERSION } from '~shared/constants' import { SubmitFormIssueBodyDto, SuccessMessageDto } from '~shared/types' import { AttachmentPresignedPostDataMapType, @@ -39,7 +36,6 @@ import { FormFieldValues } from '~templates/Field' import { createClearSubmissionFormData, createClearSubmissionWithVirusScanningFormData, - createEncryptedSubmissionData, getAttachmentsMap, } from './utils/createSubmission' import { filterHiddenInputs } from './utils/filterHiddenInputs' @@ -155,46 +151,9 @@ export const submitEmailModeForm = async ({ ).then(({ data }) => data) } -export const submitStorageModeForm = async ({ - formFields, - formLogics, - formInputs, - formId, - publicKey, - captchaResponse = null, - captchaType = '', - paymentReceiptEmail, - responseMetadata, - paymentProducts, - payments, -}: SubmitStorageFormArgs) => { - const filteredInputs = filterHiddenInputs({ - formFields, - formInputs, - formLogics, - }) - const submissionContent = await createEncryptedSubmissionData({ - formFields, - formInputs: filteredInputs, - publicKey, - responseMetadata, - paymentReceiptEmail, - payments, - paymentProducts, - }) - return ApiService.post( - `${PUBLIC_FORMS_ENDPOINT}/${formId}/submissions/encrypt`, - submissionContent, - { - params: { - captchaResponse: String(captchaResponse), - captchaType, - }, - }, - ).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 submitStorageModeFormWithVirusScanningWithFetch = async ({ formFields, formLogics, formInputs, @@ -205,63 +164,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({ @@ -284,7 +206,7 @@ export const submitStorageModeClearFormWithFetch = async ({ } // Submit storage mode form with virus scanning (storage v2.1+) -export const submitStorageModeClearFormWithVirusScanning = async ({ +export const submitStorageModeFormWithVirusScanning = async ({ formFields, formLogics, formInputs, @@ -369,56 +291,6 @@ export const submitEmailModeFormWithFetch = async ({ return processFetchResponse(response) } -// TODO (#5826): Fallback mutation using Fetch. Remove once network error is resolved -export const submitStorageModeFormWithFetch = async ({ - formFields, - formLogics, - formInputs, - formId, - publicKey, - captchaResponse = null, - captchaType = '', - paymentReceiptEmail, - responseMetadata, - paymentProducts, - payments, -}: SubmitStorageFormArgs) => { - const filteredInputs = filterHiddenInputs({ - formFields, - formInputs, - formLogics, - }) - const submissionContent = await createEncryptedSubmissionData({ - formFields, - formInputs: filteredInputs, - publicKey, - responseMetadata, - paymentReceiptEmail, - payments, - paymentProducts, - }) - - // Add captcha response to query string - const queryString = new URLSearchParams({ - captchaResponse: String(captchaResponse), - captchaType, - }).toString() - - const response = await fetch( - `${API_BASE_URL}${PUBLIC_FORMS_ENDPOINT}/${formId}/submissions/encrypt?${queryString}`, - { - method: 'POST', - body: JSON.stringify(submissionContent), - headers: { - 'Content-Type': 'application/json', - Accept: 'application/json', - }, - }, - ) - - return processFetchResponse(response) -} - /** * Post feedback for a given form. * @param formId the id of the form to post feedback for diff --git a/frontend/src/features/public-form/mutations.ts b/frontend/src/features/public-form/mutations.ts index 1975644ce7..e39c2561f4 100644 --- a/frontend/src/features/public-form/mutations.ts +++ b/frontend/src/features/public-form/mutations.ts @@ -20,13 +20,9 @@ import { submitEmailModeFormWithFetch, submitFormFeedback, submitFormIssue, - SubmitStorageFormArgs, SubmitStorageFormClearArgs, - submitStorageModeClearForm, - submitStorageModeClearFormWithFetch, - submitStorageModeClearFormWithVirusScanning, - submitStorageModeForm, - submitStorageModeFormWithFetch, + submitStorageModeFormWithVirusScanning, + submitStorageModeFormWithVirusScanningWithFetch, uploadAttachmentToQuarantine, } from './PublicFormService' @@ -82,18 +78,6 @@ export const usePublicFormMutations = ( }, ) - const submitStorageModeFormMutation = useMutation( - (args: Omit) => { - return submitStorageModeForm({ ...args, formId }) - }, - ) - - 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) => { @@ -101,15 +85,73 @@ export const usePublicFormMutations = ( }, ) - const submitStorageModeFormFetchMutation = useMutation( - (args: Omit) => { - return submitStorageModeFormWithFetch({ ...args, formId }) - }, - ) + const submitStorageModeFormWithVirusScanningFetchMutation = useMutation( + async (args: Omit) => { + const attachmentSizes = await getAttachmentSizes(args) + // If there are no attachments, submit form without virus scanning by passing in empty list + if (attachmentSizes.length === 0) { + return submitStorageModeFormWithVirusScanningWithFetch({ + ...args, + fieldIdToQuarantineKeyMap: [], + formId, + }) + } + + // Step 1: Get presigned post data for all attachment fields + return ( + getAttachmentPresignedPostData({ ...args, formId, attachmentSizes }) + .then( + // Step 2: Upload attachments to quarantine bucket asynchronously + (fieldToPresignedPostDataMap) => + Promise.all( + fieldToPresignedPostDataMap.map( + async (fieldToPresignedPostData) => { + const attachmentFile = + args.formInputs[fieldToPresignedPostData.id] - const submitStorageModeClearFormFetchMutation = useMutation( - (args: Omit) => { - return submitStorageModeClearFormWithFetch({ ...args, formId }) + // Check if response is a File object (from an attachment field) + if (!(attachmentFile instanceof File)) + throw new Error('Field is not attachment') + + const uploadResponse = await uploadAttachmentToQuarantine( + fieldToPresignedPostData.presignedPostData, + attachmentFile, + ) + + // If status code is not 200-299, throw error + if ( + uploadResponse.status < 200 || + uploadResponse.status > 299 + ) + throw new Error( + `Attachment upload failed - ${uploadResponse.statusText}`, + ) + + const quarantineBucketKey = + fieldToPresignedPostData.presignedPostData.fields.key + + if (!quarantineBucketKey) + throw new Error( + 'key is not defined in presigned post data', + ) + + return { + fieldId: fieldToPresignedPostData.id, + quarantineBucketKey, + } as FieldIdToQuarantineKeyType + }, + ), + ), + ) + // Step 3: Submit form with keys to quarantine bucket attachments + .then((fieldIdToQuarantineKeyMap) => { + return submitStorageModeFormWithVirusScanningWithFetch({ + ...args, + fieldIdToQuarantineKeyMap, + formId, + }) + }) + ) }, ) @@ -123,12 +165,12 @@ export const usePublicFormMutations = ( }, ) - const submitStorageModeClearFormWithVirusScanningMutation = useMutation( + const submitStorageModeFormWithVirusScanningMutation = useMutation( async (args: Omit) => { const attachmentSizes = await getAttachmentSizes(args) // If there are no attachments, submit form without virus scanning by passing in empty list if (attachmentSizes.length === 0) { - return submitStorageModeClearFormWithVirusScanning({ + return submitStorageModeFormWithVirusScanning({ ...args, fieldIdToQuarantineKeyMap: [], formId, @@ -182,7 +224,7 @@ export const usePublicFormMutations = ( ) // Step 3: Submit form with keys to quarantine bucket attachments .then((fieldIdToQuarantineKeyMap) => { - return submitStorageModeClearFormWithVirusScanning({ + return submitStorageModeFormWithVirusScanning({ ...args, fieldIdToQuarantineKeyMap, formId, @@ -194,13 +236,10 @@ export const usePublicFormMutations = ( return { submitEmailModeFormMutation, - submitStorageModeFormMutation, submitFormFeedbackMutation, - submitStorageModeFormFetchMutation, submitEmailModeFormFetchMutation, - submitStorageModeClearFormMutation, - submitStorageModeClearFormFetchMutation, - submitStorageModeClearFormWithVirusScanningMutation, + submitStorageModeFormWithVirusScanningFetchMutation, + submitStorageModeFormWithVirusScanningMutation, } } diff --git a/shared/constants/form.ts b/shared/constants/form.ts index a6a26085e1..6b355aa7b6 100644 --- a/shared/constants/form.ts +++ b/shared/constants/form.ts @@ -68,5 +68,4 @@ 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