Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: mrf be field validation #7718

Merged
merged 48 commits into from
Oct 15, 2024
Merged
Show file tree
Hide file tree
Changes from 36 commits
Commits
Show all changes
48 commits
Select commit Hold shift + click to select a range
83bf680
chore: remove unused verifiable mobile field
kevin9foong Oct 2, 2024
a123cfc
feat: add validation v3 code to middleware
kevin9foong Oct 2, 2024
3208b50
feat: set up scaffold
kevin9foong Oct 3, 2024
3378af0
feat: add validation v3 code
kevin9foong Oct 3, 2024
2338099
feat: add text validator
kevin9foong Oct 3, 2024
04bb49f
feat: add country, homeno and rating validation
kevin9foong Oct 3, 2024
a01fa60
feat: add uen and nric validators
kevin9foong Oct 3, 2024
01aa96c
feat: add dropdown validator
kevin9foong Oct 3, 2024
cdbbf4c
feat: add yesno and verifiable field validator
kevin9foong Oct 3, 2024
461a364
feat: add attachment validator
kevin9foong Oct 3, 2024
b2dc4f9
feat: add number and decimal validator
kevin9foong Oct 4, 2024
38d0897
feat: add children and table validator
kevin9foong Oct 7, 2024
f86534d
fix: checkbox validation to accomodate for internal val
kevin9foong Oct 7, 2024
46e95bc
fix: remove unused imports
kevin9foong Oct 7, 2024
026b87f
feat: check if validation is required
kevin9foong Oct 7, 2024
576dda2
fix: handle diff date and checkbox format
kevin9foong Oct 7, 2024
23ff8c6
fix: bug where textfield type in table do not cause rejection
kevin9foong Oct 7, 2024
a6af47e
feat: add v3 specific checks for isResponseForHidden and validation r…
kevin9foong Oct 7, 2024
b2410f2
fix: explicitly check for radio response answer properties
kevin9foong Oct 7, 2024
a28afd0
fix: include answer.answer for attachment check
kevin9foong Oct 8, 2024
a73a366
feat: add TC helper for v3 responses and tc for yesno
kevin9foong Oct 8, 2024
5e8e47c
feat: add TC for nric fields
kevin9foong Oct 8, 2024
f5b0784
feat: add date validation
kevin9foong Oct 8, 2024
4031351
fix: remove only in tc
kevin9foong Oct 8, 2024
f663b36
feat: add radio and country validator tc and fix radio button validat…
kevin9foong Oct 8, 2024
761e846
feat: add TC for dropdown validator v3
kevin9foong Oct 8, 2024
f7ad718
feat: add tc for rating validation
kevin9foong Oct 8, 2024
7d4b36a
feat: add tc for short and long text validator v3
kevin9foong Oct 8, 2024
de3885d
feat: add tc for home no validation
kevin9foong Oct 8, 2024
45f03b7
feat: add checkbox validation and fix checkbox v3 validation logic
kevin9foong Oct 8, 2024
515f907
feat: fix bug with othersInput and add TC for othersInput being inclu…
kevin9foong Oct 8, 2024
e3ab05e
feat: add tc for mobileno, fix tc bugs and mobileno validation v3 bugs
kevin9foong Oct 8, 2024
b6223d5
fix: change to relative import
kevin9foong Oct 8, 2024
06e993b
fix: empty string to 0
kevin9foong Oct 9, 2024
19e5c74
feat: add tc for table validation v3
kevin9foong Oct 9, 2024
82be2f0
chore: add exhaustivechecks on switch
KenLSM Oct 9, 2024
29fbad8
refactor: share isValidResponseFieldType, doFieldTypesMatch functions
KenLSM Oct 9, 2024
0933faa
chore: remove unintended commit of datadogchunk
KenLSM Oct 9, 2024
f1ffe02
feat: add number validation v3
KenLSM Oct 9, 2024
aff513a
Merge remote-tracking branch 'origin/feat/mrf-be-validation-ken' into…
KenLSM Oct 9, 2024
150171f
feat: add tc for attachment validation
kevin9foong Oct 9, 2024
66be9ed
feat: add tc for decimal
kevin9foong Oct 9, 2024
92d4b14
feat: add tc for uen validation v3
kevin9foong Oct 9, 2024
6e9da95
feat: refactor to use switch statement
kevin9foong Oct 9, 2024
e5f5ac0
feat: add tc for email validation v3
kevin9foong Oct 9, 2024
96be8fd
feat: add validateMrfFieldResponses tc
kevin9foong Oct 9, 2024
fe4dc21
feat: fix email default field create
kevin9foong Oct 9, 2024
1eb69c8
feat: unblock when v3 validation fails
kevin9foong Oct 9, 2024
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
97 changes: 95 additions & 2 deletions __tests__/unit/backend/helpers/generate-form-data.ts
Original file line number Diff line number Diff line change
Expand Up @@ -26,17 +26,34 @@ import {
ITableFieldSchema,
SingleAnswerFieldResponse,
} from 'src/types'
import {
ParsedClearAttachmentFieldResponseV3,
ParsedClearAttachmentResponseV3,
} from 'src/types/api'

