From 3e8b06ab542776ec9503746af1b825cf4b7eeb0a Mon Sep 17 00:00:00 2001 From: wanlingt <56983748+wanlingt@users.noreply.github.com> Date: Wed, 15 Nov 2023 15:10:31 +0800 Subject: [PATCH] feat: myinfo for storage-mode (#6870) * feat: allow myinfo field to be added to storage-mode form * feat: remove FormAuthType.MyInfo check from submitEncryptModeForm * fix: add MyInfo case for authType validator * fix: use set instead of validator for form.server.model * feat: validate myinfo response in backend * fix: clean up imports and comments * fix: refine error * test: remove MyInfo test * test: add myinfo e2e test * feat: add sgid myinfo * ref: refactor authtypes * fix: remove console.log * feat: allow storage mode in myinfo form duplication * ref: remove unused declaration * feat: process each child subfield as a new field * feat: rename session cookie to formsg.connect.sid in local dev * feat: remove containsMyInfoFields from create and dupe form wizards * ref: use common FORM_AUTHTYPES for email and storage mode forms * fix: use triple = for authType check * feat: remove check for authType in validation * test: add MyInfo and SGID MyInfo test cases * fix: use ADMIN_LOGIN_SESSION_COOKIE_NAME constant for tests * ref: extract common functions getMyInfoPrefix and getAnswersForChild to shared submission.utils * fix: error handling in validateStorageSubmission * fix: clean up constants and unused imports --- __tests__/e2e/encrypt-submission.spec.ts | 37 ++++++++- __tests__/integration/helpers/express-auth.ts | 5 +- .../integration/helpers/express-setup.ts | 2 +- .../field-panels/MyInfoPanel.tsx | 23 +----- .../AuthSettingsSection.tsx | 23 +++--- .../AuthSettingsSection/constants.ts | 25 ++---- .../UseTemplateWizardProvider.tsx | 20 +---- .../CreateFormDetailsScreen.tsx | 15 +--- .../FormResponseOptions.tsx | 4 +- .../CreateFormWizardContext.tsx | 1 - .../CreateFormWizardProvider.tsx | 2 - .../DupeFormWizardProvider.tsx | 13 +--- shared/types/response.ts | 11 +++ src/app/loaders/express/session.ts | 7 +- .../__tests__/form.server.model.spec.ts | 33 +++++--- src/app/models/field/baseField.ts | 13 +--- src/app/models/form.server.model.ts | 20 +---- src/app/modules/auth/auth.controller.ts | 3 +- .../email-submission.types.ts | 5 ++ .../email-submission/email-submission.util.ts | 65 +--------------- .../encrypt-submission.controller.ts | 59 ++++++++++---- .../encrypt-submission.middleware.ts | 76 ++++++++++++++++++- .../encrypt-submission.utils.ts | 28 +++++++ .../modules/submission/submission.utils.ts | 73 +++++++++++++++++- .../verified-content.service.ts | 2 + .../verified-content.types.ts | 7 +- .../api/v3/auth/__tests__/auth.routes.spec.ts | 11 +-- 27 files changed, 348 insertions(+), 235 deletions(-) diff --git a/__tests__/e2e/encrypt-submission.spec.ts b/__tests__/e2e/encrypt-submission.spec.ts index 4d42ce557d..8b6a65fadc 100644 --- a/__tests__/e2e/encrypt-submission.spec.ts +++ b/__tests__/e2e/encrypt-submission.spec.ts @@ -1,6 +1,11 @@ import mongoose from 'mongoose' import { featureFlags } from 'shared/constants/feature-flags' -import { BasicField, FormResponseMode } from 'shared/types' +import { + BasicField, + FormAuthType, + FormResponseMode, + MyInfoAttribute, +} from 'shared/types' import { IFeatureFlagModel, IFormModel } from 'src/types' @@ -19,6 +24,7 @@ import { } from './helpers' import { createBlankVersion, + createMyInfoField, createOptionalVersion, deleteDocById, getSettings, @@ -143,4 +149,33 @@ test.describe('Storage form submission', () => { ) await deleteDocById(Form, form._id) }) + + test('Create and submit storage mode form with MyInfo fields', async ({ + page, + }) => { + // Define + const formFields = [ + // Short answer + createMyInfoField(MyInfoAttribute.Name, 'LIM YONG XIANG', true), + // Dropdown + createMyInfoField(MyInfoAttribute.Sex, 'MALE', true), + // Date + createMyInfoField(MyInfoAttribute.DateOfBirth, '06/10/1980', true), + // Mobile + createMyInfoField(MyInfoAttribute.MobileNo, '97399245', false), + // Unverified + createMyInfoField(MyInfoAttribute.WorkpassStatus, 'Live', false), + ] + const formLogics = NO_LOGIC + const formSettings = getSettings({ + authType: FormAuthType.MyInfo, + }) + + // Test + await runEncryptSubmissionTest(page, Form, { + formFields, + formLogics, + formSettings, + }) + }) }) diff --git a/__tests__/integration/helpers/express-auth.ts b/__tests__/integration/helpers/express-auth.ts index d8545efa48..14b4fb27ee 100644 --- a/__tests__/integration/helpers/express-auth.ts +++ b/__tests__/integration/helpers/express-auth.ts @@ -6,6 +6,7 @@ import * as OtpUtils from 'src/app/utils/otp' const MOCK_VALID_OTP = '123456' const MOCK_OTP_PREFIX = 'ABC' +const ADMIN_LOGIN_SESSION_COOKIE_NAME = 'formsg.connect.sid' /** * Integration test helper to create an authenticated session where the user @@ -50,7 +51,7 @@ export const createAuthedSession = async ( // Assert // Should have session cookie returned. const sessionCookie = request.cookies.find( - (cookie) => cookie.name === 'connect.sid', + (cookie) => cookie.name === ADMIN_LOGIN_SESSION_COOKIE_NAME, ) expect(sessionCookie).toBeDefined() @@ -68,7 +69,7 @@ export const logoutSession = async (request: Session): Promise => { expect(response.status).toEqual(200) const sessionCookie = request.cookies.find( - (cookie) => cookie.name === 'connect.sid', + (cookie) => cookie.name === ADMIN_LOGIN_SESSION_COOKIE_NAME, ) expect(sessionCookie).not.toBeDefined() diff --git a/__tests__/integration/helpers/express-setup.ts b/__tests__/integration/helpers/express-setup.ts index 106b393a2c..6b2afebe97 100644 --- a/__tests__/integration/helpers/express-setup.ts +++ b/__tests__/integration/helpers/express-setup.ts @@ -17,7 +17,7 @@ const testSessionMiddlewares = () => { saveUninitialized: false, resave: false, secret: 'test-session-secret', - name: 'connect.sid', + name: 'formsg.connect.sid', store: new session.MemoryStore(), }) diff --git a/frontend/src/features/admin-form/create/builder-and-design/BuilderAndDesignDrawer/FieldListDrawer/field-panels/MyInfoPanel.tsx b/frontend/src/features/admin-form/create/builder-and-design/BuilderAndDesignDrawer/FieldListDrawer/field-panels/MyInfoPanel.tsx index 60d36bde4c..5d728ddc30 100644 --- a/frontend/src/features/admin-form/create/builder-and-design/BuilderAndDesignDrawer/FieldListDrawer/field-panels/MyInfoPanel.tsx +++ b/frontend/src/features/admin-form/create/builder-and-design/BuilderAndDesignDrawer/FieldListDrawer/field-panels/MyInfoPanel.tsx @@ -5,12 +5,7 @@ import { Box, Text } from '@chakra-ui/react' import { useFeatureIsOn, useGrowthBook } from '@growthbook/growthbook-react' import { featureFlags } from '~shared/constants' -import { - AdminFormDto, - FormAuthType, - FormResponseMode, - MyInfoAttribute, -} from '~shared/types' +import { AdminFormDto, FormAuthType, MyInfoAttribute } from '~shared/types' import { GUIDE_EMAIL_MODE } from '~constants/links' import { ADMINFORM_SETTINGS_SINGPASS_SUBROUTE } from '~constants/routes' @@ -113,14 +108,12 @@ export const MyInfoFieldPanel = () => { ) // myInfo should be disabled if - // 1. form response mode is not email mode - // 2. form auth type is not myInfo - // 3. # of myInfo fields >= 30 + // 1. form auth type is not myInfo + // 2. # of myInfo fields >= 30 const isMyInfoDisabled = useMemo( () => form ? form.form_fields.filter(isMyInfo).length >= 30 || - form.responseMode !== FormResponseMode.Email || (form.authType !== FormAuthType.MyInfo && form.authType !== FormAuthType.SGID_MyInfo) : true, @@ -232,14 +225,10 @@ export const MyInfoFieldPanel = () => { ) } -type MyInfoTextProps = Pick< - AdminFormDto, - 'authType' | 'responseMode' | 'form_fields' -> +type MyInfoTextProps = Pick const MyInfoText = ({ authType, - responseMode, form_fields, }: MyInfoTextProps): JSX.Element => { const isMyInfoDisabled = @@ -249,10 +238,6 @@ const MyInfoText = ({ [form_fields], ) - if (responseMode !== FormResponseMode.Email) { - return MyInfo fields are not available in Storage mode forms. - } - if (isMyInfoDisabled) { return ( diff --git a/frontend/src/features/admin-form/settings/components/AuthSettingsSection/AuthSettingsSection.tsx b/frontend/src/features/admin-form/settings/components/AuthSettingsSection/AuthSettingsSection.tsx index f824086b98..6cb0e880d3 100644 --- a/frontend/src/features/admin-form/settings/components/AuthSettingsSection/AuthSettingsSection.tsx +++ b/frontend/src/features/admin-form/settings/components/AuthSettingsSection/AuthSettingsSection.tsx @@ -21,7 +21,7 @@ import { isMyInfo } from '~features/myinfo/utils' import { useMutateFormSettings } from '../../mutations' -import { AUTHTYPE_TO_TEXT, STORAGE_MODE_AUTHTYPES } from './constants' +import { FORM_AUTHTYPES } from './constants' import { EsrvcIdBox } from './EsrvcIdBox' const esrvcidRequired = (authType: FormAuthType) => { @@ -42,13 +42,11 @@ interface AuthSettingsSectionProps { export const AuthSettingsSectionSkeleton = (): JSX.Element => { return ( - {Object.entries(STORAGE_MODE_AUTHTYPES).map( - ([authType, textToRender]) => ( - - {textToRender} - - ), - )} + {Object.entries(FORM_AUTHTYPES).map(([authType, textToRender]) => ( + + {textToRender} + + ))} ) } @@ -116,12 +114,9 @@ export const AuthSettingsSection = ({ [isDisabled, mutateFormAuthType, settings.authType], ) - const radioOptions: [FormAuthType, string][] = useMemo(() => { - return Object.entries(AUTHTYPE_TO_TEXT[settings.responseMode]) as [ - FormAuthType, - string, - ][] - }, [settings.responseMode]) + const radioOptions: [FormAuthType, string][] = Object.entries( + FORM_AUTHTYPES, + ) as [FormAuthType, string][] return ( diff --git a/frontend/src/features/admin-form/settings/components/AuthSettingsSection/constants.ts b/frontend/src/features/admin-form/settings/components/AuthSettingsSection/constants.ts index 6a4357d815..01a6e5837e 100644 --- a/frontend/src/features/admin-form/settings/components/AuthSettingsSection/constants.ts +++ b/frontend/src/features/admin-form/settings/components/AuthSettingsSection/constants.ts @@ -1,21 +1,12 @@ -import { FormAuthType, FormResponseMode } from '~shared/types/form' +import { FormAuthType } from '~shared/types/form' export type EmailFormAuthType = FormAuthType -export type StorageFormAuthType = - | FormAuthType.NIL - | FormAuthType.SP - | FormAuthType.CP - | FormAuthType.SGID +export type StorageFormAuthType = FormAuthType -export const STORAGE_MODE_AUTHTYPES: Record = { - [FormAuthType.NIL]: 'None', - [FormAuthType.SGID]: 'Singpass App-only Login', - [FormAuthType.SP]: 'Singpass', - [FormAuthType.CP]: 'Singpass (Corporate)', -} - -// Not using STORAGE_MODE_AUTHTYPES due to wanting a different order. -export const EMAIL_MODE_AUTHTYPES: Record = { +export const FORM_AUTHTYPES: Record< + StorageFormAuthType | EmailFormAuthType, + string +> = { [FormAuthType.NIL]: 'None', [FormAuthType.SGID]: 'Singpass App-only Login', [FormAuthType.SGID_MyInfo]: 'Singpass App-only with Myinfo', @@ -23,7 +14,3 @@ export const EMAIL_MODE_AUTHTYPES: Record = { [FormAuthType.MyInfo]: 'Singpass with Myinfo', [FormAuthType.CP]: 'Singpass (Corporate)', } -export const AUTHTYPE_TO_TEXT = { - [FormResponseMode.Email]: EMAIL_MODE_AUTHTYPES, - [FormResponseMode.Encrypt]: STORAGE_MODE_AUTHTYPES, -} diff --git a/frontend/src/features/admin-form/template/UseTemplateModal/UseTemplateWizardProvider.tsx b/frontend/src/features/admin-form/template/UseTemplateModal/UseTemplateWizardProvider.tsx index a4f3b1f0ea..b39b8fc4d6 100644 --- a/frontend/src/features/admin-form/template/UseTemplateModal/UseTemplateWizardProvider.tsx +++ b/frontend/src/features/admin-form/template/UseTemplateModal/UseTemplateWizardProvider.tsx @@ -1,9 +1,8 @@ -import { useEffect, useMemo } from 'react' +import { useEffect } from 'react' import { FormResponseMode } from '~shared/types' import { useFormTemplate } from '~features/admin-form/common/queries' -import { isMyInfo } from '~features/myinfo/utils' import { CreateFormFlowStates, CreateFormWizardContext, @@ -23,11 +22,6 @@ export const useUseTemplateWizardContext = ( /* enabled= */ !!formId, ) - const containsMyInfoFields = useMemo( - () => !!templateFormData?.form.form_fields.find((ff) => isMyInfo(ff)), - [templateFormData?.form.form_fields], - ) - const { formMethods, currentStep, direction, keypair, setCurrentStep } = useCommonFormWizardProvider() @@ -41,18 +35,9 @@ export const useUseTemplateWizardContext = ( reset({ ...getValues(), - responseMode: containsMyInfoFields - ? FormResponseMode.Email - : FormResponseMode.Encrypt, title: `[Template] ${templateFormData?.form.title}`, }) - }, [ - reset, - getValues, - containsMyInfoFields, - isTemplateFormLoading, - templateFormData?.form.title, - ]) + }, [reset, getValues, isTemplateFormLoading, templateFormData?.form.title]) const { handleSubmit } = formMethods @@ -98,7 +83,6 @@ export const useUseTemplateWizardContext = ( formMethods, handleDetailsSubmit, handleCreateStorageModeForm, - containsMyInfoFields, modalHeader: 'Duplicate form', } } diff --git a/frontend/src/features/workspace/components/CreateFormModal/CreateFormModalContent/CreateFormDetailsScreen.tsx b/frontend/src/features/workspace/components/CreateFormModal/CreateFormModalContent/CreateFormDetailsScreen.tsx index 9a5800bf43..f581e3a42c 100644 --- a/frontend/src/features/workspace/components/CreateFormModal/CreateFormModalContent/CreateFormDetailsScreen.tsx +++ b/frontend/src/features/workspace/components/CreateFormModal/CreateFormModalContent/CreateFormDetailsScreen.tsx @@ -17,7 +17,6 @@ import Button from '~components/Button' import FormErrorMessage from '~components/FormControl/FormErrorMessage' import FormFieldMessage from '~components/FormControl/FormFieldMessage' import FormLabel from '~components/FormControl/FormLabel' -import InlineMessage from '~components/InlineMessage' import Input from '~components/Input' import { useCreateFormWizard } from '../CreateFormWizardContext' @@ -35,7 +34,6 @@ export const CreateFormDetailsScreen = (): JSX.Element => { isLoading, isFetching, modalHeader, - containsMyInfoFields, } = useCreateFormWizard() const { register, @@ -79,23 +77,12 @@ export const CreateFormDetailsScreen = (): JSX.Element => { ( - - )} + render={({ field }) => } rules={{ required: 'Please select a form response mode' }} /> {errors.responseMode?.message} - {containsMyInfoFields && ( - - {`This form contains MyInfo fields. Only **Email** mode is supported at - this point.`} - - )} {responseModeValue === FormResponseMode.Email && ( void value: FormResponseMode } @@ -29,7 +28,7 @@ const OptionDescription = ({ listItems = [] }: { listItems: string[] }) => { export const FormResponseOptions = forwardRef< FormResponseOptionsProps, 'button' ->(({ value, onChange, containsMyInfoFields }, ref) => { +>(({ value, onChange }, ref) => { return ( Recommended} isActive={value === FormResponseMode.Encrypt} - isDisabled={containsMyInfoFields} onClick={() => onChange(FormResponseMode.Encrypt)} isFullWidth flex={1} diff --git a/frontend/src/features/workspace/components/CreateFormModal/CreateFormWizardContext.tsx b/frontend/src/features/workspace/components/CreateFormModal/CreateFormWizardContext.tsx index 1af1f663ed..11284dc68c 100644 --- a/frontend/src/features/workspace/components/CreateFormModal/CreateFormWizardContext.tsx +++ b/frontend/src/features/workspace/components/CreateFormModal/CreateFormWizardContext.tsx @@ -34,7 +34,6 @@ export type CreateFormWizardContextReturn = { isFetching: boolean isLoading: boolean modalHeader: string - containsMyInfoFields: boolean } export const CreateFormWizardContext = createContext< diff --git a/frontend/src/features/workspace/components/CreateFormModal/CreateFormWizardProvider.tsx b/frontend/src/features/workspace/components/CreateFormModal/CreateFormWizardProvider.tsx index 3b985d4a6e..bfe69cd1ea 100644 --- a/frontend/src/features/workspace/components/CreateFormModal/CreateFormWizardProvider.tsx +++ b/frontend/src/features/workspace/components/CreateFormModal/CreateFormWizardProvider.tsx @@ -105,8 +105,6 @@ const useCreateFormWizardContext = (): CreateFormWizardContextReturn => { handleDetailsSubmit, handleCreateStorageModeForm, modalHeader: 'Set up your form', - // Creation will never contain any fields. - containsMyInfoFields: false, } } diff --git a/frontend/src/features/workspace/components/DuplicateFormModal/DupeFormWizardProvider.tsx b/frontend/src/features/workspace/components/DuplicateFormModal/DupeFormWizardProvider.tsx index 5dd7d23be7..31d4cab2bb 100644 --- a/frontend/src/features/workspace/components/DuplicateFormModal/DupeFormWizardProvider.tsx +++ b/frontend/src/features/workspace/components/DuplicateFormModal/DupeFormWizardProvider.tsx @@ -1,9 +1,8 @@ -import { useEffect, useMemo } from 'react' +import { useEffect } from 'react' import { FormResponseMode } from '~shared/types' import { usePreviewForm } from '~features/admin-form/common/queries' -import { isMyInfo } from '~features/myinfo/utils' import { useDuplicateFormMutations } from '~features/workspace/mutations' import { useDashboard } from '~features/workspace/queries' import { makeDuplicateFormTitle } from '~features/workspace/utils/createDuplicateFormTitle' @@ -27,11 +26,6 @@ export const useDupeFormWizardContext = (): CreateFormWizardContextReturn => { /* enabled= */ !!activeFormMeta, ) - const containsMyInfoFields = useMemo( - () => !!previewFormData?.form.form_fields.find((ff) => isMyInfo(ff)), - [previewFormData?.form.form_fields], - ) - const { formMethods, currentStep, direction, keypair, setCurrentStep } = useCommonFormWizardProvider() @@ -50,9 +44,6 @@ export const useDupeFormWizardContext = (): CreateFormWizardContextReturn => { reset({ ...getValues(), - responseMode: containsMyInfoFields - ? FormResponseMode.Email - : FormResponseMode.Encrypt, title: makeDuplicateFormTitle(previewFormData.form.title, dashboardForms), }) }, [ @@ -62,7 +53,6 @@ export const useDupeFormWizardContext = (): CreateFormWizardContextReturn => { isPreviewFormLoading, isWorkspaceLoading, dashboardForms, - containsMyInfoFields, ]) const { handleSubmit } = formMethods @@ -116,7 +106,6 @@ export const useDupeFormWizardContext = (): CreateFormWizardContextReturn => { formMethods, handleDetailsSubmit, handleCreateStorageModeForm, - containsMyInfoFields, modalHeader: 'Duplicate form', } } diff --git a/shared/types/response.ts b/shared/types/response.ts index c42bd29e0a..de3ab7ad59 100644 --- a/shared/types/response.ts +++ b/shared/types/response.ts @@ -144,6 +144,16 @@ export type ChildBirthRecordsResponse = z.infer< typeof ChildBirthRecordsResponse > +// These fieldTypes are used for the child fields in MYINFO_ATTRIBUTE_MAP +export const SingleChildSubRecordResponse = MyInfoableSingleResponse.extend({ + fieldType: z.literal( + BasicField.Children || BasicField.ShortText || BasicField.Dropdown, + ), +}) +export type SingleChildSubRecordResponse = z.infer< + typeof SingleChildSubRecordResponse +> + export type FieldResponse = | HeaderResponse | EmailResponse @@ -165,3 +175,4 @@ export type FieldResponse = | TableResponse | UenResponse | ChildBirthRecordsResponse + | SingleChildSubRecordResponse diff --git a/src/app/loaders/express/session.ts b/src/app/loaders/express/session.ts index 707c3a199b..dd9eccb36b 100644 --- a/src/app/loaders/express/session.ts +++ b/src/app/loaders/express/session.ts @@ -6,6 +6,10 @@ import { Connection } from 'mongoose' import config from '../../config/config' +export const ADMIN_LOGIN_SESSION_COOKIE_NAME = config.isDev + ? 'formsg.connect.sid' + : 'connect.sid' + const sessionMiddlewares = (connection: Connection): RequestHandler[] => { // Configure express-session and connect to mongo const expressSession = session({ @@ -13,7 +17,8 @@ const sessionMiddlewares = (connection: Connection): RequestHandler[] => { resave: false, secret: config.sessionSecret, cookie: config.cookieSettings, - name: 'connect.sid', + // TODO: FRM-1512: Standardise cookie name across environments + name: ADMIN_LOGIN_SESSION_COOKIE_NAME, store: MongoStore.create({ client: connection.getClient(), }), diff --git a/src/app/models/__tests__/form.server.model.spec.ts b/src/app/models/__tests__/form.server.model.spec.ts index bd2e6bfe0c..4760c83612 100644 --- a/src/app/models/__tests__/form.server.model.spec.ts +++ b/src/app/models/__tests__/form.server.model.spec.ts @@ -694,31 +694,46 @@ describe('Form Model', () => { ) }) - it('should set authType to NIL when given authType is MyInfo', async () => { + // Ensure that encrypted sgID forms can be created since they could not before + it('should set authType to SGID when given authType is SGID', async () => { // Arrange - const malformedParams = merge({}, MOCK_ENCRYPTED_FORM_PARAMS, { + const encryptFormParams = merge({}, MOCK_ENCRYPTED_FORM_PARAMS, { + authType: FormAuthType.SGID, + }) + + // Act + const sgidForm = await EncryptedForm.create(encryptFormParams) + + // Assert + await expect(sgidForm.authType).toBe(FormAuthType.SGID) + }) + + // Ensure that encrypted MyInfo forms can be created since they could not before + it('should set authType to MyInfo when given authType is MyInfo', async () => { + // Arrange + const encryptFormParams = merge({}, MOCK_ENCRYPTED_FORM_PARAMS, { authType: FormAuthType.MyInfo, }) // Act - const invalidForm = await EncryptedForm.create(malformedParams) + const myInfoForm = await EncryptedForm.create(encryptFormParams) // Assert - await expect(invalidForm.authType).toBe(FormAuthType.NIL) + await expect(myInfoForm.authType).toBe(FormAuthType.MyInfo) }) - // Ensure that encrypted sgID forms can be created since they could not before - it('should set authType to SGID when given authType is SGID', async () => { + // Ensure that encrypted SGID MyInfo forms can be created since they could not before + it('should set authType to SGID MyInfo when given authType is SGID MyInfo', async () => { // Arrange const encryptFormParams = merge({}, MOCK_ENCRYPTED_FORM_PARAMS, { - authType: FormAuthType.SGID, + authType: FormAuthType.SGID_MyInfo, }) // Act - const sgidForm = await EncryptedForm.create(encryptFormParams) + const sgidMyInfoForm = await EncryptedForm.create(encryptFormParams) // Assert - await expect(sgidForm.authType).toBe(FormAuthType.SGID) + await expect(sgidMyInfoForm.authType).toBe(FormAuthType.SGID_MyInfo) }) it('should save with default payments settings', async () => { diff --git a/src/app/models/field/baseField.ts b/src/app/models/field/baseField.ts index 524f651135..444d6e8a81 100644 --- a/src/app/models/field/baseField.ts +++ b/src/app/models/field/baseField.ts @@ -1,11 +1,7 @@ import { Schema } from 'mongoose' import UIDGenerator from 'uid-generator' -import { - BasicField, - FormResponseMode, - MyInfoAttribute, -} from '../../../../shared/types' +import { BasicField, MyInfoAttribute } from '../../../../shared/types' import { IFieldSchema, IMyInfoSchema, ITableFieldSchema } from '../../../types' const uidgen3 = new UIDGenerator(256, UIDGenerator.BASE62) @@ -62,13 +58,6 @@ BaseFieldSchema.pre('validate', function (next) { return next(Error('Field type is incorrect or unspecified')) } - // Prevent MyInfo fields from being set in encrypt mode. - if (this.parent().responseMode === FormResponseMode.Encrypt) { - if (this.myInfo?.attr) { - return next(Error('MyInfo fields are not allowed for storage mode forms')) - } - } - // No errors. return next() }) diff --git a/src/app/models/form.server.model.ts b/src/app/models/form.server.model.ts index 0f4f887fa0..4526073c4c 100644 --- a/src/app/models/form.server.model.ts +++ b/src/app/models/form.server.model.ts @@ -311,13 +311,12 @@ const compileFormModel = (db: Mongoose): IFormModel => { return ( myInfoFieldCount === 0 || ((this.authType === FormAuthType.MyInfo || - this.authType == FormAuthType.SGID_MyInfo) && - this.responseMode === FormResponseMode.Email && + this.authType === FormAuthType.SGID_MyInfo) && myInfoFieldCount <= 30) ) }, message: - 'Check that your form is MyInfo-authenticated, is an email mode form and has 30 or fewer MyInfo fields.', + 'Check that your form is MyInfo-authenticated and has 30 or fewer MyInfo fields.', }, }, form_logics: { @@ -457,21 +456,6 @@ const compileFormModel = (db: Mongoose): IFormModel => { // Do not allow authType to be changed if form is published if (this.authType !== v && this.status === FormStatus.Public) { return this.authType - // Singpass/Corppass/SGID authentication is available for both email - // and storage mode - // Important - this case must come before the MyInfo + storage - // mode case, or else we may accidentally set Singpass/Corppass/SGID - // storage mode forms to FormAuthType.NIL - } else if ( - [FormAuthType.SP, FormAuthType.CP, FormAuthType.SGID].includes(v) - ) { - return v - } else if ( - this.responseMode === FormResponseMode.Encrypt && - // MyInfo is not available for storage mode - (v === FormAuthType.MyInfo || v === FormAuthType.SGID_MyInfo) - ) { - return FormAuthType.NIL } else { return v } diff --git a/src/app/modules/auth/auth.controller.ts b/src/app/modules/auth/auth.controller.ts index fb51f93fbd..edc996b9bd 100644 --- a/src/app/modules/auth/auth.controller.ts +++ b/src/app/modules/auth/auth.controller.ts @@ -4,6 +4,7 @@ import { SendOtpResponseDto } from 'shared/types/user' import { SUPPORT_FORM_LINK } from '../../../../shared/constants/links' import { createLoggerWithLabel } from '../../config/logger' +import { ADMIN_LOGIN_SESSION_COOKIE_NAME } from '../../loaders/express/session' import MailService from '../../services/mail/mail.service' import { createReqMeta, getRequestIp } from '../../utils/request' import { ControllerHandler } from '../core/core.types' @@ -256,7 +257,7 @@ export const handleSignout: ControllerHandler = async (req, res) => { } // No error. - res.clearCookie('connect.sid') + res.clearCookie(ADMIN_LOGIN_SESSION_COOKIE_NAME) return res.status(StatusCodes.OK).json({ message: 'Sign out successful' }) }) } diff --git a/src/app/modules/submission/email-submission/email-submission.types.ts b/src/app/modules/submission/email-submission/email-submission.types.ts index 198334e0f2..eab814517e 100644 --- a/src/app/modules/submission/email-submission/email-submission.types.ts +++ b/src/app/modules/submission/email-submission/email-submission.types.ts @@ -26,3 +26,8 @@ export interface IPopulatedEmailFormWithResponsesAndHash { parsedResponses: ParsedResponsesObject hashedFields?: Set } + +export interface IPopulatedStorageFormWithResponsesAndHash { + parsedResponses: ParsedResponsesObject + hashedFields?: Set +} diff --git a/src/app/modules/submission/email-submission/email-submission.util.ts b/src/app/modules/submission/email-submission/email-submission.util.ts index 5ac3609369..09f5074893 100644 --- a/src/app/modules/submission/email-submission/email-submission.util.ts +++ b/src/app/modules/submission/email-submission/email-submission.util.ts @@ -1,12 +1,7 @@ import { StatusCodes } from 'http-status-codes' import { compact } from 'lodash' -import { MYINFO_ATTRIBUTE_MAP } from '../../../../../shared/constants/field/myinfo' -import { - BasicField, - FormAuthType, - MyInfoAttribute, -} from '../../../../../shared/types' +import { BasicField, FormAuthType } from '../../../../../shared/types' import { EmailAdminDataField, EmailDataCollationToolField, @@ -58,7 +53,6 @@ import { MyInfoMissingLoginCookieError, } from '../../myinfo/myinfo.errors' import { MyInfoKey } from '../../myinfo/myinfo.types' -import { getMyInfoChildHashKey } from '../../myinfo/myinfo.util' import { SgidInvalidJwtError, SgidMissingJwtError, @@ -80,14 +74,13 @@ import { } from '../submission.errors' import { ProcessedCheckboxResponse, - ProcessedChildrenResponse, ProcessedFieldResponse, ProcessedTableResponse, } from '../submission.types' +import { getAnswersForChild, getMyInfoPrefix } from '../submission.utils' import { ATTACHMENT_PREFIX, - MYINFO_PREFIX, TABLE_PREFIX, VERIFIED_PREFIX, } from './email-submission.constants' @@ -96,22 +89,6 @@ import { ResponseFormattedForEmail } from './email-submission.types' const logger = createLoggerWithLabel(module) -/** - * Determines the prefix for a question based on whether it is verified - * by MyInfo. - * @param response - * @param hashedFields Field ids of hashed fields. - * @returns the prefix - */ -const getMyInfoPrefix = ( - response: ResponseFormattedForEmail, - hashedFields: Set, -): string => { - return !!response.myInfo?.attr && hashedFields.has(response._id) - ? MYINFO_PREFIX - : '' -} - /** * Determines the prefix for a question based on whether it was verified * by a user during form submission. @@ -214,44 +191,6 @@ export const getAnswerForCheckbox = ( } } -export const getAnswersForChild = ( - response: ProcessedChildrenResponse, -): ResponseFormattedForEmail[] => { - const subFields = response.childSubFieldsArray - const qnChildIdx = response.childIdx ?? 0 - if (!subFields) { - return [] - } - return response.answerArray.flatMap((arr, childIdx) => { - // First array element is always child name - const childName = arr[0] - return arr.map((answer, idx) => { - const subfield = subFields[idx] - return { - _id: getMyInfoChildHashKey( - response._id, - subFields[idx], - childIdx, - childName, - ), - fieldType: response.fieldType, - // qnChildIdx represents the index of the MyInfo field - // childIdx represents the index of the child in this MyInfo field - // as there might be >1 child for each MyInfo child field if "Add another child" is used - question: `Child ${qnChildIdx + childIdx + 1} ${ - MYINFO_ATTRIBUTE_MAP[subfield].description - }`, - myInfo: { - attr: subFields[idx] as unknown as MyInfoAttribute, - }, - isVisible: response.isVisible, - isUserVerified: response.isUserVerified, - answer, - } - }) - }) -} - /** * Formats the response for sending to the submitter (autoReplyData), * the table that is sent to the admin (formData), 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 1e5c77bdc2..88a8e09ab4 100644 --- a/src/app/modules/submission/encrypt-submission/encrypt-submission.controller.ts +++ b/src/app/modules/submission/encrypt-submission/encrypt-submission.controller.ts @@ -39,11 +39,12 @@ import * as TurnstileMiddleware from '../../../services/turnstile/turnstile.midd import { Pipeline } from '../../../utils/pipeline-middleware' import { createReqMeta } from '../../../utils/request' import { getFormAfterPermissionChecks } from '../../auth/auth.service' -import { MalformedParametersError } from '../../core/core.errors' import { ControllerHandler } from '../../core/core.types' import { setFormTags } from '../../datadog/datadog.utils' import { getFeatureFlag } from '../../feature-flags/feature-flags.service' import { PermissionLevel } from '../../form/admin-form/admin-form.types' +import { MyInfoService } from '../../myinfo/myinfo.service' +import { extractMyInfoLoginJwt } from '../../myinfo/myinfo.util' import { SgidService } from '../../sgid/sgid.service' import { getOidcService } from '../../spcp/spcp.oidc.service' import { getPopulatedUserById } from '../../user/user.service' @@ -146,19 +147,6 @@ const submitEncryptModeForm = async ( let userInfo const { authType } = formDef switch (authType) { - case FormAuthType.MyInfo: { - logger.error({ - message: - 'Storage mode form is not allowed to have MyInfo authorisation', - meta: logMeta, - }) - const { errorMessage, statusCode } = mapRouteError( - new MalformedParametersError( - 'Storage mode form is not allowed to have MyInfo authType', - ), - ) - return res.status(statusCode).json({ message: errorMessage }) - } case FormAuthType.SP: { const oidcService = getOidcService(FormAuthType.SP) const jwtPayloadResult = await oidcService @@ -204,6 +192,45 @@ const submitEncryptModeForm = async ( userInfo = jwtPayloadResult.value.userInfo break } + case FormAuthType.SGID_MyInfo: + case FormAuthType.MyInfo: { + const jwtPayloadResult = await extractMyInfoLoginJwt( + req.cookies, + authType, + ) + .andThen(MyInfoService.verifyLoginJwt) + .map(({ uinFin }) => { + return uinFin + }) + .mapErr((error) => { + logger.error({ + message: `Error verifying MyInfo${ + authType === FormAuthType.SGID_MyInfo ? '(over SGID)' : '' + } hashes`, + meta: logMeta, + error, + }) + return error + }) + if (jwtPayloadResult.isErr()) { + const { statusCode, errorMessage } = mapRouteError( + jwtPayloadResult.error, + ) + logger.error({ + message: `Failed to verify ${ + authType === FormAuthType.SGID_MyInfo ? 'SGID' : 'Singpass' + } JWT with auth client`, + meta: logMeta, + error: jwtPayloadResult.error, + }) + return res.status(statusCode).json({ + message: errorMessage, + spcpSubmissionFailure: true, + }) + } + uinFin = jwtPayloadResult.value + break + } case FormAuthType.SGID: { const jwtPayloadResult = SgidService.extractSgidSingpassJwtPayload( req.cookies.jwtSgid, @@ -232,7 +259,9 @@ const submitEncryptModeForm = async ( if ( form.authType === FormAuthType.SP || form.authType === FormAuthType.CP || - form.authType === FormAuthType.SGID + form.authType === FormAuthType.SGID || + form.authType === FormAuthType.MyInfo || + form.authType === FormAuthType.SGID_MyInfo ) { const encryptVerifiedContentResult = VerifiedContentService.getVerifiedContent({ 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 3ae11e7310..c9e2de26c8 100644 --- a/src/app/modules/submission/encrypt-submission/encrypt-submission.middleware.ts +++ b/src/app/modules/submission/encrypt-submission/encrypt-submission.middleware.ts @@ -11,6 +11,7 @@ import { } from '../../../../../shared/constants' import { BasicField, + FormAuthType, StorageModeAttachment, StorageModeAttachmentsMap, } from '../../../../../shared/types' @@ -29,6 +30,9 @@ 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 { MyInfoService } from '../../myinfo/myinfo.service' +import { extractMyInfoLoginJwt } from '../../myinfo/myinfo.util' +import { IPopulatedStorageFormWithResponsesAndHash } from '../email-submission/email-submission.types' import ParsedResponsesObject from '../ParsedResponsesObject.class' import { sharedSubmissionParams } from '../submission.constants' import * as SubmissionService from '../submission.service' @@ -58,7 +62,10 @@ import { StorageSubmissionMiddlewareHandlerType, ValidateSubmissionMiddlewareHandlerRequest, } from './encrypt-submission.types' -import { mapRouteError } from './encrypt-submission.utils' +import { + formatMyInfoStorageResponseData, + mapRouteError, +} from './encrypt-submission.utils' import IncomingEncryptSubmission from './IncomingEncryptSubmission.class' const logger = createLoggerWithLabel(module) @@ -388,6 +395,7 @@ export const validateStorageSubmission = async ( next: NextFunction, ) => { const formDef = req.formsg.formDef + let spcpSubmissionFailure: undefined | true const logMeta = { action: 'validateStorageSubmission', @@ -413,12 +421,14 @@ export const validateStorageSubmission = async ( else { // eslint-disable-next-line @typescript-eslint/no-unused-vars const { filename: __, content: ___, ...restAttachments } = rest - responses.push({ ...restAttachments } as EncryptAttachmentResponse) + responses.push({ + ...restAttachments, + } as EncryptAttachmentResponse) } } } req.formsg.filteredResponses = responses - return next() + return { parsedResponses, form: formDef } }) .mapErr((error) => { // TODO(FRM-1318): Set DB flag to true to harden submission validation after validation has similar error rates as email mode forms. @@ -444,8 +454,68 @@ export const validateStorageSubmission = async ( error, }) req.formsg.filteredResponses = req.body.responses + return error + }) + .andThen(({ parsedResponses, form }) => { + // Validate MyInfo responses + const { authType } = form + switch (authType) { + case FormAuthType.SGID_MyInfo: + case FormAuthType.MyInfo: { + return extractMyInfoLoginJwt(req.cookies, authType) + .andThen(MyInfoService.verifyLoginJwt) + .asyncAndThen(({ uinFin }) => + MyInfoService.fetchMyInfoHashes(uinFin, form._id) + .andThen((hashes) => + MyInfoService.checkMyInfoHashes( + parsedResponses.responses, + hashes, + ), + ) + .map( + (hashedFields) => ({ + hashedFields, + parsedResponses, + }), + ), + ) + .mapErr((error) => { + spcpSubmissionFailure = true + logger.error({ + message: `Error verifying MyInfo${ + authType === FormAuthType.SGID_MyInfo ? '(over SGID)' : '' + } hashes`, + meta: logMeta, + error, + }) + return error + }) + } + default: + return ok({ + parsedResponses, + }) + } + }) + .map(({ parsedResponses, hashedFields }) => { + const storageFormData = formatMyInfoStorageResponseData( + parsedResponses.getAllResponses(), + hashedFields, + ) + req.body.responses = storageFormData return next() }) + .mapErr((error) => { + logger.error({ + message: 'Error saving responses in req.body', + meta: logMeta, + error, + }) + return res.status(StatusCodes.INTERNAL_SERVER_ERROR).json({ + message: 'Error saving responses in req.body', + spcpSubmissionFailure, + }) + }) } const encryptAttachment = async ( diff --git a/src/app/modules/submission/encrypt-submission/encrypt-submission.utils.ts b/src/app/modules/submission/encrypt-submission/encrypt-submission.utils.ts index 631d04e0d9..e31cde8c88 100644 --- a/src/app/modules/submission/encrypt-submission/encrypt-submission.utils.ts +++ b/src/app/modules/submission/encrypt-submission/encrypt-submission.utils.ts @@ -11,6 +11,7 @@ import { SubmissionType, } from '../../../../../shared/types' import { calculatePrice } from '../../../../../shared/utils/paymentProductPrice' +import { isProcessedChildResponse } from '../../../../app/utils/field-validation/field-validation.guards' import { IEncryptedSubmissionSchema, IPopulatedEncryptedForm, @@ -48,6 +49,7 @@ import { FormNotFoundError, PrivateFormError, } from '../../form/form.errors' +import { MyInfoKey } from '../../myinfo/myinfo.types' import { PaymentNotFoundError } from '../../payments/payments.errors' import { SgidInvalidJwtError, @@ -71,6 +73,8 @@ import { SubmissionNotFoundError, ValidateFieldError, } from '../submission.errors' +import { ProcessedFieldResponse } from '../submission.types' +import { getAnswersForChild, getMyInfoPrefix } from '../submission.utils' import { AttachmentSizeLimitExceededError, @@ -359,3 +363,27 @@ export const getPaymentIntentDescription = ( } } } + +export const formatMyInfoStorageResponseData = ( + parsedResponses: ProcessedFieldResponse[], + hashedFields?: Set, +) => { + if (!hashedFields) { + return parsedResponses + } else { + return parsedResponses.flatMap((response) => { + if (isProcessedChildResponse(response)) { + return getAnswersForChild(response).map((childField) => { + const myInfoPrefix = getMyInfoPrefix(childField, hashedFields) + childField.question = `${myInfoPrefix}${childField.question}` + return childField + }) + } else { + // Obtain prefix for question based on whether it is verified by MyInfo. + const myInfoPrefix = getMyInfoPrefix(response, hashedFields) + response.question = `${myInfoPrefix}${response.question}` + return response + } + }) + } +} diff --git a/src/app/modules/submission/submission.utils.ts b/src/app/modules/submission/submission.utils.ts index 1266ad634b..4e6c63b300 100644 --- a/src/app/modules/submission/submission.utils.ts +++ b/src/app/modules/submission/submission.utils.ts @@ -9,10 +9,12 @@ import { import { err, ok, Result } from 'neverthrow' import { FIELDS_TO_REJECT } from '../../../../shared/constants/field/basic' +import { MYINFO_ATTRIBUTE_MAP } from '../../../../shared/constants/field/myinfo' import { BasicField, FormField, FormResponseMode, + MyInfoAttribute, } from '../../../../shared/types' import * as FileValidation from '../../../../shared/utils/file-validation' import { @@ -26,9 +28,18 @@ import { ParsedClearFormFieldResponse, } from '../../../types/api' import { AutoReplyMailData } from '../../services/mail/mail.types' +import { MyInfoKey } from '../myinfo/myinfo.types' +import { getMyInfoChildHashKey } from '../myinfo/myinfo.util' +import { MYINFO_PREFIX } from './email-submission/email-submission.constants' +import { ResponseFormattedForEmail } from './email-submission/email-submission.types' import { ConflictError } from './submission.errors' -import { FilteredResponse } from './submission.types' +import { + FilteredResponse, + ProcessedChildrenResponse, + ProcessedFieldResponse, + ProcessedSingleAnswerResponse, +} from './submission.types' type ResponseModeFilterParam = { fieldType: BasicField @@ -266,3 +277,63 @@ export const mapAttachmentsFromResponses = ( content: response.content, })) } + +/** + * Determines the prefix for a question based on whether it is verified + * by MyInfo. + * @param response + * @param hashedFields Field ids of hashed fields. + * @returns the prefix + */ +export const getMyInfoPrefix = ( + response: ResponseFormattedForEmail | ProcessedFieldResponse, + hashedFields: Set, +): string => { + return !!response.myInfo?.attr && hashedFields.has(response._id) + ? MYINFO_PREFIX + : '' +} + +/** + * Expands child subfields into individual fields, so that they are no longer nested under + * 1 parent field. + * @param response + * @returns + */ +export const getAnswersForChild = ( + response: ProcessedChildrenResponse, +): ProcessedSingleAnswerResponse[] => { + const subFields = response.childSubFieldsArray + const qnChildIdx = response.childIdx ?? 0 + if (!subFields) { + return [] + } + return response.answerArray.flatMap((arr, childIdx) => { + // First array element is always child name + const childName = arr[0] + return arr.map((answer, idx) => { + const subfield = subFields[idx] + return { + _id: getMyInfoChildHashKey( + response._id, + subFields[idx], + childIdx, + childName, + ), + fieldType: response.fieldType, + // qnChildIdx represents the index of the MyInfo field + // childIdx represents the index of the child in this MyInfo field + // as there might be >1 child for each MyInfo child field if "Add another child" is used + question: `Child ${qnChildIdx + childIdx + 1} ${ + MYINFO_ATTRIBUTE_MAP[subfield].description + }`, + myInfo: { + attr: subFields[idx] as unknown as MyInfoAttribute, + }, + isVisible: response.isVisible, + isUserVerified: response.isUserVerified, + answer, + } + }) + }) +} diff --git a/src/app/modules/verified-content/verified-content.service.ts b/src/app/modules/verified-content/verified-content.service.ts index 826cf00db8..dafded0e66 100644 --- a/src/app/modules/verified-content/verified-content.service.ts +++ b/src/app/modules/verified-content/verified-content.service.ts @@ -34,10 +34,12 @@ export const getVerifiedContent = ({ CpVerifiedContent | SpVerifiedContent | SgidVerifiedContent > => { switch (type) { + case FormAuthType.MyInfo: case FormAuthType.SP: return getSpVerifiedContent(data) case FormAuthType.CP: return getCpVerifiedContent(data) + case FormAuthType.SGID_MyInfo: case FormAuthType.SGID: return getSgidVerifiedContent(data) } diff --git a/src/app/modules/verified-content/verified-content.types.ts b/src/app/modules/verified-content/verified-content.types.ts index c7aecc77fb..896861bd60 100644 --- a/src/app/modules/verified-content/verified-content.types.ts +++ b/src/app/modules/verified-content/verified-content.types.ts @@ -41,6 +41,11 @@ export type EncryptVerificationContentParams = { } export type GetVerifiedContentParams = { - type: FormAuthType.SP | FormAuthType.CP | FormAuthType.SGID + type: + | FormAuthType.SP + | FormAuthType.CP + | FormAuthType.SGID + | FormAuthType.MyInfo + | FormAuthType.SGID_MyInfo data: Record } diff --git a/src/app/routes/api/v3/auth/__tests__/auth.routes.spec.ts b/src/app/routes/api/v3/auth/__tests__/auth.routes.spec.ts index 1c27619e83..63279c91bb 100644 --- a/src/app/routes/api/v3/auth/__tests__/auth.routes.spec.ts +++ b/src/app/routes/api/v3/auth/__tests__/auth.routes.spec.ts @@ -6,6 +6,7 @@ import { errAsync, okAsync } from 'neverthrow' import supertest, { Session } from 'supertest-session' import validator from 'validator' +import { ADMIN_LOGIN_SESSION_COOKIE_NAME } from 'src/app/loaders/express/session' import MailService from 'src/app/services/mail/mail.service' import { HashingError } from 'src/app/utils/hash' import * as OtpUtils from 'src/app/utils/otp' @@ -511,7 +512,7 @@ describe('auth.routes', () => { }) // Should have session cookie returned. const sessionCookie = request.cookies.find( - (cookie) => cookie.name === 'connect.sid', + (cookie) => cookie.name === ADMIN_LOGIN_SESSION_COOKIE_NAME, ) expect(sessionCookie).toBeDefined() }) @@ -538,7 +539,7 @@ describe('auth.routes', () => { }) // Should have session cookie returned. const sessionCookie = request.cookies.find( - (cookie) => cookie.name === 'connect.sid', + (cookie) => cookie.name === ADMIN_LOGIN_SESSION_COOKIE_NAME, ) expect(sessionCookie).toBeDefined() }) @@ -591,9 +592,9 @@ describe('auth.routes', () => { // Assert expect(response.status).toEqual(200) expect(response.body).toEqual({ message: 'Sign out successful' }) - // connect.sid should now be empty. + // Login cookie should now be empty. expect(response.header['set-cookie'][0]).toEqual( - expect.stringContaining('connect.sid=;'), + expect.stringContaining(`${ADMIN_LOGIN_SESSION_COOKIE_NAME}=;`), ) }) @@ -629,7 +630,7 @@ describe('auth.routes', () => { // Assert // Should have session cookie returned. const sessionCookie = request.cookies.find( - (cookie) => cookie.name === 'connect.sid', + (cookie) => cookie.name === ADMIN_LOGIN_SESSION_COOKIE_NAME, ) expect(sessionCookie).toBeDefined() return response.body