From 5adb99219f64d3695c51ad0055010f558e122273 Mon Sep 17 00:00:00 2001
From: Lin Huiqing <37061143+LinHuiqing@users.noreply.github.com>
Date: Thu, 31 Aug 2023 16:59:40 +0800
Subject: [PATCH 1/6] feat: % rollout of new storage submission endpoint
(#6665)
* feat: use new endpoint for storage submissions
* fix: answerArray fields hidden error
- response.answerArray did not return visibility true
* feat: growthbook on frontend
* feat: script to set encryptionBoundaryShift to true
* feat: default `encryptionBoundaryShift` to true
* fix: form server model backend tests
- set encryptionBoundaryShift flag for encrypted forms
* refactor: setBody -> additionalBodyFields naming
* refactor: rename ...FormV2Args -> ...FormClearArgs
* fix: tighter typing for answerArray
* refactor: create FormDataArgs types
* feat: fetch fallback for new storage submission
- refactor: use 'clear' instead of 'V2'
* fix: add missing dependencies to handleSubmitForm
* feat: use db env feature flags for BE
* fix: missing args with env feature flags
* fix: admin-form.controller tests
* fix: IncomingEncryptSubmission tests
* feat: script to unset encryptionBoundaryShift flag
* feat: rm form-level encryptionBoundaryShift flags
* fix: use docker images from edge/main instead
* fix: use dl-cdn rather than nl
- nl.alpinelinux.org mirror is outdated
* fix: typo for dl-cdn
* fix: testing instead of main url
- font-wqy-zenhei@edge only available on edge/testing (ref: https://repology.org/project/fonts:wqy-zenhei/versions)
* fix: update alpinelinux mirror for dev docker
* fix: add growthbook to connect src CSP
* fix: add more growthbook paths to csp
* fix: helmet tests
* feat: growthbook reverse proxy api host
* feat: use appropriate domains for proxy
* revert: changes to add growthbook to CSP
* docs: ENCRYPTION_BOUNDARY_SHIFT_ENCRYPTION_VERSION
* refactor: isDev for clarity of apiHost
* refactor: use constants for link and path
- refactor: rm unused formsgSdkMode
* feat: unset encryptionBoundaryShift only if present
* refactor: mv feature loading to AppRouter
---
docker-compose.yml | 2 +
frontend/package-lock.json | 55 +++++++++
frontend/package.json | 1 +
frontend/src/app/App.tsx | 5 +-
frontend/src/app/AppRouter.tsx | 11 +-
frontend/src/constants/links.ts | 3 +
frontend/src/constants/routes.ts | 3 +
frontend/src/contexts/GrowthbookContext.tsx | 26 ++++
.../admin-form/common/AdminViewFormService.ts | 12 +-
.../public-form/PublicFormProvider.tsx | 54 +++++++--
.../features/public-form/PublicFormService.ts | 113 ++++++++++++++++--
.../src/features/public-form/mutations.ts | 16 +++
.../public-form/utils/createSubmission.ts | 31 ++++-
frontend/src/growthbook.ts | 17 +++
.../unset-encryption-boundary-shift.js | 43 +++++++
shared/constants/feature-flags.ts | 1 +
shared/types/core.ts | 1 +
shared/types/form/form.ts | 3 -
src/app/config/features/growthbook.config.ts | 18 +++
.../__tests__/form.server.model.spec.ts | 2 +-
src/app/models/form.server.model.ts | 5 -
.../__tests__/admin-form.controller.spec.ts | 12 ++
.../form/admin-form/admin-form.controller.ts | 4 +-
src/app/modules/frontend/frontend.service.ts | 3 +
.../submission/ParsedResponsesObject.class.ts | 7 +-
.../email-submission.controller.ts | 6 +-
.../IncomingEncryptSubmission.class.ts | 14 ++-
.../IncomingEncryptSubmission.class.spec.ts | 9 +-
.../encrypt-submission.controller.ts | 2 +
.../encrypt-submission.middleware.ts | 39 ++++--
.../modules/submission/submission.utils.ts | 3 +-
src/types/api/encrypt_submission.ts | 1 +
src/types/form.ts | 3 -
33 files changed, 463 insertions(+), 62 deletions(-)
create mode 100644 frontend/src/contexts/GrowthbookContext.tsx
create mode 100644 frontend/src/growthbook.ts
create mode 100644 scripts/20230826_unset-encryption-boundary-shift/unset-encryption-boundary-shift.js
create mode 100644 src/app/config/features/growthbook.config.ts
diff --git a/docker-compose.yml b/docker-compose.yml
index 6cc5d1f8c3..065ab61ce2 100644
--- a/docker-compose.yml
+++ b/docker-compose.yml
@@ -121,6 +121,8 @@ services:
- GOGOV_API_KEY
# Bearer token API key format
- API_KEY_VERSION=v1
+ # env vars for growthbook
+ - GROWTHBOOK_CLIENT_KEY
mockpass:
build: https://github.com/opengovsg/mockpass.git#v4.0.4
diff --git a/frontend/package-lock.json b/frontend/package-lock.json
index b86df0e23b..ae5eccdd7b 100644
--- a/frontend/package-lock.json
+++ b/frontend/package-lock.json
@@ -15,6 +15,7 @@
"@emotion/react": "^11.7.0",
"@emotion/styled": "^11.6.0",
"@floating-ui/react-dom-interactions": "^0.9.3",
+ "@growthbook/growthbook-react": "^0.17.0",
"@opengovsg/formsg-sdk": "^0.10.0",
"@stablelib/base64": "^1.0.1",
"@stripe/react-stripe-js": "^1.15.0",
@@ -3864,6 +3865,31 @@
"resolved": "https://registry.npmjs.org/@gilbarbara/deep-equal/-/deep-equal-0.1.1.tgz",
"integrity": "sha512-SjSBspHXlclODLtSoPIQwBhfeBjncC05NlNoFELJ6xZQkyYDJsVCcs7+f+etHR2cYPbHLjnh1C06lQlCbMEWEA=="
},
+ "node_modules/@growthbook/growthbook": {
+ "version": "0.27.0",
+ "resolved": "https://registry.npmjs.org/@growthbook/growthbook/-/growthbook-0.27.0.tgz",
+ "integrity": "sha512-gt/DWXfgyudY3gXAUjzKm9kMjVfCzfRc8d0nQQiXCP523ow23jThrmBzkRKCN+zxYVxUKcHjP9yjU9EKJqP4Fw==",
+ "dependencies": {
+ "dom-mutator": "^0.5.0"
+ },
+ "engines": {
+ "node": ">=10"
+ }
+ },
+ "node_modules/@growthbook/growthbook-react": {
+ "version": "0.17.0",
+ "resolved": "https://registry.npmjs.org/@growthbook/growthbook-react/-/growthbook-react-0.17.0.tgz",
+ "integrity": "sha512-od+bSg3RD9AXfi/TWFDSzRaB6RuJLkUX5eFOZz8u8wL8yDbgd5RF9cwu4lQIF9/tyG1WLu4qipV+SM2v5s4BcQ==",
+ "dependencies": {
+ "@growthbook/growthbook": "^0.27.0"
+ },
+ "engines": {
+ "node": ">=10"
+ },
+ "peerDependencies": {
+ "react": "^16.8.0-0 || ^17.0.0-0 || ^18.0.0-0"
+ }
+ },
"node_modules/@hapi/address": {
"version": "2.1.4",
"resolved": "https://registry.npmjs.org/@hapi/address/-/address-2.1.4.tgz",
@@ -21833,6 +21859,14 @@
"utila": "~0.4"
}
},
+ "node_modules/dom-mutator": {
+ "version": "0.5.0",
+ "resolved": "https://registry.npmjs.org/dom-mutator/-/dom-mutator-0.5.0.tgz",
+ "integrity": "sha512-bbeX8HWE8JGzraFgbVBX4ws2g3heZFuTtrleQBuN7huy+7n2n7etSuVnot3/1z3jdY2MiwuvoS4Ep1UT2rrGBw==",
+ "engines": {
+ "node": ">=10"
+ }
+ },
"node_modules/dom-serializer": {
"version": "0.2.2",
"resolved": "https://registry.npmjs.org/dom-serializer/-/dom-serializer-0.2.2.tgz",
@@ -51138,6 +51172,22 @@
"resolved": "https://registry.npmjs.org/@gilbarbara/deep-equal/-/deep-equal-0.1.1.tgz",
"integrity": "sha512-SjSBspHXlclODLtSoPIQwBhfeBjncC05NlNoFELJ6xZQkyYDJsVCcs7+f+etHR2cYPbHLjnh1C06lQlCbMEWEA=="
},
+ "@growthbook/growthbook": {
+ "version": "0.27.0",
+ "resolved": "https://registry.npmjs.org/@growthbook/growthbook/-/growthbook-0.27.0.tgz",
+ "integrity": "sha512-gt/DWXfgyudY3gXAUjzKm9kMjVfCzfRc8d0nQQiXCP523ow23jThrmBzkRKCN+zxYVxUKcHjP9yjU9EKJqP4Fw==",
+ "requires": {
+ "dom-mutator": "^0.5.0"
+ }
+ },
+ "@growthbook/growthbook-react": {
+ "version": "0.17.0",
+ "resolved": "https://registry.npmjs.org/@growthbook/growthbook-react/-/growthbook-react-0.17.0.tgz",
+ "integrity": "sha512-od+bSg3RD9AXfi/TWFDSzRaB6RuJLkUX5eFOZz8u8wL8yDbgd5RF9cwu4lQIF9/tyG1WLu4qipV+SM2v5s4BcQ==",
+ "requires": {
+ "@growthbook/growthbook": "^0.27.0"
+ }
+ },
"@hapi/address": {
"version": "2.1.4",
"resolved": "https://registry.npmjs.org/@hapi/address/-/address-2.1.4.tgz",
@@ -64689,6 +64739,11 @@
"utila": "~0.4"
}
},
+ "dom-mutator": {
+ "version": "0.5.0",
+ "resolved": "https://registry.npmjs.org/dom-mutator/-/dom-mutator-0.5.0.tgz",
+ "integrity": "sha512-bbeX8HWE8JGzraFgbVBX4ws2g3heZFuTtrleQBuN7huy+7n2n7etSuVnot3/1z3jdY2MiwuvoS4Ep1UT2rrGBw=="
+ },
"dom-serializer": {
"version": "0.2.2",
"resolved": "https://registry.npmjs.org/dom-serializer/-/dom-serializer-0.2.2.tgz",
diff --git a/frontend/package.json b/frontend/package.json
index b3ece67cf9..84dfe1fb17 100644
--- a/frontend/package.json
+++ b/frontend/package.json
@@ -10,6 +10,7 @@
"@emotion/react": "^11.7.0",
"@emotion/styled": "^11.6.0",
"@floating-ui/react-dom-interactions": "^0.9.3",
+ "@growthbook/growthbook-react": "^0.17.0",
"@opengovsg/formsg-sdk": "^0.10.0",
"@stablelib/base64": "^1.0.1",
"@stripe/react-stripe-js": "^1.15.0",
diff --git a/frontend/src/app/App.tsx b/frontend/src/app/App.tsx
index c7aefeecde..0fedfda1a9 100644
--- a/frontend/src/app/App.tsx
+++ b/frontend/src/app/App.tsx
@@ -9,6 +9,7 @@ import { datadogLogs } from '@datadog/browser-logs'
import { theme } from '~theme/index'
import { AuthProvider } from '~contexts/AuthContext'
+import { GrowthBookProvider } from '~contexts/GrowthbookContext'
import { HttpError } from '~services/ApiService'
import { AppHelmet } from './AppHelmet'
@@ -70,7 +71,9 @@ export const App = (): JSX.Element => {
-
+
+
+
diff --git a/frontend/src/app/AppRouter.tsx b/frontend/src/app/AppRouter.tsx
index 9cab6884ec..e8594856f4 100644
--- a/frontend/src/app/AppRouter.tsx
+++ b/frontend/src/app/AppRouter.tsx
@@ -1,6 +1,7 @@
-import { lazy, Suspense } from 'react'
+import { lazy, Suspense, useEffect } from 'react'
import { Route, Routes } from 'react-router-dom'
import { Box } from '@chakra-ui/react'
+import { useGrowthBook } from '@growthbook/growthbook-react'
import {
ADMINFORM_PREVIEW_ROUTE,
@@ -72,6 +73,14 @@ const WithSuspense = ({ children }: { children: React.ReactNode }) => (
)
export const AppRouter = (): JSX.Element => {
+ const growthbook = useGrowthBook()
+ useEffect(() => {
+ if (growthbook) {
+ // Load features from the GrowthBook API
+ growthbook.loadFeatures({ autoRefresh: true })
+ }
+ }, [growthbook])
+
return (
diff --git a/frontend/src/constants/links.ts b/frontend/src/constants/links.ts
index b0f8563477..68d8958e28 100644
--- a/frontend/src/constants/links.ts
+++ b/frontend/src/constants/links.ts
@@ -84,3 +84,6 @@ export const OGP_SGID = 'https://go.gov.sg/sgid-formsg'
export const OGP_FORMSG_REPO = 'https://github.com/opengovsg/formsg'
export const FORMSG_UAT = 'https://uat.form.gov.sg'
+
+export const GROWTHBOOK_DEV_PROXY =
+ 'https://proxy-growthbook-stg.formsg.workers.dev'
diff --git a/frontend/src/constants/routes.ts b/frontend/src/constants/routes.ts
index 24e9ef6577..01eee2bf69 100644
--- a/frontend/src/constants/routes.ts
+++ b/frontend/src/constants/routes.ts
@@ -50,3 +50,6 @@ export const ACTIVE_ADMINFORM_RESULTS_ROUTE_REGEX = new RegExp(
'i',
)
export const PAYMENT_PAGE_SUBROUTE = 'payment/:paymentId'
+
+// Path for growthbook api proxy, set up on cloudflare workers. Worker script: https://github.com/opengovsg/formsg-private/pull/171.
+export const GROWTHBOOK_API_HOST_PATH = '/api/v1/proxy/growthbook'
diff --git a/frontend/src/contexts/GrowthbookContext.tsx b/frontend/src/contexts/GrowthbookContext.tsx
new file mode 100644
index 0000000000..5de2f6e58c
--- /dev/null
+++ b/frontend/src/contexts/GrowthbookContext.tsx
@@ -0,0 +1,26 @@
+import { ReactNode } from 'react'
+import { GrowthBookProvider as BaseGrowthBookProvider } from '@growthbook/growthbook-react'
+
+import { createGrowthbookInstance } from '~/growthbook'
+
+import { useEnv } from '~features/env/queries'
+
+/**
+ * Provider component that wraps your app and makes auth object available to any
+ * child component that calls `useAuth()`.
+ */
+export const GrowthBookProvider = ({ children }: { children: ReactNode }) => {
+ const { data: { growthbookClientKey } = {} } = useEnv()
+
+ return (
+
+ {children}
+
+ )
+}
diff --git a/frontend/src/features/admin-form/common/AdminViewFormService.ts b/frontend/src/features/admin-form/common/AdminViewFormService.ts
index dde6add4df..565bd17b9c 100644
--- a/frontend/src/features/admin-form/common/AdminViewFormService.ts
+++ b/frontend/src/features/admin-form/common/AdminViewFormService.ts
@@ -23,7 +23,7 @@ import {
SubmitStorageFormArgs,
} from '~features/public-form/PublicFormService'
import {
- createEmailSubmissionFormData,
+ createClearSubmissionFormData,
createEncryptedSubmissionData,
filterHiddenInputs,
} from '~features/public-form/utils'
@@ -166,7 +166,10 @@ export const submitEmailModeFormPreview = async ({
formInputs,
formLogics,
})
- const formData = createEmailSubmissionFormData(formFields, filteredInputs)
+ const formData = createClearSubmissionFormData({
+ formFields,
+ formInputs: filteredInputs,
+ })
return ApiService.post(
`${ADMIN_FORM_ENDPOINT}/${formId}/preview/submissions/email`,
@@ -216,7 +219,10 @@ export const submitEmailModeFormPreviewWithFetch = async ({
formInputs,
formLogics,
})
- const formData = createEmailSubmissionFormData(formFields, filteredInputs)
+ const formData = createClearSubmissionFormData({
+ formFields,
+ formInputs: filteredInputs,
+ })
const response = await fetch(
`${API_BASE_URL}${ADMIN_FORM_ENDPOINT}/${formId}/preview/submissions/email`,
diff --git a/frontend/src/features/public-form/PublicFormProvider.tsx b/frontend/src/features/public-form/PublicFormProvider.tsx
index fecb70d246..68eeb73e69 100644
--- a/frontend/src/features/public-form/PublicFormProvider.tsx
+++ b/frontend/src/features/public-form/PublicFormProvider.tsx
@@ -11,6 +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 { differenceInMilliseconds, isPast } from 'date-fns'
import get from 'lodash/get'
import simplur from 'simplur'
@@ -130,6 +131,22 @@ 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 routeToNewStorageModeSubmission = useFeatureIsOn(
+ featureFlags.encryptionBoundaryShift,
+ )
+
// Scroll to top of page when user has finished their submission.
useLayoutEffect(() => {
if (submissionData) {
@@ -252,8 +269,14 @@ export const PublicFormProvider = ({
submitStorageModeFormMutation,
submitEmailModeFormFetchMutation,
submitStorageModeFormFetchMutation,
+ submitStorageModeClearFormMutation,
+ submitStorageModeClearFormFetchMutation,
} = usePublicFormMutations(formId, submissionData?.id ?? '')
+ const submitStorageModeVnMutation = routeToNewStorageModeSubmission
+ ? submitStorageModeClearFormMutation
+ : submitStorageModeFormMutation
+
const { handleLogoutMutation } = usePublicAuthMutations(formId)
const navigate = useNavigate()
@@ -464,7 +487,12 @@ export const PublicFormProvider = ({
},
})
- return submitStorageModeFormFetchMutation
+ const submitStorageModeVnFetchMutation =
+ routeToNewStorageModeSubmission
+ ? submitStorageModeClearFormFetchMutation
+ : submitStorageModeFormFetchMutation
+
+ return submitStorageModeVnFetchMutation
.mutateAsync(
{
...formData,
@@ -522,7 +550,7 @@ export const PublicFormProvider = ({
})
return (
- submitStorageModeFormMutation
+ submitStorageModeVnMutation
.mutateAsync(
{
...formData,
@@ -578,21 +606,23 @@ export const PublicFormProvider = ({
[
data,
enableTurnstileFeatureFlag,
- getTurnstileResponse,
- getCaptchaResponse,
captchaType,
+ startTime,
+ isPaymentEnabled,
+ numVisibleFields,
+ useFetchForSubmissions,
+ getTurnstileResponse,
showErrorToast,
+ getCaptchaResponse,
+ submitEmailModeFormFetchMutation,
submitEmailModeFormMutation,
- submitStorageModeFormMutation,
- formId,
+ submitStorageModeVnMutation,
+ routeToNewStorageModeSubmission,
+ submitStorageModeClearFormFetchMutation,
+ submitStorageModeFormFetchMutation,
navigate,
+ formId,
storePaymentMemory,
- submitEmailModeFormFetchMutation,
- submitStorageModeFormFetchMutation,
- useFetchForSubmissions,
- numVisibleFields,
- startTime,
- isPaymentEnabled,
],
)
diff --git a/frontend/src/features/public-form/PublicFormService.ts b/frontend/src/features/public-form/PublicFormService.ts
index e51391e85f..76e198e076 100644
--- a/frontend/src/features/public-form/PublicFormService.ts
+++ b/frontend/src/features/public-form/PublicFormService.ts
@@ -25,13 +25,17 @@ import {
import { FormFieldValues } from '~templates/Field'
import {
- createEmailSubmissionFormData,
+ createClearSubmissionFormData,
createEncryptedSubmissionData,
} from './utils/createSubmission'
import { filterHiddenInputs } from './utils/filterHiddenInputs'
export const PUBLIC_FORMS_ENDPOINT = '/forms'
+// 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
+const ENCRYPTION_BOUNDARY_SHIFT_ENCRYPTION_VERSION = 2
+
/**
* Gets public view of form, along with any
* identify information obtained from Singpass/Corppass/MyInfo.
@@ -93,6 +97,12 @@ export type SubmitStorageFormArgs = SubmitEmailFormArgs & {
payments?: PaymentFieldsDto
}
+export type SubmitStorageFormClearArgs = SubmitEmailFormArgs & {
+ paymentReceiptEmail?: string
+ paymentProducts?: Array
+ payments?: PaymentFieldsDto
+}
+
export const submitEmailModeForm = async ({
formFields,
formLogics,
@@ -107,11 +117,11 @@ export const submitEmailModeForm = async ({
formInputs,
formLogics,
})
- const formData = createEmailSubmissionFormData(
+ const formData = createClearSubmissionFormData({
formFields,
- filteredInputs,
+ formInputs: filteredInputs,
responseMetadata,
- )
+ })
return ApiService.post(
`${PUBLIC_FORMS_ENDPOINT}/${formId}/submissions/email`,
@@ -164,6 +174,95 @@ export const submitStorageModeForm = async ({
).then(({ data }) => data)
}
+export const submitStorageModeClearForm = 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_ENCRYPTION_VERSION,
+ })
+
+ return ApiService.post(
+ `${PUBLIC_FORMS_ENDPOINT}/${formId}/submissions/storage`,
+ formData,
+ {
+ params: {
+ captchaResponse: String(captchaResponse),
+ captchaType: captchaType,
+ },
+ },
+ ).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_ENCRYPTION_VERSION,
+ })
+
+ // 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/storage?${queryString}`,
+ {
+ method: 'POST',
+ body: formData,
+ headers: {
+ Accept: 'application/json',
+ },
+ },
+ )
+
+ return processFetchResponse(response)
+}
+
// TODO (#5826): Fallback mutation using Fetch. Remove once network error is resolved
export const submitEmailModeFormWithFetch = async ({
formFields,
@@ -179,11 +278,11 @@ export const submitEmailModeFormWithFetch = async ({
formInputs,
formLogics,
})
- const formData = createEmailSubmissionFormData(
+ const formData = createClearSubmissionFormData({
formFields,
- filteredInputs,
+ formInputs: filteredInputs,
responseMetadata,
- )
+ })
// Add captcha response to query string
const queryString = new URLSearchParams({
diff --git a/frontend/src/features/public-form/mutations.ts b/frontend/src/features/public-form/mutations.ts
index 32dca83f19..f8d91a7d45 100644
--- a/frontend/src/features/public-form/mutations.ts
+++ b/frontend/src/features/public-form/mutations.ts
@@ -18,6 +18,8 @@ import {
submitFormFeedback,
submitFormIssue,
SubmitStorageFormArgs,
+ submitStorageModeClearForm,
+ submitStorageModeClearFormWithFetch,
submitStorageModeForm,
submitStorageModeFormWithFetch,
} from './PublicFormService'
@@ -80,6 +82,12 @@ 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,6 +101,12 @@ export const usePublicFormMutations = (
},
)
+ const submitStorageModeClearFormFetchMutation = useMutation(
+ (args: Omit) => {
+ return submitStorageModeClearFormWithFetch({ ...args, formId })
+ },
+ )
+
const submitFormFeedbackMutation = useMutation(
(args: SubmitFormFeedbackBodyDto) =>
submitFormFeedback(formId, submissionId, args),
@@ -109,6 +123,8 @@ export const usePublicFormMutations = (
submitFormFeedbackMutation,
submitStorageModeFormFetchMutation,
submitEmailModeFormFetchMutation,
+ submitStorageModeClearFormMutation,
+ submitStorageModeClearFormFetchMutation,
}
}
diff --git a/frontend/src/features/public-form/utils/createSubmission.ts b/frontend/src/features/public-form/utils/createSubmission.ts
index 5eedd2ebbc..c24b723e58 100644
--- a/frontend/src/features/public-form/utils/createSubmission.ts
+++ b/frontend/src/features/public-form/utils/createSubmission.ts
@@ -77,21 +77,42 @@ export const createEncryptedSubmissionData = async ({
}
}
+type CreateEmailSubmissionFormDataArgs = {
+ formFields: FormFieldDto[]
+ formInputs: FormFieldValues
+ responseMetadata?: ResponseMetadata
+}
+
+type CreateStorageSubmissionFormDataArgs = CreateEmailSubmissionFormDataArgs & {
+ paymentReceiptEmail?: string
+ paymentProducts?: ProductItem[]
+ payments?: PaymentFieldsDto
+ version: number
+}
+
/**
+ * Used for both Email mode submissions and Storage mode submissions after encryption boundary shift.
* @returns formData containing form responses and attachments.
* @throws Error if form inputs are invalid.
*/
-export const createEmailSubmissionFormData = (
- formFields: FormFieldDto[],
- formInputs: FormFieldValues,
- responseMetadata?: ResponseMetadata,
+export const createClearSubmissionFormData = (
+ formDataArgs:
+ | CreateEmailSubmissionFormDataArgs
+ | CreateStorageSubmissionFormDataArgs,
) => {
+ const { formFields, formInputs, ...formDataArgsRest } = formDataArgs
const responses = createResponsesArray(formFields, formInputs)
const attachments = getAttachmentsMap(formFields, formInputs)
// Convert content to FormData object.
const formData = new FormData()
- formData.append('body', JSON.stringify({ responses, responseMetadata }))
+ formData.append(
+ 'body',
+ JSON.stringify({
+ responses,
+ ...formDataArgsRest,
+ }),
+ )
if (!isEmpty(attachments)) {
forOwn(attachments, (attachment, fieldId) => {
diff --git a/frontend/src/growthbook.ts b/frontend/src/growthbook.ts
new file mode 100644
index 0000000000..c8aa435e67
--- /dev/null
+++ b/frontend/src/growthbook.ts
@@ -0,0 +1,17 @@
+import { GrowthBook } from '@growthbook/growthbook-react'
+
+import { GROWTHBOOK_DEV_PROXY } from '~constants/links'
+import { GROWTHBOOK_API_HOST_PATH } from '~constants/routes'
+
+export const createGrowthbookInstance = (clientKey: string) => {
+ const isDev = process.env.NODE_ENV === 'development'
+
+ return new GrowthBook({
+ apiHost: `${
+ isDev ? GROWTHBOOK_DEV_PROXY : process.env.REACT_APP_URL
+ }${GROWTHBOOK_API_HOST_PATH}`,
+ clientKey: clientKey,
+ // Enable easier debugging during development
+ enableDevMode: isDev,
+ })
+}
diff --git a/scripts/20230826_unset-encryption-boundary-shift/unset-encryption-boundary-shift.js b/scripts/20230826_unset-encryption-boundary-shift/unset-encryption-boundary-shift.js
new file mode 100644
index 0000000000..1854ef124f
--- /dev/null
+++ b/scripts/20230826_unset-encryption-boundary-shift/unset-encryption-boundary-shift.js
@@ -0,0 +1,43 @@
+/* eslint-disable */
+/**
+ * This script sets encryptionBoundaryShift flag for all forms to true.
+ * A session is used to dry run the update operation before committing the changes.
+ */
+
+// Start a session.
+session = db.getMongo().startSession( { readPreference: { mode: "primary" } } );
+
+// Start a transaction
+session.startTransaction( { readConcern: { level: "local" }, writeConcern: { w: "majority" } } );
+
+formsCollection = session.getDatabase("formsg").forms;
+
+console.log('=== before ===')
+
+// total # of documents
+const totalDocs = formsCollection.countDocuments()
+console.log('total # of documents:', totalDocs);
+
+// # of documents with encryptionBoundaryShift set
+const docsSetBefore = formsCollection.countDocuments({ encryptionBoundaryShift: { $exists: true } })
+console.log('# of documents with encryptionBoundaryShift set:', docsSetBefore);
+
+
+// set documents
+formsCollection.updateMany(
+ { encryptionBoundaryShift: { $exists: true } },
+ {
+ $unset: { encryptionBoundaryShift: true }
+ }
+);
+
+console.log('=== after ===')
+
+// # of documents with encryptionBoundaryShift set to true (should match # of documents to modify)
+const docsSetAfter = formsCollection.countDocuments({ encryptionBoundaryShift: { $exists: true } })
+console.log('# of documents with encryptionBoundaryShift set:', docsSetAfter);
+
+session.abortTransaction();
+// session.commitTransaction();
+
+session.endSession();
diff --git a/shared/constants/feature-flags.ts b/shared/constants/feature-flags.ts
index 3d2881b641..bd0267c58f 100644
--- a/shared/constants/feature-flags.ts
+++ b/shared/constants/feature-flags.ts
@@ -2,4 +2,5 @@ export const featureFlags = {
payment: 'payment' as const,
goLinks: 'goLinks' as const,
turnstile: 'turnstile' as const,
+ encryptionBoundaryShift: 'encryption-boundary-shift' as const,
}
diff --git a/shared/types/core.ts b/shared/types/core.ts
index d8cfadbc5b..b3d880bfbc 100644
--- a/shared/types/core.ts
+++ b/shared/types/core.ts
@@ -38,4 +38,5 @@ export type ClientEnvVars = {
goGovBaseUrl: string
adminFeedbackFieldThreshold: number
adminFeedbackDisplayFrequency: number
+ growthbookClientKey: string
}
diff --git a/shared/types/form/form.ts b/shared/types/form/form.ts
index 4831994125..b66101175e 100644
--- a/shared/types/form/form.ts
+++ b/shared/types/form/form.ts
@@ -150,14 +150,11 @@ export interface FormBase {
webhook: FormWebhook
responseMode: FormResponseMode
-
- encryptionBoundaryShift?: boolean
}
export interface EmailFormBase extends FormBase {
responseMode: FormResponseMode.Email
emails: string[]
- encryptionBoundaryShift?: never
}
export interface StorageFormBase extends FormBase {
diff --git a/src/app/config/features/growthbook.config.ts b/src/app/config/features/growthbook.config.ts
new file mode 100644
index 0000000000..bc3f65e30c
--- /dev/null
+++ b/src/app/config/features/growthbook.config.ts
@@ -0,0 +1,18 @@
+import convict, { Schema } from 'convict'
+
+export interface IGrowthbook {
+ growthbookClientKey: string
+}
+
+const growthbookSchema: Schema = {
+ growthbookClientKey: {
+ doc: 'Growthbook Client Key',
+ format: String,
+ default: '',
+ env: 'GROWTHBOOK_CLIENT_KEY',
+ },
+}
+
+export const growthbookConfig = convict(growthbookSchema)
+ .validate({ allowed: 'strict' })
+ .getProperties()
diff --git a/src/app/models/__tests__/form.server.model.spec.ts b/src/app/models/__tests__/form.server.model.spec.ts
index 3ef6e66ff0..bd2e6bfe0c 100644
--- a/src/app/models/__tests__/form.server.model.spec.ts
+++ b/src/app/models/__tests__/form.server.model.spec.ts
@@ -431,7 +431,7 @@ describe('Form Model', () => {
describe('Encrypted form schema', () => {
const ENCRYPT_FORM_DEFAULTS = merge(
- { responseMode: 'encrypt', encryptionBoundaryShift: false },
+ { responseMode: 'encrypt' },
FORM_DEFAULTS,
PAYMENTS_DEFAULTS,
)
diff --git a/src/app/models/form.server.model.ts b/src/app/models/form.server.model.ts
index 1e120397d0..0e57fc723a 100644
--- a/src/app/models/form.server.model.ts
+++ b/src/app/models/form.server.model.ts
@@ -226,11 +226,6 @@ const EncryptedFormSchema = new Schema({
gstRegNo: { type: String, required: true, trim: true },
},
},
-
- encryptionBoundaryShift: {
- type: Boolean,
- default: false,
- },
})
const EncryptedFormDocumentSchema =
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 b823057a56..be1f5d41ee 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
@@ -5420,6 +5420,7 @@ describe('admin-form.controller', () => {
expect(MockParsedResponsesObject.parseResponses).toHaveBeenCalledWith(
MOCK_FORM,
MOCK_RESPONSES,
+ false,
)
expect(MockAdminFormService.extractMyInfoFieldIds).toHaveBeenCalledWith(
MOCK_FORM.form_fields,
@@ -5954,6 +5955,7 @@ describe('admin-form.controller', () => {
expect(MockParsedResponsesObject.parseResponses).toHaveBeenCalledWith(
MOCK_FORM,
MOCK_RESPONSES,
+ false,
)
expect(MockAdminFormService.extractMyInfoFieldIds).not.toHaveBeenCalled()
expect(
@@ -6011,6 +6013,7 @@ describe('admin-form.controller', () => {
expect(MockParsedResponsesObject.parseResponses).toHaveBeenCalledWith(
MOCK_FORM,
MOCK_RESPONSES,
+ false,
)
expect(MockAdminFormService.extractMyInfoFieldIds).not.toHaveBeenCalled()
expect(
@@ -6068,6 +6071,7 @@ describe('admin-form.controller', () => {
expect(MockParsedResponsesObject.parseResponses).toHaveBeenCalledWith(
MOCK_FORM,
MOCK_RESPONSES,
+ false,
)
expect(MockAdminFormService.extractMyInfoFieldIds).not.toHaveBeenCalled()
expect(
@@ -6125,6 +6129,7 @@ describe('admin-form.controller', () => {
expect(MockParsedResponsesObject.parseResponses).toHaveBeenCalledWith(
MOCK_FORM,
MOCK_RESPONSES,
+ false,
)
expect(MockAdminFormService.extractMyInfoFieldIds).toHaveBeenCalledWith(
MOCK_FORM.form_fields,
@@ -6191,6 +6196,7 @@ describe('admin-form.controller', () => {
expect(MockParsedResponsesObject.parseResponses).toHaveBeenCalledWith(
MOCK_FORM,
MOCK_RESPONSES,
+ false,
)
expect(MockAdminFormService.extractMyInfoFieldIds).toHaveBeenCalledWith(
MOCK_FORM.form_fields,
@@ -6257,6 +6263,7 @@ describe('admin-form.controller', () => {
expect(MockParsedResponsesObject.parseResponses).toHaveBeenCalledWith(
MOCK_FORM,
MOCK_RESPONSES,
+ false,
)
expect(MockAdminFormService.extractMyInfoFieldIds).toHaveBeenCalledWith(
MOCK_FORM.form_fields,
@@ -6391,6 +6398,7 @@ describe('admin-form.controller', () => {
MOCK_FORM,
MOCK_RESPONSES,
MOCK_ENCRYPTED_CONTENT,
+ true,
)
expect(
MockEncryptSubmissionService.createEncryptSubmissionWithoutSave,
@@ -6793,6 +6801,7 @@ describe('admin-form.controller', () => {
MOCK_FORM,
MOCK_RESPONSES,
MOCK_ENCRYPTED_CONTENT,
+ true,
)
expect(
MockEncryptSubmissionService.createEncryptSubmissionWithoutSave,
@@ -6846,6 +6855,7 @@ describe('admin-form.controller', () => {
MOCK_FORM,
MOCK_RESPONSES,
MOCK_ENCRYPTED_CONTENT,
+ true,
)
expect(
MockEncryptSubmissionService.createEncryptSubmissionWithoutSave,
@@ -6899,6 +6909,7 @@ describe('admin-form.controller', () => {
MOCK_FORM,
MOCK_RESPONSES,
MOCK_ENCRYPTED_CONTENT,
+ true,
)
expect(
MockEncryptSubmissionService.createEncryptSubmissionWithoutSave,
@@ -6952,6 +6963,7 @@ describe('admin-form.controller', () => {
MOCK_FORM,
MOCK_RESPONSES,
MOCK_ENCRYPTED_CONTENT,
+ true,
)
expect(
MockEncryptSubmissionService.createEncryptSubmissionWithoutSave,
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 4597e6e137..2a7e615e55 100644
--- a/src/app/modules/form/admin-form/admin-form.controller.ts
+++ b/src/app/modules/form/admin-form/admin-form.controller.ts
@@ -1679,7 +1679,7 @@ export const submitEncryptPreview: ControllerHandler<
}),
)
.andThen((form) =>
- IncomingEncryptSubmission.init(form, responses, encryptedContent)
+ IncomingEncryptSubmission.init(form, responses, encryptedContent, true) // set as true as there's no need to gate anything if the its not a real submission
.map((incomingSubmission) => ({ incomingSubmission, form }))
.mapErr((error) => {
logger.error({
@@ -1779,7 +1779,7 @@ export const submitEmailPreview: ControllerHandler<
const parsedResponsesResult = await SubmissionService.validateAttachments(
responses,
form.responseMode,
- ).andThen(() => ParsedResponsesObject.parseResponses(form, responses))
+ ).andThen(() => ParsedResponsesObject.parseResponses(form, responses, false)) // email mode submissions (esp previews) does not need to use encryption boundary shift code
if (parsedResponsesResult.isErr()) {
logger.error({
message: 'Error while parsing responses for preview submission',
diff --git a/src/app/modules/frontend/frontend.service.ts b/src/app/modules/frontend/frontend.service.ts
index 0cd7349473..f97dfa8c29 100644
--- a/src/app/modules/frontend/frontend.service.ts
+++ b/src/app/modules/frontend/frontend.service.ts
@@ -3,6 +3,7 @@ import config from '../../config/config'
import { captchaConfig } from '../../config/features/captcha.config'
import { goGovConfig } from '../../config/features/gogov.config'
import { googleAnalyticsConfig } from '../../config/features/google-analytics.config'
+import { growthbookConfig } from '../../config/features/growthbook.config'
import { paymentConfig } from '../../config/features/payment.config'
import { sentryConfig } from '../../config/features/sentry.config'
import { spcpMyInfoConfig } from '../../config/features/spcp-myinfo.config'
@@ -40,5 +41,7 @@ export const getClientEnvVars = (): ClientEnvVars => {
// Used for admin feedback in frontend
adminFeedbackFieldThreshold: config.adminFeedbackFieldThreshold,
adminFeedbackDisplayFrequency: config.adminFeedbackDisplayFrequency,
+
+ growthbookClientKey: growthbookConfig.growthbookClientKey,
}
}
diff --git a/src/app/modules/submission/ParsedResponsesObject.class.ts b/src/app/modules/submission/ParsedResponsesObject.class.ts
index 196969d702..f867ee6343 100644
--- a/src/app/modules/submission/ParsedResponsesObject.class.ts
+++ b/src/app/modules/submission/ParsedResponsesObject.class.ts
@@ -84,11 +84,16 @@ export default class ParsedResponsesObject {
static parseResponses(
form: IFormDocument,
responses: FieldResponse[],
+ encryptionBoundaryShiftEnabled: boolean,
): Result<
ParsedResponsesObject,
ProcessingError | ConflictError | ValidateFieldError
> {
- const filteredResponsesResult = getFilteredResponses(form, responses)
+ const filteredResponsesResult = getFilteredResponses(
+ form,
+ responses,
+ encryptionBoundaryShiftEnabled,
+ )
if (filteredResponsesResult.isErr()) {
return err(filteredResponsesResult.error)
}
diff --git a/src/app/modules/submission/email-submission/email-submission.controller.ts b/src/app/modules/submission/email-submission/email-submission.controller.ts
index 2cc6f73dde..852e2d2bb1 100644
--- a/src/app/modules/submission/email-submission/email-submission.controller.ts
+++ b/src/app/modules/submission/email-submission/email-submission.controller.ts
@@ -178,7 +178,11 @@ const submitEmailModeForm: ControllerHandler<
form.responseMode,
)
.andThen(() =>
- ParsedResponsesObject.parseResponses(form, req.body.responses),
+ ParsedResponsesObject.parseResponses(
+ form,
+ req.body.responses,
+ false, // email mode submissions do not need to use encryption boundary shift code
+ ),
)
.map((parsedResponses) => ({ parsedResponses, form }))
.mapErr((error) => {
diff --git a/src/app/modules/submission/encrypt-submission/IncomingEncryptSubmission.class.ts b/src/app/modules/submission/encrypt-submission/IncomingEncryptSubmission.class.ts
index 2c9a6de896..755e91923f 100644
--- a/src/app/modules/submission/encrypt-submission/IncomingEncryptSubmission.class.ts
+++ b/src/app/modules/submission/encrypt-submission/IncomingEncryptSubmission.class.ts
@@ -32,15 +32,16 @@ export default class IncomingEncryptSubmission extends IncomingSubmission {
form: IPopulatedEncryptedForm,
responses: FieldResponse[],
encryptedContent: string,
+ encryptionBoundaryShiftEnabled: boolean,
): Result<
IncomingEncryptSubmission,
ProcessingError | ConflictError | ValidateFieldError[]
> {
return checkIsEncryptedEncoding(encryptedContent)
.andThen(() => {
- if (form.encryptionBoundaryShift)
+ if (encryptionBoundaryShiftEnabled)
return ok(responses as FilteredResponse[])
- else return getFilteredResponses(form, responses)
+ else return getFilteredResponses(form, responses, false)
})
.andThen((filteredResponses) =>
this.getFieldMap(form, filteredResponses).map((fieldMap) => ({
@@ -60,9 +61,12 @@ export default class IncomingEncryptSubmission extends IncomingSubmission {
responseVisibilityPredicate(response: FieldResponse): boolean {
return (
- 'answer' in response &&
- typeof response.answer === 'string' &&
- response.answer.trim() !== ''
+ ('answer' in response &&
+ typeof response.answer === 'string' &&
+ response.answer.trim() !== '') ||
+ ('answerArray' in response &&
+ Array.isArray(response.answerArray) &&
+ response.answerArray.length > 0)
)
}
}
diff --git a/src/app/modules/submission/encrypt-submission/__tests__/IncomingEncryptSubmission.class.spec.ts b/src/app/modules/submission/encrypt-submission/__tests__/IncomingEncryptSubmission.class.spec.ts
index a841e2a604..05439fffe9 100644
--- a/src/app/modules/submission/encrypt-submission/__tests__/IncomingEncryptSubmission.class.spec.ts
+++ b/src/app/modules/submission/encrypt-submission/__tests__/IncomingEncryptSubmission.class.spec.ts
@@ -74,6 +74,7 @@ describe('IncomingEncryptSubmission', () => {
} as unknown as IPopulatedEncryptedForm,
responses,
'',
+ false,
)
expect(initResult._unsafeUnwrap().responses).toEqual(responses)
})
@@ -125,10 +126,10 @@ describe('IncomingEncryptSubmission', () => {
{
responseMode: FormResponseMode.Encrypt,
form_fields: basicFormFields,
- encryptionBoundaryShift: false,
} as unknown as IPopulatedEncryptedForm,
responses,
'',
+ false,
)
const filteredResponses = [mobileResponse, emailResponse]
expect(initResult._unsafeUnwrap().responses).toEqual(filteredResponses)
@@ -175,10 +176,10 @@ describe('IncomingEncryptSubmission', () => {
{
responseMode: FormResponseMode.Encrypt,
form_fields: basicFormFields,
- encryptionBoundaryShift: true,
} as unknown as IPopulatedEncryptedForm,
responses,
'',
+ true,
)
expect(initResult._unsafeUnwrap().responses).toEqual(filteredResponses)
})
@@ -211,6 +212,7 @@ describe('IncomingEncryptSubmission', () => {
} as unknown as IPopulatedEncryptedForm,
responses,
'',
+ false,
)
expect(initResult._unsafeUnwrapErr()).toEqual(
new ConflictError('Some form fields are missing'),
@@ -258,6 +260,7 @@ describe('IncomingEncryptSubmission', () => {
} as unknown as IPopulatedEncryptedForm,
responses,
'',
+ false,
)
expect(result.isOk()).toEqual(true)
@@ -280,6 +283,7 @@ describe('IncomingEncryptSubmission', () => {
} as unknown as IPopulatedEncryptedForm,
[mobileResponse],
'',
+ false,
)
expect(result.isErr()).toEqual(true)
@@ -305,6 +309,7 @@ describe('IncomingEncryptSubmission', () => {
} as unknown as IPopulatedEncryptedForm,
[],
'',
+ false,
)
expect(result.isErr()).toEqual(true)
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 8bf99d053a..84c3694da4 100644
--- a/src/app/modules/submission/encrypt-submission/encrypt-submission.controller.ts
+++ b/src/app/modules/submission/encrypt-submission/encrypt-submission.controller.ts
@@ -7,6 +7,7 @@ import mongoose from 'mongoose'
import { okAsync } from 'neverthrow'
import Stripe from 'stripe'
+import { featureFlags } from '../../../../../shared/constants'
import {
ErrorDto,
FormAuthType,
@@ -135,6 +136,7 @@ const submitEncryptModeForm = async (
form,
responses,
encryptedContent,
+ req.formsg.featureFlags.includes(featureFlags.encryptionBoundaryShift),
)
if (incomingSubmissionResult.isErr()) {
const { statusCode, errorMessage } = mapRouteError(
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 2cc985ac4c..c46cb3222f 100644
--- a/src/app/modules/submission/encrypt-submission/encrypt-submission.middleware.ts
+++ b/src/app/modules/submission/encrypt-submission/encrypt-submission.middleware.ts
@@ -4,6 +4,7 @@ import { NextFunction } from 'express'
import { StatusCodes } from 'http-status-codes'
import { chain, omit } from 'lodash'
+import { featureFlags } from '../../../../../shared/constants'
import {
BasicField,
StorageModeAttachment,
@@ -14,6 +15,7 @@ import { paymentConfig } from '../../../config/features/payment.config'
import formsgSdk from '../../../config/formsg-sdk'
import { createLoggerWithLabel } from '../../../config/logger'
import { createReqMeta } from '../../../utils/request'
+import * as FeatureFlagService from '../../feature-flags/feature-flags.service'
import { JoiPaymentProduct } from '../../form/admin-form/admin-form.payments.constants'
import * as FormService from '../../form/form.service'
import ParsedResponsesObject from '../ParsedResponsesObject.class'
@@ -130,9 +132,10 @@ export const checkNewBoundaryEnabled = async (
res: Parameters[1],
next: NextFunction,
) => {
- const formDef = req.formsg.formDef
-
- if (!formDef.encryptionBoundaryShift) {
+ const newBoundaryEnabled = req.formsg.featureFlags.includes(
+ featureFlags.encryptionBoundaryShift,
+ )
+ if (!newBoundaryEnabled) {
return res
.status(StatusCodes.FORBIDDEN)
.json({ message: 'This endpoint has not been enabled for this form.' })
@@ -160,7 +163,11 @@ export const validateSubmission = async (
formDef.responseMode,
)
.andThen(() =>
- ParsedResponsesObject.parseResponses(formDef, req.body.responses),
+ ParsedResponsesObject.parseResponses(
+ formDef,
+ req.body.responses,
+ req.formsg.featureFlags.includes(featureFlags.encryptionBoundaryShift),
+ ),
)
.map(() => next())
.mapErr((error) => {
@@ -265,6 +272,7 @@ export const encryptSubmission = async (
const filteredResponses = getFilteredResponses(
encryptedFormDef,
req.body.responses,
+ req.formsg.featureFlags.includes(featureFlags.encryptionBoundaryShift),
)
if (filteredResponses.isErr()) {
@@ -338,7 +346,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({
@@ -350,11 +371,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({
@@ -370,10 +391,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/modules/submission/submission.utils.ts b/src/app/modules/submission/submission.utils.ts
index 44130ef04f..761fdb1ded 100644
--- a/src/app/modules/submission/submission.utils.ts
+++ b/src/app/modules/submission/submission.utils.ts
@@ -149,10 +149,11 @@ export const extractEmailConfirmationData = (
export const getFilteredResponses = (
form: IFormDocument,
responses: FieldResponse[],
+ encryptionBoundaryShiftEnabled: boolean,
): Result => {
const isEncryptedMode =
form.responseMode === FormResponseMode.Encrypt &&
- !form.encryptionBoundaryShift
+ !encryptionBoundaryShiftEnabled
const responseModeFilter = getResponseModeFilter(isEncryptedMode)
const formFieldModeFilter = getFormFieldModeFilter(isEncryptedMode)
diff --git a/src/types/api/encrypt_submission.ts b/src/types/api/encrypt_submission.ts
index db0f9bd723..0a97d9c5b5 100644
--- a/src/types/api/encrypt_submission.ts
+++ b/src/types/api/encrypt_submission.ts
@@ -32,6 +32,7 @@ export type ParsedStorageModeSubmissionBody = ParsedEmailModeSubmissionBody & {
}
export type FormLoadedDto = {
+ featureFlags: string[]
formDef: IPopulatedForm
encryptedFormDef: IPopulatedEncryptedForm
}
diff --git a/src/types/form.ts b/src/types/form.ts
index 3e2993e065..b4bf67633b 100644
--- a/src/types/form.ts
+++ b/src/types/form.ts
@@ -112,8 +112,6 @@ export interface IFormSchema extends IForm, Document, PublicView {
created?: Date
lastModified?: Date
- encryptionBoundaryShift?: boolean
-
/**
* Replaces the field corresponding to given id to given new field
* @param fieldId the id of the field to update
@@ -304,7 +302,6 @@ export interface IEmailForm extends IForm {
// strings to string array.
emails: string[] | string
publicKey?: never
- encryptionBoundaryShift?: never
}
export type IEmailFormSchema = IEmailForm & IFormSchema
From af594bb0a86a1986b3b0b7da910fb0c47f629ec7 Mon Sep 17 00:00:00 2001
From: Ken Lee Shu Ming
Date: Thu, 31 Aug 2023 17:40:51 +0800
Subject: [PATCH 2/6] feat: payment security account email check (#6604)
* test: add cases for failed whitelisting domain
* feat: add agency domain check when connecting stripe account
* feat: add feature flag to allow release in dark
* test: add cases to test for disabled fetaure flag
* chore: update comments for clarity
* chore: remove unused imports
* refactor: use guard clause for flag checks
* refcator: rename defaultvalue to fallback as its is applied during errors
* feat: add be to redirect with query params when domain check fails
* feat: add fe to show alert when non-whitelisted domain error is returned
* chore: fix typo on whitelisting
* Update src/app/modules/payments/__tests__/stripe.service.spec.ts
Co-authored-by: Lin Huiqing <37061143+LinHuiqing@users.noreply.github.com>
---------
Co-authored-by: Lin Huiqing <37061143+LinHuiqing@users.noreply.github.com>
---
.../PaymentSettingsSection.tsx | 50 ++++++--
shared/constants/errors.ts | 4 +
shared/constants/feature-flags.ts | 1 +
shared/constants/index.ts | 1 +
.../feature-flags/feature-flags.service.ts | 49 ++++++-
.../payments/__tests__/stripe.service.spec.ts | 121 +++++++++++++++++-
src/app/modules/payments/stripe.controller.ts | 13 ++
src/app/modules/payments/stripe.service.ts | 87 ++++++++++---
8 files changed, 293 insertions(+), 33 deletions(-)
create mode 100644 shared/constants/errors.ts
diff --git a/frontend/src/features/admin-form/settings/components/PaymentSettingsSection/PaymentSettingsSection.tsx b/frontend/src/features/admin-form/settings/components/PaymentSettingsSection/PaymentSettingsSection.tsx
index 02eca3cc8f..f916b41b61 100644
--- a/frontend/src/features/admin-form/settings/components/PaymentSettingsSection/PaymentSettingsSection.tsx
+++ b/frontend/src/features/admin-form/settings/components/PaymentSettingsSection/PaymentSettingsSection.tsx
@@ -1,4 +1,5 @@
import { useState } from 'react'
+import { useSearchParams } from 'react-router-dom'
import {
As,
Divider,
@@ -10,6 +11,10 @@ import {
VStack,
} from '@chakra-ui/react'
+import {
+ DISALLOW_CONNECT_NON_WHITELIST_STRIPE_ACCOUNT,
+ ERROR_QUERY_PARAM_KEY,
+} from '~shared/constants'
import { FormResponseMode, PaymentChannel } from '~shared/types'
import { BxsCheckCircle, BxsError, BxsInfoCircle } from '~assets/icons'
@@ -39,6 +44,27 @@ const BeforeConnectionInstructions = ({
}): JSX.Element => {
const [allowConnect, setAllowConnect] = useState(false)
const { data: paymentGuideLink } = usePaymentGuideLink()
+ const [searchParams] = useSearchParams()
+
+ const queryParams = Object.fromEntries([...searchParams])
+ const isInvalidDomain =
+ queryParams[ERROR_QUERY_PARAM_KEY] ===
+ DISALLOW_CONNECT_NON_WHITELIST_STRIPE_ACCOUNT
+
+ if (isInvalidDomain) {
+ return (
+ <>
+
+
+ Your Stripe account could not be connected because it was created
+ with a non-whitelisted email domain. Try reconnecting an account
+ that was created with a whitelisted email domain.
+
+
+
+
+ )
+ }
if (isProductionEnv) {
return (
@@ -84,19 +110,19 @@ const BeforeConnectionInstructions = ({
/>
)
- } else {
- return (
- <>
-
-
- You are currently in test mode. You can choose to skip connecting a
- Stripe account after clicking the button below.
-
-
-
-
- )
}
+
+ return (
+ <>
+
+
+ You are currently in test mode. You can choose to skip connecting a
+ Stripe account after clicking the button below.
+
+
+
+
+ )
}
const ConnectionStatusText = ({
diff --git a/shared/constants/errors.ts b/shared/constants/errors.ts
new file mode 100644
index 0000000000..cef0710f81
--- /dev/null
+++ b/shared/constants/errors.ts
@@ -0,0 +1,4 @@
+export const ERROR_QUERY_PARAM_KEY = 'error_type'
+
+// Payment errors namespace (100xxx)
+export const DISALLOW_CONNECT_NON_WHITELIST_STRIPE_ACCOUNT = '100001'
diff --git a/shared/constants/feature-flags.ts b/shared/constants/feature-flags.ts
index bd0267c58f..42f8171758 100644
--- a/shared/constants/feature-flags.ts
+++ b/shared/constants/feature-flags.ts
@@ -2,5 +2,6 @@ export const featureFlags = {
payment: 'payment' as const,
goLinks: 'goLinks' as const,
turnstile: 'turnstile' as const,
+ validateStripeEmailDomain: 'validateStripeEmailDomain' as const,
encryptionBoundaryShift: 'encryption-boundary-shift' as const,
}
diff --git a/shared/constants/index.ts b/shared/constants/index.ts
index 552e860119..41da90f12e 100644
--- a/shared/constants/index.ts
+++ b/shared/constants/index.ts
@@ -2,4 +2,5 @@ export * from './file'
export * from './form'
export * from './links'
export * from './feature-flags'
+export * from './errors'
export * from './dates'
diff --git a/src/app/modules/feature-flags/feature-flags.service.ts b/src/app/modules/feature-flags/feature-flags.service.ts
index 063bf9d619..33d906d87a 100644
--- a/src/app/modules/feature-flags/feature-flags.service.ts
+++ b/src/app/modules/feature-flags/feature-flags.service.ts
@@ -1,7 +1,8 @@
import mongoose from 'mongoose'
import { okAsync, ResultAsync } from 'neverthrow'
-import { createLoggerWithLabel } from '../../config/logger'
+import { featureFlags } from '../../../../shared/constants'
+import { createLoggerWithLabel, CustomLoggerParams } from '../../config/logger'
import getFeatureFlagModel from '../../models/feature_flag.server.model'
import { DatabaseError } from '../core/core.errors'
@@ -23,3 +24,49 @@ export const getEnabledFlags = (): ResultAsync => {
okAsync(enabledFlagsDocs.map((doc) => doc.name)),
)
}
+
+/**
+ * Wrapper over getEnabledFlags function to gracefully handle errors
+ * and returns the `options.fallbackValue` instead.
+ *
+ * @example
+ * ```
+ * const logMeta = {
+ * action: 'linkStripeAccountToForm',
+ * }
+ * return getFeatureFlag(featureFlags.validateStripeEmailDomain, {
+ * fallbackValue: false,
+ * logMeta,
+ * })
+ *.andThen((shouldValidateStripeEmailDomain) => ...)
+ *```
+ *
+ * @param flag
+ * @param options.fallbackValue the value to fall back to in the event that there's an error retrieving the feature flag
+ * @returns boolean that represents the status of the feature flag or the fallback value
+ */
+export const getFeatureFlag = (
+ flag: keyof typeof featureFlags,
+ options?: {
+ fallbackValue?: boolean
+ logMeta?: CustomLoggerParams['meta']
+ },
+): ResultAsync => {
+ const _fallbackValue = options?.fallbackValue ?? false
+ return getEnabledFlags()
+ .andThen((featureFlagsListResult) =>
+ okAsync(featureFlagsListResult.includes(flag)),
+ )
+ .orElse((error) => {
+ logger.error({
+ message: 'Error occurred whilst retrieving enabled feature flags',
+ meta: options?.logMeta
+ ? options.logMeta
+ : {
+ action: 'getFeatureFlag',
+ },
+ error,
+ })
+ return okAsync(_fallbackValue)
+ })
+}
diff --git a/src/app/modules/payments/__tests__/stripe.service.spec.ts b/src/app/modules/payments/__tests__/stripe.service.spec.ts
index f0f1c9f826..404ecbf4bd 100644
--- a/src/app/modules/payments/__tests__/stripe.service.spec.ts
+++ b/src/app/modules/payments/__tests__/stripe.service.spec.ts
@@ -19,6 +19,9 @@ import {
IPopulatedUser,
} from 'src/types'
+import { InvalidDomainError } from '../../auth/auth.errors'
+import * as AuthService from '../../auth/auth.service'
+import * as FeatureFlagService from '../../feature-flags/feature-flags.service'
import { PaymentNotFoundError } from '../payments.errors'
import * as PaymentsService from '../payments.service'
import { StripeMetadataInvalidError } from '../stripe.errors'
@@ -689,7 +692,11 @@ describe('stripe.service', () => {
})
})
describe('linkStripeAccountToForm', () => {
- it('should call func to attach payment account information', async () => {
+ beforeEach(() => {
+ jest.restoreAllMocks()
+ })
+
+ it('should attach payment account information for new accounts', async () => {
// Arrange
await dbHandler.insertFormCollectionReqs({
userId: MOCK_USER_ID,
@@ -701,8 +708,22 @@ describe('stripe.service', () => {
})
.populate('admin')
.execPopulate()) as IPopulatedEncryptedForm
- const spiedFn = jest.spyOn(mockForm, 'addPaymentAccountId')
- const expectedAccountId = 'accountId'
+
+ const getFeatureFlagSpy = jest
+ .spyOn(FeatureFlagService, 'getFeatureFlag')
+ .mockReturnValueOnce(okAsync(true))
+ const addPaymentAccountIdSpy = jest.spyOn(mockForm, 'addPaymentAccountId')
+ const expectedAccountId = 'newAccountId'
+ const stripeAccountsRetrieveApiSpy = jest
+ .spyOn(stripe.accounts, 'retrieve')
+ .mockReturnValueOnce(
+ Promise.resolve({
+ email: 'mockEmail',
+ } as unknown as Stripe.Response),
+ )
+ const authServiceSpy = jest
+ .spyOn(AuthService, 'validateEmailDomain')
+ .mockReturnValue(okAsync(true) as any)
// Act
const result = await StripeService.linkStripeAccountToForm(mockForm, {
@@ -711,7 +732,10 @@ describe('stripe.service', () => {
})
// Assert
- expect(spiedFn).toHaveBeenCalledTimes(1)
+ expect(getFeatureFlagSpy).toHaveBeenCalledTimes(1)
+ expect(stripeAccountsRetrieveApiSpy).toHaveBeenCalledTimes(1)
+ expect(authServiceSpy).toHaveBeenCalledTimes(1)
+ expect(addPaymentAccountIdSpy).toHaveBeenCalledTimes(1)
expect(result._unsafeUnwrap()).toBe(expectedAccountId)
})
@@ -724,12 +748,26 @@ describe('stripe.service', () => {
})
.populate('admin')
.execPopulate()) as IPopulatedEncryptedForm
+
+ const getFeatureFlagSpy = jest
+ .spyOn(FeatureFlagService, 'getFeatureFlag')
+ .mockReturnValueOnce(okAsync(true))
const expectedAccountId = 'existingAccountId'
mockForm.payments_channel = {
target_account_id: expectedAccountId,
channel: PaymentChannel.Stripe,
publishable_key: 'publishableKey',
}
+ const stripeAccountsRetrieveApiSpy = jest
+ .spyOn(stripe.accounts, 'retrieve')
+ .mockReturnValueOnce(
+ Promise.resolve({
+ email: 'mockEmail',
+ } as unknown as Stripe.Response),
+ )
+ const authServiceSpy = jest
+ .spyOn(AuthService, 'validateEmailDomain')
+ .mockReturnValue(okAsync(true) as any)
// Act
const result = await StripeService.linkStripeAccountToForm(mockForm, {
@@ -738,7 +776,82 @@ describe('stripe.service', () => {
})
// Assert
+ expect(getFeatureFlagSpy).toHaveBeenCalledTimes(1)
expect(result._unsafeUnwrap()).toBe(expectedAccountId)
+ expect(stripeAccountsRetrieveApiSpy).toHaveBeenCalledTimes(1)
+ expect(authServiceSpy).toHaveBeenCalledTimes(1)
+ })
+
+ it('should not connect when stripe account email is not whitelisted', async () => {
+ // Arrange
+ const mockForm = (await new EncryptedForm({
+ admin: MOCK_USER,
+ title: 'Test Form',
+ publicKey: 'mockPublicKey',
+ }).execPopulate()) as IPopulatedEncryptedForm
+ const addPaymentAccountIdSpy = jest.spyOn(mockForm, 'addPaymentAccountId')
+
+ const getFeatureFlagSpy = jest
+ .spyOn(FeatureFlagService, 'getFeatureFlag')
+ .mockReturnValueOnce(okAsync(true))
+ const stripeAccountsRetrieveApiSpy = jest
+ .spyOn(stripe.accounts, 'retrieve')
+ .mockReturnValueOnce(
+ Promise.resolve({
+ email: 'mockEmail',
+ } as unknown as Stripe.Response),
+ )
+ const authServiceSpy = jest
+ .spyOn(AuthService, 'validateEmailDomain')
+ .mockReturnValue(errAsync(new InvalidDomainError()))
+
+ // Act
+ const result = await StripeService.linkStripeAccountToForm(mockForm, {
+ accountId: 'accountId',
+ publishableKey: 'publishableKey',
+ })
+
+ // Assert
+ expect(getFeatureFlagSpy).toHaveBeenCalledTimes(1)
+ expect(stripeAccountsRetrieveApiSpy).toHaveBeenCalledTimes(1)
+ expect(authServiceSpy).toHaveBeenCalledTimes(1)
+ expect(addPaymentAccountIdSpy).toHaveBeenCalledTimes(0)
+ expect(result.isErr()).toBeTrue()
+ expect(result._unsafeUnwrapErr()).toBeInstanceOf(InvalidDomainError)
+ })
+
+ it('should not check email whitelisting if feature flag is disabled', async () => {
+ // Arrange
+ const mockForm = (await new EncryptedForm({
+ admin: MOCK_USER,
+ title: 'Test Form',
+ publicKey: 'mockPublicKey',
+ })
+ .populate('admin')
+ .execPopulate()) as IPopulatedEncryptedForm
+
+ const getFeatureFlagSpy = jest
+ .spyOn(FeatureFlagService, 'getFeatureFlag')
+ .mockReturnValueOnce(okAsync(false))
+ const addPaymentAccountIdSpy = jest.spyOn(mockForm, 'addPaymentAccountId')
+ const stripeAccountsRetrieveApiSpy = jest.spyOn(
+ stripe.accounts,
+ 'retrieve',
+ )
+ const authServiceSpy = jest.spyOn(AuthService, 'validateEmailDomain')
+
+ // Act
+ const result = await StripeService.linkStripeAccountToForm(mockForm, {
+ accountId: 'accountId',
+ publishableKey: 'publishableKey',
+ })
+
+ // Assert
+ expect(getFeatureFlagSpy).toHaveBeenCalledTimes(1)
+ expect(stripeAccountsRetrieveApiSpy).toHaveBeenCalledTimes(0)
+ expect(authServiceSpy).toHaveBeenCalledTimes(0)
+ expect(addPaymentAccountIdSpy).toHaveBeenCalledTimes(1)
+ expect(result.isOk()).toBeTrue()
})
})
diff --git a/src/app/modules/payments/stripe.controller.ts b/src/app/modules/payments/stripe.controller.ts
index 868145668a..ca05057d78 100644
--- a/src/app/modules/payments/stripe.controller.ts
+++ b/src/app/modules/payments/stripe.controller.ts
@@ -3,8 +3,13 @@
import { celebrate, Joi, Segments } from 'celebrate'
import { StatusCodes } from 'http-status-codes'
import { errAsync, okAsync, ResultAsync } from 'neverthrow'
+import querystring from 'querystring'
import Stripe from 'stripe'
+import {
+ DISALLOW_CONNECT_NON_WHITELIST_STRIPE_ACCOUNT,
+ ERROR_QUERY_PARAM_KEY,
+} from '../../../../shared/constants'
import {
ErrorDto,
GetPaymentInfoDto,
@@ -16,6 +21,7 @@ import config from '../../config/config'
import { createLoggerWithLabel } from '../../config/logger'
import { stripe } from '../../loaders/stripe'
import { createReqMeta } from '../../utils/request'
+import { InvalidDomainError } from '../auth/auth.errors'
import { ControllerHandler } from '../core/core.types'
import * as FormService from '../form/form.service'
import * as PendingSubmissionModel from '../pending-submission/pending-submission.service'
@@ -180,6 +186,13 @@ const _handleConnectOauthCallback: ControllerHandler<
},
error,
})
+ if (error.constructor === InvalidDomainError) {
+ const queryString = querystring.stringify({
+ [ERROR_QUERY_PARAM_KEY]:
+ DISALLOW_CONNECT_NON_WHITELIST_STRIPE_ACCOUNT,
+ })
+ return res.redirect(redirectUrl + '?' + queryString)
+ }
return res.redirect(redirectUrl)
})
)
diff --git a/src/app/modules/payments/stripe.service.ts b/src/app/modules/payments/stripe.service.ts
index 20cff044fd..aa3d8ef335 100644
--- a/src/app/modules/payments/stripe.service.ts
+++ b/src/app/modules/payments/stripe.service.ts
@@ -8,6 +8,7 @@ import Stripe from 'stripe'
import { MarkRequired } from 'ts-essentials'
import isURL from 'validator/lib/isURL'
+import { featureFlags } from '../../../../shared/constants'
import {
PaymentStatus,
ReconciliationReportLine,
@@ -27,7 +28,10 @@ import {
getMongoErrorMessage,
transformMongoError,
} from '../../utils/handle-mongo-error'
+import { InvalidDomainError } from '../auth/auth.errors'
+import * as AuthService from '../auth/auth.service'
import { DatabaseError, DatabaseWriteConflictError } from '../core/core.errors'
+import { getFeatureFlag } from '../feature-flags/feature-flags.service'
import { FormNotFoundError } from '../form/form.errors'
import {
PendingSubmissionNotFoundError,
@@ -672,24 +676,75 @@ export const linkStripeAccountToForm = (
accountId: string
publishableKey: string
},
-): ResultAsync =>
- ResultAsync.fromPromise(
- form.addPaymentAccountId({ accountId, publishableKey }),
- (error) => {
- const errMsg = 'Failed to update payment account id'
- logger.error({
- message: errMsg,
- meta: {
- action: 'linkStripeAccountToForm',
- stripeAccountId: accountId,
- stripePublishableKey: publishableKey,
- formId: form._id,
+): ResultAsync<
+ string,
+ DatabaseError | StripeFetchError | StripeAccountError | InvalidDomainError
+> => {
+ const logMeta = {
+ action: 'linkStripeAccountToForm',
+ stripeAccountId: accountId,
+ stripePublishableKey: publishableKey,
+ formId: form._id,
+ }
+
+ return getFeatureFlag(featureFlags.validateStripeEmailDomain, {
+ // If getFeatureFlag throws a DatabaseError, we want to log it, but respond
+ // to the client as if the flag is not found.
+ fallbackValue: false,
+ logMeta,
+ })
+ .andThen((shouldValidateStripeEmailDomain) => {
+ if (!shouldValidateStripeEmailDomain) {
+ // skip validation
+ return okAsync(undefined)
+ }
+
+ return ResultAsync.fromPromise(
+ stripe.accounts.retrieve(accountId),
+ (error) => {
+ logger.error({
+ message: 'Error retriving Stripe account',
+ meta: {
+ action: 'linkStripeAccountToForm',
+ stripeAccountId: accountId,
+ },
+ error,
+ })
+ return new StripeFetchError(String(error))
},
- error,
+ ).andThen((account) => {
+ // Check if the email domain is whitelisted
+ if (!account.email) {
+ logger.error({
+ message: 'Error retriving Stripe account email',
+ meta: {
+ action: 'linkStripeAccountToForm',
+ stripeAccountId: accountId,
+ account,
+ },
+ })
+ return errAsync(
+ new StripeAccountError('Stripe account email is missing'),
+ )
+ }
+ return AuthService.validateEmailDomain(account.email)
})
- return new DatabaseError(errMsg)
- },
- ).map((updatedForm) => updatedForm.payments_channel.target_account_id)
+ })
+ .andThen(() =>
+ ResultAsync.fromPromise(
+ form.addPaymentAccountId({ accountId, publishableKey }),
+ (error) => {
+ const errMsg = 'Failed to update payment account id'
+ logger.error({
+ message: errMsg,
+ meta: logMeta,
+ error,
+ })
+ return new DatabaseError(errMsg)
+ },
+ ).map((updatedForm) => updatedForm.payments_channel.target_account_id),
+ )
+}
export const unlinkStripeAccountFromForm = (
form: IPopulatedEncryptedForm,
From 808b92308a47b96e9a300f6f171aa21cea5a8185 Mon Sep 17 00:00:00 2001
From: Bryce Goh
Date: Sun, 3 Sep 2023 17:16:51 +0800
Subject: [PATCH 3/6] fix: Find latest successful payment query to sort by
completed payment date and constraint query to last 30 days (#6615)
* fix: change query sort and add date constraint
* test: add cases to test for query sort and date constraint
* fix: standardise to use moment
---
.../__tests__/payments.controller.spec.ts | 83 +++++++++++++++++++
.../__tests__/payments.service.spec.ts | 40 +++++++--
src/app/modules/payments/payments.service.ts | 6 +-
3 files changed, 123 insertions(+), 6 deletions(-)
diff --git a/src/app/modules/payments/__tests__/payments.controller.spec.ts b/src/app/modules/payments/__tests__/payments.controller.spec.ts
index 74f919b95b..901d93149b 100644
--- a/src/app/modules/payments/__tests__/payments.controller.spec.ts
+++ b/src/app/modules/payments/__tests__/payments.controller.spec.ts
@@ -2,6 +2,7 @@ import dbHandler from '__tests__/unit/backend/helpers/jest-db'
import expressHandler from '__tests__/unit/backend/helpers/jest-express'
import { ObjectId } from 'bson'
import { StatusCodes } from 'http-status-codes'
+import moment from 'moment-timezone'
import mongoose from 'mongoose'
import { PaymentStatus } from 'shared/types'
@@ -41,6 +42,7 @@ describe('payments.controller', () => {
email: email,
completedPayment: {
receiptUrl: 'http://form.gov.sg',
+ paymentDate: new Date(),
},
gstEnabled: false,
})
@@ -61,6 +63,87 @@ describe('payments.controller', () => {
expect(mockRes.status).toHaveBeenCalledWith(200)
expect(mockRes.send).toHaveBeenCalledOnce()
})
+ it('should return 200 and the latest payment record id if there are multiple successful payments', async () => {
+ const email = 'formsg@tech.gov.sg'
+ const now = moment().utc()
+
+ // create 2 payments with different payment dates but same email
+ const latestPayment = await Payment.create({
+ formId: MOCK_FORM_ID,
+ targetAccountId: 'acct_MOCK_ACCOUNT_ID',
+ pendingSubmissionId: new ObjectId(),
+ amount: 12345,
+ status: PaymentStatus.Succeeded,
+ paymentIntentId: 'pi_MOCK_PAYMENT_INTENT',
+ email: email,
+ completedPayment: {
+ receiptUrl: 'http://form.gov.sg',
+ paymentDate: now.toDate(),
+ },
+ gstEnabled: false,
+ })
+ await Payment.create({
+ formId: MOCK_FORM_ID,
+ targetAccountId: 'acct_MOCK_ACCOUNT_ID',
+ pendingSubmissionId: new ObjectId(),
+ amount: 12345,
+ status: PaymentStatus.Succeeded,
+ paymentIntentId: 'pi_MOCK_PAYMENT_INTENT',
+ email: email,
+ completedPayment: {
+ receiptUrl: 'http://form.gov.sg',
+ paymentDate: now.subtract(1, 'hour').toDate(),
+ },
+ gstEnabled: false,
+ })
+
+ const mockReq = expressHandler.mockRequest({
+ params: { formId: MOCK_FORM_ID },
+ body: { email },
+ })
+
+ const mockRes = expressHandler.mockResponse()
+
+ await PaymentsController.handleGetPreviousPaymentId(
+ mockReq,
+ mockRes,
+ jest.fn(),
+ )
+ expect(mockRes.status).toHaveBeenCalledWith(200)
+ expect(mockRes.send).toHaveBeenCalledWith(latestPayment._id)
+ })
+ it('should return 404 if there are no successful payments made within the alst 30 days', async () => {
+ const email = 'formsg@tech.gov.sg'
+
+ await Payment.create({
+ formId: MOCK_FORM_ID,
+ targetAccountId: 'acct_MOCK_ACCOUNT_ID',
+ pendingSubmissionId: new ObjectId(),
+ amount: 12345,
+ status: PaymentStatus.Succeeded,
+ paymentIntentId: 'pi_MOCK_PAYMENT_INTENT',
+ email: email,
+ completedPayment: {
+ receiptUrl: 'http://form.gov.sg',
+ paymentDate: moment().subtract(31, 'days').toDate(),
+ },
+ gstEnabled: false,
+ })
+
+ const mockReq = expressHandler.mockRequest({
+ params: { formId: MOCK_FORM_ID },
+ body: { email },
+ })
+
+ const mockRes = expressHandler.mockResponse()
+
+ await PaymentsController.handleGetPreviousPaymentId(
+ mockReq,
+ mockRes,
+ jest.fn(),
+ )
+ expect(mockRes.sendStatus).toHaveBeenCalledWith(404)
+ })
it('should return 404 if there are no previous payments by the specific email', async () => {
const payment = await Payment.create({
formId: MOCK_FORM_ID,
diff --git a/src/app/modules/payments/__tests__/payments.service.spec.ts b/src/app/modules/payments/__tests__/payments.service.spec.ts
index 78ba5e2764..150c26d379 100644
--- a/src/app/modules/payments/__tests__/payments.service.spec.ts
+++ b/src/app/modules/payments/__tests__/payments.service.spec.ts
@@ -1,5 +1,6 @@
import dbHandler from '__tests__/unit/backend/helpers/jest-db'
import { ObjectId } from 'bson'
+import moment from 'moment-timezone'
import mongoose, { Query } from 'mongoose'
import { PaymentStatus } from 'shared/types'
@@ -75,6 +76,7 @@ describe('payments.service', () => {
describe('findLatestSuccessfulPaymentByEmailAndFormId', () => {
const expectedObjectId = new ObjectId()
const email = 'someone@mail.com'
+ const now = moment().utc()
beforeEach(async () => {
await dbHandler.clearCollection(Payment.collection.name)
@@ -88,6 +90,9 @@ describe('payments.service', () => {
email: email,
status: PaymentStatus.Succeeded,
gstEnabled: false,
+ completedPayment: {
+ paymentDate: now.toDate(),
+ },
})
})
afterEach(() => jest.clearAllMocks())
@@ -104,11 +109,9 @@ describe('payments.service', () => {
expect(result.isOk()).toBeTrue()
})
- it('should return the latest payment based on id creation', async () => {
- const latestId = new ObjectId()
- // create the latest payment object
+ it('should return the latest payment based on completed payment date', async () => {
+ // create a payment object with an older successful payment date
await Payment.create({
- _id: latestId,
formId: MOCK_FORM_ID,
targetAccountId: 'acct_MOCK_ACCOUNT_ID',
pendingSubmissionId: new ObjectId(),
@@ -117,7 +120,11 @@ describe('payments.service', () => {
email: email,
status: PaymentStatus.Succeeded,
gstEnabled: false,
+ completedPayment: {
+ paymentDate: now.subtract(1, 'days').toDate(),
+ },
})
+
const result =
await PaymentsService.findLatestSuccessfulPaymentByEmailAndFormId(
email,
@@ -126,7 +133,7 @@ describe('payments.service', () => {
// Assert latest payment document
result.map((payment) => {
- expect(payment.id).toEqual(latestId.toHexString())
+ expect(payment.id).toEqual(expectedObjectId.toHexString())
})
// Assert
@@ -175,6 +182,29 @@ describe('payments.service', () => {
)
expect(result.isErr()).toBeTrue()
})
+ it('should return with error if there is no successful payments for the email and formId within the last 30 days', async () => {
+ const newEmail = 'new@new.com'
+ const newFormId = new ObjectId().toHexString()
+ await Payment.create({
+ formId: newFormId,
+ targetAccountId: 'acct_MOCK_ACCOUNT_ID',
+ pendingSubmissionId: new ObjectId(),
+ paymentIntentId: 'somePaymentIntentId',
+ amount: 314159,
+ email: newEmail,
+ status: PaymentStatus.Succeeded,
+ gstEnabled: false,
+ completedPayment: {
+ paymentDate: moment().subtract(31, 'days').utc().toDate(),
+ },
+ })
+ const result =
+ await PaymentsService.findLatestSuccessfulPaymentByEmailAndFormId(
+ newEmail,
+ newFormId,
+ )
+ expect(result.isErr()).toBeTrue()
+ })
it('should return with error if mongodb is not ready', async () => {
await dbHandler.closeDatabase()
diff --git a/src/app/modules/payments/payments.service.ts b/src/app/modules/payments/payments.service.ts
index bf02271be6..350f2cabd0 100644
--- a/src/app/modules/payments/payments.service.ts
+++ b/src/app/modules/payments/payments.service.ts
@@ -1,3 +1,4 @@
+import moment from 'moment-timezone'
import { ObjectId } from 'mongodb'
import mongoose from 'mongoose'
import { errAsync, okAsync, ResultAsync } from 'neverthrow'
@@ -312,8 +313,11 @@ export const findLatestSuccessfulPaymentByEmailAndFormId = (
email: email,
formId: formId,
status: PaymentStatus.Succeeded,
+ 'completedPayment.paymentDate': {
+ $gt: moment().subtract(30, 'days').utc().toDate(),
+ },
})
- .sort({ _id: -1 })
+ .sort({ 'completedPayment.paymentDate': -1 })
.exec(),
(error) => {
logger.error({
From 6ba61554e3f9c83b27eb1aea8df5ddd241afe852 Mon Sep 17 00:00:00 2001
From: LinHuiqing
Date: Mon, 4 Sep 2023 07:48:13 +0800
Subject: [PATCH 4/6] chore: bump version to v6.75.0
---
CHANGELOG.md | 11 +++++++++++
frontend/package-lock.json | 4 ++--
frontend/package.json | 2 +-
package-lock.json | 4 ++--
package.json | 2 +-
5 files changed, 17 insertions(+), 6 deletions(-)
diff --git a/CHANGELOG.md b/CHANGELOG.md
index e3fa896f40..56133c2f0f 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -4,9 +4,20 @@ All notable changes to this project will be documented in this file. Dates are d
Generated by [`auto-changelog`](https://github.com/CookPete/auto-changelog).
+#### [v6.75.0](https://github.com/opengovsg/FormSG/compare/v6.74.1...v6.75.0)
+
+- fix: Find latest successful payment query to sort by completed payment date and constraint query to last 30 days [`#6615`](https://github.com/opengovsg/FormSG/pull/6615)
+- feat: payment security account email check [`#6604`](https://github.com/opengovsg/FormSG/pull/6604)
+- feat: % rollout of new storage submission endpoint [`#6665`](https://github.com/opengovsg/FormSG/pull/6665)
+- build: merge v6.74.1 into develop [`#6683`](https://github.com/opengovsg/FormSG/pull/6683)
+- build: release v6.74.1 [`#6682`](https://github.com/opengovsg/FormSG/pull/6682)
+
#### [v6.74.1](https://github.com/opengovsg/FormSG/compare/v6.74.0...v6.74.1)
+> 31 August 2023
+
- fix: payment quantity modal input inconsistencies [`#6681`](https://github.com/opengovsg/FormSG/pull/6681)
+- chore: bump version to 6.74.1 [`1c96e9e`](https://github.com/opengovsg/FormSG/commit/1c96e9ea74c772e8cd45bc6877dfc8b1e41e4832)
#### [v6.74.0](https://github.com/opengovsg/FormSG/compare/v6.73.0...v6.74.0)
diff --git a/frontend/package-lock.json b/frontend/package-lock.json
index ae5eccdd7b..1b8bf31768 100644
--- a/frontend/package-lock.json
+++ b/frontend/package-lock.json
@@ -1,12 +1,12 @@
{
"name": "form-frontend",
- "version": "6.74.0",
+ "version": "6.75.0",
"lockfileVersion": 2,
"requires": true,
"packages": {
"": {
"name": "form-frontend",
- "version": "6.74.0",
+ "version": "6.75.0",
"hasInstallScript": true,
"dependencies": {
"@chakra-ui/react": "^1.8.6",
diff --git a/frontend/package.json b/frontend/package.json
index 84dfe1fb17..277aa7cf0b 100644
--- a/frontend/package.json
+++ b/frontend/package.json
@@ -1,6 +1,6 @@
{
"name": "form-frontend",
- "version": "6.74.0",
+ "version": "6.75.0",
"homepage": ".",
"private": true,
"dependencies": {
diff --git a/package-lock.json b/package-lock.json
index 0da72946e2..f16a6bb582 100644
--- a/package-lock.json
+++ b/package-lock.json
@@ -1,12 +1,12 @@
{
"name": "FormSG",
- "version": "6.74.1",
+ "version": "6.75.0",
"lockfileVersion": 2,
"requires": true,
"packages": {
"": {
"name": "FormSG",
- "version": "6.74.1",
+ "version": "6.75.0",
"hasInstallScript": true,
"dependencies": {
"@aws-sdk/client-cloudwatch-logs": "^3.347.1",
diff --git a/package.json b/package.json
index 8bb929f43e..93505b32c0 100644
--- a/package.json
+++ b/package.json
@@ -1,7 +1,7 @@
{
"name": "FormSG",
"description": "Form Manager for Government",
- "version": "6.74.1",
+ "version": "6.75.0",
"homepage": "https://form.gov.sg",
"authors": [
"FormSG "
From a5749937952c1d70a16177114339e5cc0f4ded89 Mon Sep 17 00:00:00 2001
From: Lin Huiqing <37061143+LinHuiqing@users.noreply.github.com>
Date: Mon, 4 Sep 2023 13:12:57 +0800
Subject: [PATCH 5/6] fix: payments params in new storage submission endpoint
(#6689)
* fix: joi validation for new storage endpoint
* fix: pass payments params for new endpoint
---
.../encrypt-submission.middleware.ts | 18 ++++++++++++++++++
.../modules/submission/submission.constants.ts | 9 ---------
src/types/api/encrypt_submission.ts | 5 +++++
3 files changed, 23 insertions(+), 9 deletions(-)
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 c46cb3222f..0732a944ac 100644
--- a/src/app/modules/submission/encrypt-submission/encrypt-submission.middleware.ts
+++ b/src/app/modules/submission/encrypt-submission/encrypt-submission.middleware.ts
@@ -119,6 +119,21 @@ export const validateEncryptSubmissionParams = celebrate({
export const validateStorageSubmissionParams = celebrate({
[Segments.BODY]: Joi.object({
...sharedSubmissionParams,
+ paymentProducts: Joi.array().items(
+ Joi.object().keys({
+ data: JoiPaymentProduct.required(),
+ selected: Joi.boolean(),
+ quantity: JoiInt.positive().required(),
+ }),
+ ),
+ paymentReceiptEmail: Joi.string(),
+ payments: Joi.object({
+ amount_cents: Joi.number()
+ .integer()
+ .positive()
+ .min(paymentConfig.minPaymentAmountCents)
+ .max(paymentConfig.maxPaymentAmountCents),
+ }),
version: Joi.number().required(),
}),
})
@@ -307,6 +322,9 @@ export const encryptSubmission = async (
responses: filteredResponses.value as EncryptFormFieldResponse[],
encryptedContent,
version: req.body.version,
+ paymentProducts: req.body.paymentProducts,
+ paymentReceiptEmail: req.body.paymentReceiptEmail,
+ payments: req.body.payments,
}
return next()
diff --git a/src/app/modules/submission/submission.constants.ts b/src/app/modules/submission/submission.constants.ts
index d76fd4b57e..6e566e13e9 100644
--- a/src/app/modules/submission/submission.constants.ts
+++ b/src/app/modules/submission/submission.constants.ts
@@ -1,7 +1,6 @@
import { Joi } from 'celebrate'
import { BasicField } from '../../../../shared/types'
-import { paymentConfig } from '../../config/features/payment.config'
export const sharedSubmissionParams = {
responses: Joi.array()
@@ -25,14 +24,6 @@ export const sharedSubmissionParams = {
.with('filename', 'content'), // if filename is present, content must be present
)
.required(),
- paymentReceiptEmail: Joi.string(),
- payments: Joi.object({
- amount_cents: Joi.number()
- .integer()
- .positive()
- .min(paymentConfig.minPaymentAmountCents)
- .max(paymentConfig.maxPaymentAmountCents),
- }),
responseMetadata: Joi.object({
responseTimeMs: Joi.number(),
numVisibleFields: Joi.number(),
diff --git a/src/types/api/encrypt_submission.ts b/src/types/api/encrypt_submission.ts
index 0a97d9c5b5..521e750ec1 100644
--- a/src/types/api/encrypt_submission.ts
+++ b/src/types/api/encrypt_submission.ts
@@ -3,6 +3,8 @@ import type { Merge } from 'type-fest'
import {
AttachmentResponse,
FieldResponse,
+ PaymentFieldsDto,
+ ProductItem,
StorageModeSubmissionContentDto,
} from '../../../shared/types'
import { IPopulatedEncryptedForm, IPopulatedForm } from '../form'
@@ -28,6 +30,9 @@ export type EncryptFormFieldResponse =
* ReceiverMiddleware.receiveStorageSubmission middleware.
*/
export type ParsedStorageModeSubmissionBody = ParsedEmailModeSubmissionBody & {
+ paymentProducts?: Array
+ paymentReceiptEmail?: string
+ payments?: PaymentFieldsDto
version: number
}
From af06ba371fee38a26f1b27125e382521a0748dbe Mon Sep 17 00:00:00 2001
From: LinHuiqing
Date: Mon, 4 Sep 2023 13:17:00 +0800
Subject: [PATCH 6/6] chore: bump version to 6.75.1
---
CHANGELOG.md | 7 +++++++
package-lock.json | 4 ++--
package.json | 2 +-
3 files changed, 10 insertions(+), 3 deletions(-)
diff --git a/CHANGELOG.md b/CHANGELOG.md
index 56133c2f0f..43e0360b1e 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -4,13 +4,20 @@ All notable changes to this project will be documented in this file. Dates are d
Generated by [`auto-changelog`](https://github.com/CookPete/auto-changelog).
+#### [v6.75.1](https://github.com/opengovsg/FormSG/compare/v6.75.0...v6.75.1)
+
+- fix: payments params in new storage submission endpoint [`#6689`](https://github.com/opengovsg/FormSG/pull/6689)
+
#### [v6.75.0](https://github.com/opengovsg/FormSG/compare/v6.74.1...v6.75.0)
+> 4 September 2023
+
- fix: Find latest successful payment query to sort by completed payment date and constraint query to last 30 days [`#6615`](https://github.com/opengovsg/FormSG/pull/6615)
- feat: payment security account email check [`#6604`](https://github.com/opengovsg/FormSG/pull/6604)
- feat: % rollout of new storage submission endpoint [`#6665`](https://github.com/opengovsg/FormSG/pull/6665)
- build: merge v6.74.1 into develop [`#6683`](https://github.com/opengovsg/FormSG/pull/6683)
- build: release v6.74.1 [`#6682`](https://github.com/opengovsg/FormSG/pull/6682)
+- chore: bump version to v6.75.0 [`6ba6155`](https://github.com/opengovsg/FormSG/commit/6ba61554e3f9c83b27eb1aea8df5ddd241afe852)
#### [v6.74.1](https://github.com/opengovsg/FormSG/compare/v6.74.0...v6.74.1)
diff --git a/package-lock.json b/package-lock.json
index f16a6bb582..563c749ee4 100644
--- a/package-lock.json
+++ b/package-lock.json
@@ -1,12 +1,12 @@
{
"name": "FormSG",
- "version": "6.75.0",
+ "version": "6.75.1",
"lockfileVersion": 2,
"requires": true,
"packages": {
"": {
"name": "FormSG",
- "version": "6.75.0",
+ "version": "6.75.1",
"hasInstallScript": true,
"dependencies": {
"@aws-sdk/client-cloudwatch-logs": "^3.347.1",
diff --git a/package.json b/package.json
index 93505b32c0..e52334b99f 100644
--- a/package.json
+++ b/package.json
@@ -1,7 +1,7 @@
{
"name": "FormSG",
"description": "Form Manager for Government",
- "version": "6.75.0",
+ "version": "6.75.1",
"homepage": "https://form.gov.sg",
"authors": [
"FormSG "