import {
AllowMyInfoBase,
AttachmentSize,
BasicField,
CheckboxFieldResponsesV3,
CheckboxResponse,
CheckboxResponseV3,
Column,
DropdownFieldBase,
EmailResponseV3,
FormField,
GenericStringAnswerFieldResponseV3,
MobileResponseV3,
RadioFieldResponsesV3,
RadioResponseV3,
ShortTextFieldBase,
StringAnswerResponseV3,
TableFieldResponsesV3,
TableResponseV3,
TableRow,
VerifiableFieldResponseV3,
YesNoFieldResponseV3,
YesNoResponseV3,
} from '../../../../shared/types'

export const generateDefaultField = (
Expand Down Expand Up @@ -368,7 +385,7 @@ export const generateNewTableResponse = (
})

export const generateTableDropdownColumn = (
customParams?: Partial<DropdownFieldBase>,
customParams?: Partial<DropdownFieldBase> & { _id?: string },
): Column => {
return {
title: 'some title',
Expand All @@ -392,7 +409,7 @@ export const generateTableDropdownColumn = (
}

export const generateTableShortTextColumn = (
customParams?: Partial<ShortTextFieldBase>,
customParams?: Partial<ShortTextFieldBase> & { _id?: string },
): Column => {
return {
title: 'some title',
Expand Down Expand Up @@ -420,3 +437,79 @@ export const generateTableShortTextColumn = (
},
} as Column
}

export const generateGenericStringAnswerResponseV3 = ({
fieldType,
answer = 'answer',
}: {
fieldType: GenericStringAnswerFieldResponseV3['fieldType']
answer: StringAnswerResponseV3
}): GenericStringAnswerFieldResponseV3 => {
return {
fieldType,
answer,
}
}

export const generateYesNoAnswerResponseV3 = (
answer: YesNoFieldResponseV3 = 'Yes',
): YesNoResponseV3 => {
return {
fieldType: BasicField.YesNo,
answer: answer,
}
}

export const generateAttachmentResponseV3 = (
answer: ParsedClearAttachmentFieldResponseV3 = {
filename: 'Attachment filename',
content: Buffer.from('Attachment content'),
answer: 'Attachment answer',
hasBeenScanned: false,
},
): ParsedClearAttachmentResponseV3 => {
return {
fieldType: BasicField.Attachment,
answer,
}
}

export const generateVerifiableAnswerResponseV3 = ({
fieldType,
answer,
}: {
fieldType: BasicField.Email | BasicField.Mobile
answer: VerifiableFieldResponseV3
}): EmailResponseV3 | MobileResponseV3 => {
return {
fieldType,
answer,
}
}

export const generateTableResponseV3 = (
answer: TableFieldResponsesV3,
): TableResponseV3 => {
return {
fieldType: BasicField.Table,
answer,
}
}

export const generateRadioResponseV3 = (
answer: RadioFieldResponsesV3,
): RadioResponseV3 => {
return {
fieldType: BasicField.Radio,
answer,
}
}

export const generateCheckboxResponseV3 = (
answer: CheckboxFieldResponsesV3,
): CheckboxResponseV3 => {
return {
fieldType: BasicField.Checkbox,
answer,
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ import {
FieldResponseAnswerMapV3,
RadioFieldResponsesV3,
TableFieldResponsesV3,
VerifiableFieldResponsesV3,
VerifiableFieldResponseV3,
YesNoFieldResponseV3,
} from '~shared/types'
import { BasicField, FormFieldDto } from '~shared/types/field'
Expand Down Expand Up @@ -65,7 +65,7 @@ const transformToVerifiableOutput = <
F extends EmailFieldSchema | MobileFieldSchema,
>(
schema: F,
input?: VerifiableFieldValues | VerifiableFieldResponsesV3,
input?: VerifiableFieldValues | VerifiableFieldResponseV3,
): VerifiableAnswerOutput<F> => {
return {
...pickBaseOutputFromSchema(schema),
Expand Down
1 change: 1 addition & 0 deletions frontend/static/js/datadog-chunk.50a858fcfc3b8b0c176e.js

Large diffs are not rendered by default.

3 changes: 3 additions & 0 deletions shared/constants/form.ts
Original file line number Diff line number Diff line change
Expand Up @@ -92,6 +92,9 @@ export const PAYMENT_VARIABLE_INPUT_AMOUNT_FIELD_ID =
export const CLIENT_RADIO_OTHERS_INPUT_VALUE =
'!!FORMSG_INTERNAL_RADIO_OTHERS_VALUE!!'

export const CLIENT_CHECKBOX_OTHERS_INPUT_VALUE =
kevin9foong marked this conversation as resolved.
Show resolved Hide resolved
'!!FORMSG_INTERNAL_CHECKBOX_OTHERS_VALUE!!'

// The current encrypt version to assign to the encrypted submission.
// This is needed if we ever break backwards compatibility with
// end-to-end encryption
Expand Down
17 changes: 17 additions & 0 deletions shared/types/field/base.ts
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,23 @@ export enum BasicField {
Children = 'children',
}

/**
* Contains field types where the answer is a generic string type
*/
export enum GenericStringAnswerResponseFieldV3 {
Number = 'number',
Decimal = 'decimal',
ShortText = 'textfield',
LongText = 'textarea',
HomeNo = 'homeno',
Dropdown = 'dropdown',
Rating = 'rating',
Nric = 'nric',
Uen = 'uen',
Date = 'date',
CountryRegion = 'country_region',
}

export enum MyInfoAttribute {
Name = 'name',
PassportNumber = 'passportnumber',
Expand Down
55 changes: 30 additions & 25 deletions shared/types/response-v3.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4,25 +4,28 @@ export type FieldResponsesV3 = Record<FormFieldDto['_id'], FieldResponseV3>

export type FieldResponseV3 =
| HeaderResponseV3
| RadioResponseV3
| CheckboxResponseV3
| AttachmentResponseV3
| TableResponseV3
| ChildBirthRecordsResponseV3
| YesNoResponseV3
| EmailResponseV3
| MobileResponseV3
| HomeNoResponseV3
| GenericStringAnswerFieldResponseV3

export type GenericStringAnswerFieldResponseV3 =
| NumberResponseV3
| DecimalResponseV3
| ShortTextResponseV3
| LongTextResponseV3
| HomeNoResponseV3
| DropdownResponseV3
| CountryRegionResponseV3
| YesNoResponseV3
| CheckboxResponseV3
| RadioResponseV3
| AttachmentResponseV3
| DateResponseV3
| RatingResponseV3
| NricResponseV3
| TableResponseV3
| UenResponseV3
| ChildBirthRecordsResponseV3
| DateResponseV3
| CountryRegionResponseV3

export type HeaderResponseV3 = FieldResponseFactoryV3<BasicField.Section>
export type EmailResponseV3 = FieldResponseFactoryV3<BasicField.Email>
Expand Down Expand Up @@ -57,25 +60,14 @@ export type FieldResponseV3Base = {
fieldType: BasicField
}
export type FieldResponseAnswerMapV3<F extends BasicField = BasicField> =
F extends
| BasicField.Number
| BasicField.Decimal
| BasicField.ShortText
| BasicField.LongText
| BasicField.HomeNo
| BasicField.Dropdown
| BasicField.Rating
| BasicField.Nric
| BasicField.Uen
| BasicField.Date
| BasicField.CountryRegion
? SingleAnswerResponseV3
F extends GenericStringAnswerResponseFieldTypeV3
? StringAnswerResponseV3
: F extends BasicField.YesNo
? YesNoFieldResponseV3
: F extends BasicField.Attachment
? AttachmentFieldResponseV3
: F extends BasicField.Email | BasicField.Mobile
? VerifiableFieldResponsesV3
? VerifiableFieldResponseV3
: F extends BasicField.Table
? TableFieldResponsesV3
: F extends BasicField.Radio
Expand All @@ -86,9 +78,22 @@ export type FieldResponseAnswerMapV3<F extends BasicField = BasicField> =
? ChildrenCompoundFieldResponsesV3
: never

export type SingleAnswerResponseV3 = string
export type GenericStringAnswerResponseFieldTypeV3 =
| NumberResponseV3['fieldType']
| DecimalResponseV3['fieldType']
| ShortTextResponseV3['fieldType']
| LongTextResponseV3['fieldType']
| HomeNoResponseV3['fieldType']
| DropdownResponseV3['fieldType']
| RatingResponseV3['fieldType']
| NricResponseV3['fieldType']
| UenResponseV3['fieldType']
| DateResponseV3['fieldType']
| CountryRegionResponseV3['fieldType']

export type StringAnswerResponseV3 = string
export type YesNoFieldResponseV3 = 'Yes' | 'No'
export type VerifiableFieldResponsesV3 = {
export type VerifiableFieldResponseV3 = {
signature?: string
value: string
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ import {
SubmissionType,
} from '../../../../../shared/types'
import { isDev } from '../../../../app/config/config'
import { FormFieldSchema } from '../../../../types'
import {
ParsedClearAttachmentResponseV3,
ParsedClearFormFieldResponsesV3,
Expand All @@ -19,7 +20,9 @@ import {
import { MultirespondentFormLoadedDto } from '../../../../types/api/multirespondent_submission'
import formsgSdk from '../../../config/formsg-sdk'
import { createLoggerWithLabel } from '../../../config/logger'
import { validateFieldV3 } from '../../../utils/field-validation'
import {
FieldIdSet,
getLogicUnitPreventingSubmitV3,
getVisibleFieldIdsV3,
} from '../../../utils/logic-adaptor'
Expand All @@ -35,6 +38,7 @@ import {
InvalidSubmissionTypeError,
MaliciousFileDetectedError,
ProcessingError,
ValidateFieldError,
VirusScanFailedError,
} from '../submission.errors'
import {
Expand Down Expand Up @@ -308,11 +312,71 @@ export const scanAndRetrieveAttachments = async (
return next()
}

/**
* Validates each field by individual field rules.
* @param formId formId, used for logging
* @param formFields all form fields in the form. Purpose: used to validate responses against the form field properties.
* @param responses responses to validate
* @returns initial responses if all responses are valid, else an error.
*/
const validateMrfFieldResponses = ({
formId,
visibleFieldIds,
formFields,
responses,
}: {
formId: string
visibleFieldIds: FieldIdSet
formFields: FormFieldSchema[]
responses: ParsedClearFormFieldResponsesV3
}): Result<
ParsedClearFormFieldResponsesV3,
ValidateFieldError | ProcessingError
> => {
const idToFieldMap = formFields.reduce<{
[fieldId: string]: FormFieldSchema
}>((acc, field) => {
acc[field._id] = field
return acc
}, {})

for (const [responseId, response] of Object.entries(responses)) {
const formField = idToFieldMap[responseId]
if (!formField) {
return err(
new ProcessingError('Response Id does not match form field Ids'),
)
}

// Since Myinfo fields are not currently supported for MRF
if (response.fieldType === BasicField.Children) {
return err(
new ValidateFieldError(
'Children field type is not supported for MRF submisisons',
),
)
}

const validateFieldV3Result = validateFieldV3({
formId,
formField,
response,
isVisible: visibleFieldIds.has(responseId),
})
if (validateFieldV3Result.isErr()) {
return err(validateFieldV3Result.error)
}
}

return ok(responses)
}

/**
* What types of fields are there?
* Visible Not visible
* Editable Regular field validation Not allowed
* Non-editable Not allowed / prev submiss Not allowed
* | Visible | Not visible
* -------------|-----------------------------|-------------------
* Editable | Regular field validation | Not allowed
* Non-editable | Not allowed / prev submiss | Not allowed
*
* Initial submission:
* 1. Retrieve form object
Expand Down Expand Up @@ -513,8 +577,12 @@ export const validateMultirespondentSubmission = async (
).map(() => undefined)
})
.andThen(() =>
// TODO: Step 4: Validate each field content with each field's validator rules individually.
ok(undefined),
validateMrfFieldResponses({
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Since we're potentially blocking submissions here and we don't have full confidence that we're making the right blocks. Would recommend us to implement this through a "shadow"[1] instead.

Lmk if you have similar concerns on the confidence of the validation implementation.

[1] We log that a validation has failed, instead of block. Once we have sufficient information and confidence, we can officially swap that to a block.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

done, when a validation error for v3 occurs, ValidateFieldErrorV3 will be thrown which will be logged on DD and we can use that for monitoring for the next 1-2 weeks.

formId,
visibleFieldIds,
formFields: form_fields as FormFieldSchema[],
responses: req.body.responses,
}),
),
)
},
Expand Down
Loading
Loading