Skip to content

fix: allow empty field titles in decrypted responses#86

Merged
justynoh merged 1 commit intodevelopfrom
fix/allow-empty-field-titles
Dec 16, 2022
Merged

fix: allow empty field titles in decrypted responses#86
justynoh merged 1 commit intodevelopfrom
fix/allow-empty-field-titles

Conversation

@justynoh
Copy link
Contributor

Problem

On 14 Nov 2022, a bug was identified in FormSG that allowed admins to create fields with empty field titles via the UI. As an unintended consequence, form responses to such storage mode forms became undecryptable. However, this was not caught to be an issue at the time, and thus was not communicated to form admins. This issue only came to light on 14 Dec 2022, when an admin of an affected form ended up not being able to decrypt form responses.

Solution

This PR changes the decrypted response validator to check for the type of the field title, in order to allow for empty strings. It also adds a test for this in the spec.

Copy link
Contributor

@karrui karrui left a comment

Choose a reason for hiding this comment

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

Lgtm

expect(decrypted).toHaveProperty('responses', plaintextMultiLang)
})

it('should be able to encrypt and decrypt submissions with empty field titles from 2022-11-14 end-to-end successfully', () => {
Copy link
Contributor

Choose a reason for hiding this comment

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

Don't need so specific test la haha, just say can decrypt empty strings

Copy link
Contributor Author

Choose a reason for hiding this comment

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

hahah oops i follow format only xD

@justynoh justynoh merged commit 504be65 into develop Dec 16, 2022
@justynoh justynoh deleted the fix/allow-empty-field-titles branch December 16, 2022 05:22
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants