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

refactor/submit-form: reduce possibility of race conditions in submission flow #1045

Merged
merged 7 commits into from
Jan 26, 2021
72 changes: 18 additions & 54 deletions src/public/modules/forms/base/directives/submit-form.directive.js
Original file line number Diff line number Diff line change
Expand Up @@ -6,8 +6,6 @@ const {
getLogicUnitPreventingSubmit,
} = require('../../../../../shared/util/logic')

// The current encrypt version to assign to the encrypted submission.
const ENCRYPT_VERSION = 1
/**
* @typedef {number} FormState
*/
Expand Down Expand Up @@ -83,9 +81,11 @@ function submitFormDirective(
progressModal: null,
submitPrevented: false,
}

// Also used to store a backup of the form state during submission, the state
// of the progress modal
const controllerState = {}
scope.controllerState = {}

scope.hasMyInfoFields = FormFields.containsMyInfoFields(scope.form)
scope.formLogin = function (authType, rememberMe) {
// Fire GA tracking event
Expand Down Expand Up @@ -211,7 +211,7 @@ function submitFormDirective(
}
break
case FORM_STATES.SUBMITTING:
controllerState.savedForm = cloneDeep(scope.form)
scope.controllerState.savedForm = cloneDeep(scope.form)
scope.form.lockFields()
scope.uiState.submitButtonClicked = true
if (scope.form.hasAttachments()) {
Expand All @@ -231,7 +231,7 @@ function submitFormDirective(
}
break
case FORM_STATES.SUBMISSION_ERROR:
scope.form = controllerState.savedForm
scope.form = scope.controllerState.savedForm
setFormState(FORM_STATES.DEFAULT)
break
case FORM_STATES.SUBMIT_PREVENTED:
Expand All @@ -251,7 +251,7 @@ function submitFormDirective(
* Opens the submission progress modal.
*/
const openProgressModal = function () {
controllerState.progressModal = $uibModal.open({
scope.controllerState.progressModal = $uibModal.open({
animation: true,
backdrop: 'static',
keyboard: false,
Expand All @@ -265,9 +265,9 @@ function submitFormDirective(
* Closes the submission progress modal if it is currently open.
*/
const closeProgressModal = () => {
if (controllerState.progressModal) {
controllerState.progressModal.close()
controllerState.progressModal = null
if (scope.controllerState.progressModal) {
scope.controllerState.progressModal.close()
scope.controllerState.progressModal = null
}
}

Expand All @@ -276,9 +276,8 @@ function submitFormDirective(
* and handle any errors.
*/
scope.submitForm = () => {
let form = cloneDeep(scope.form)
try {
submitFormMain(form)
submitFormMain(scope.form)
} catch (error) {
handleSubmitFailure(error, 'Please try again later.')
}
Expand All @@ -292,36 +291,21 @@ function submitFormDirective(
// Disable UI and optionally open progress modal while processing
setFormState(FORM_STATES.SUBMITTING)

// submissionContent is the POST body to backend when we submit the form
let submissionContent

try {
form.attachments = await form.getAttachments()
submissionContent = Object.assign(
{ captchaResponse: captchaService.response },
await form.getSubmissionContent(),
)
} catch (err) {
return handleSubmitFailure(
err,
'Could not encrypt your attachments. Please contact the form administrator.',
'Could not prepare your submission. Please contact the form administrator.',
)
}

const responses = form.getResponses()

// submissionContent is the POST body to backend when we submit the form
let submissionContent = {
attachments: form.attachments,
captchaResponse: captchaService.response,
isPreview: form.isPreview,
responses,
}
if (form.responseMode === responseModeEnum.ENCRYPT && form.publicKey) {
try {
encryptSubmissionContent(submissionContent, form.publicKey)
submissionContent.version = ENCRYPT_VERSION
} catch (err) {
return handleSubmitFailure(
err,
'Could not encrypt your submission. Please contact the form administrator.',
)
}
}

Submissions.post(
{
formId: scope.form._id,
Expand All @@ -331,26 +315,6 @@ function submitFormDirective(
).then(handleSubmitSuccess, handleSubmitFailure)
}

/**
* Encrypts submission content for end-to-end encryption.
* @param {Object} submissionContent Content to encrypt
* @param {String} publicKey Encryption public key
*/
const encryptSubmissionContent = (submissionContent, publicKey) => {
submissionContent.encryptedContent = FormSgSdk.crypto.encrypt(
submissionContent.responses,
publicKey,
)
submissionContent.responses = submissionContent.responses
.filter((item) => ['mobile', 'email'].includes(item.fieldType))
.map((item) => {
return _(item)
.pick(['fieldType', '_id', 'answer', 'signature'])
.omitBy(_.isNull)
.value()
})
}

/**
* Helper function for Google Analytics to check for persistent login.
*/
Expand Down
65 changes: 61 additions & 4 deletions src/public/modules/forms/viewmodels/Form.class.js
Original file line number Diff line number Diff line change
@@ -1,3 +1,6 @@
const _ = require('lodash')
const formsg = require('@opengovsg/formsg-sdk')()
Copy link
Contributor

Choose a reason for hiding this comment

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

we are initialising the SDK in production mode here, even if the code is running in dev/staging. this works fine for now because the SDK's crypto.encrypt function does not depend on FormSG's public key, but may lead to unexpected behaviour in future if this class has to call other SDK methods.


const FieldFactory = require('../helpers/field-factory')
const {
getEncryptedAttachmentsMap,
Expand All @@ -6,6 +9,11 @@ const {
} = require('../helpers/attachments-map')
const { NoAnswerField } = require('./Fields')

// The current encrypt version to assign to the encrypted submission.
// This is needed if we ever break backwards compatibility with
// end-to-end encryption
const ENCRYPT_VERSION = 1

/**
* Deserialises raw form object returned by backend and
* manages form-wide operations.
Expand All @@ -31,10 +39,11 @@ class Form {
}

/**
* Gets the submitted values of all fields.
* Internal helper function to get the submitted values of all fields
* that are supposed to have answers.
* @returns {Array} Array of response objects.
*/
getResponses() {
_getResponses() {
return this.form_fields
.filter((field) => !(field instanceof NoAnswerField))
.map((field) => {
Expand All @@ -46,10 +55,12 @@ class Form {
}

/**
* Creates a map of field ID to attachment file.
* Internal helper function that creates a map of field ID to attachment file.
* The values of the map are encrypted for Storage Mode.
* forms.
* @returns {Object} Map of { id: file }
*/
getAttachments() {
_getAttachments() {
const attachmentsMap = getAttachmentsMap(this.form_fields)
if (this.responseMode === 'encrypt') {
return getEncryptedAttachmentsMap(attachmentsMap, this.publicKey)
Expand All @@ -65,6 +76,52 @@ class Form {
hasAttachments() {
return this.form_fields.some(fieldHasAttachment)
}

/**
* Gets the encrypted values of the responses. Only applicable to
* Storage Mode forms.
*/
_getEncryptedContent() {
if (this.responseMode === 'encrypt') {
return formsg.crypto.encrypt(this._getResponses(), this.publicKey)
}
return null
}

/**
* Method to abstract away edge cases for submission responses in email vs encrypt mode
*/
_getResponsesForSubmission() {
if (this.responseMode === 'encrypt') {
// Edge case: We still send mobile and email fields to the server in plaintext
// even with end-to-end encryption in order to support SMS and email autoreplies
return this._getResponses()
.filter((item) => ['mobile', 'email'].includes(item.fieldType))
.map((item) => {
return _(item)
.pick(['fieldType', '_id', 'answer', 'signature'])
.omitBy(_.isNull)
.value()
})
} else return this._getResponses()
}

/**
* Method to determine what to POST to the backend submission endpoint.
* Does not include captcha verification.
*/
async getSubmissionContent() {
const submissionContent = {
attachments: await this._getAttachments(),
isPreview: this.isPreview,
responses: this._getResponsesForSubmission(),
}
if (this.responseMode === 'encrypt') {
submissionContent.encryptedContent = this._getEncryptedContent()
submissionContent.version = ENCRYPT_VERSION
}
return submissionContent
}
}

module.exports = Form