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

fix(mrf): attachment v2 #7281

Merged
merged 6 commits into from
May 3, 2024
Merged

fix(mrf): attachment v2 #7281

merged 6 commits into from
May 3, 2024

Conversation

KenLSM
Copy link
Contributor

@KenLSM KenLSM commented Apr 23, 2024

Problem

Encrypted content of MRF with attachments is huge as it contains the attachments contents.

Solution

Overview

First, we introduce a new backward-compatibility key mrfVersion in the multirespondent submission object. This is initialized as 1 for all new MRF submissions. Here are the differences between the two values.

Old version: mrfVersion === undefined

  • On submission
    • Response + attachments is saved to DB encrypted with submission key.
    • Attachment is also saved to S3 encrypted with form key.
  • On retrieval
    • Respondent flow (R2 onwards): response + attachment is retrieved from DB and decrypted with submission key.
    • Admin flow (individual response page, csv download): attachment is retrieved from S3 and decrypted with form key.

New version: mrfVersion === 1

  • On submission
    • Response is saved to DB encrypted with submission key.
    • Attachment is saved to S3 encrypted with submission key.
  • On retrieval
    • Respondent flow (R2 onwards): response is retrieved from DB and decrypted with submission key. attachment is retrieved from S3 and decrypted with submission key.
    • Admin flow (individual response page, csv download): attachment is retrieved from S3 and decrypted with form key (via encrypted submission key).

Changelog

Therefore, this PR does the following changes.

  • On submission
    1. Exclude attachment contents in the saved response encryptedContent
    2. Encrypt attachments using the submission key instead.
  • On retrieval (all of these must be backward compatible, thus gated by mrfVersion === 1 check)
    • Respondent flow
      1. a new type MultirespondentSubmissionDtoWithAttachments is created to allow for rehydration of the encrypted attachments
      2. Add rehydration of attachments during getMultirespondentSubmissionById. (This will slow down the initial load, but provides compatibility quickly. If the UX is slow, it might make sense for us to provide a loading screen to handle this.)
      3. evaluation of which attachment to attach as defaults to the existing form is streamlined into the useEffect in PublicFormProvider, via the previousAttachments variable, based on whether mrfVersion === 1
    • Admin flow
      1. Change Admin Response Page to use Submission Key to decrypt files for mrfVersion === 1 responses, since attachments are now encrypted with a different key

Breaking Changes

  • No - this PR is backwards compatible
    • Backwards compatibility is handled through mrfVersion: 1 on the encryptedContent.

Tests

Before deployment tests

Prepare for backward compatibility tests

  • Create an MRF with an attachment field (of less than 4MB).
  • Make a new submission with an attachment. This should be successful.
  • Run the following attachment loading tests:
    • In the admin individual response page, download the attachment. This should be successful.
    • In the admin individual response page, download all attachments as ZIP. This should be successful.
    • In the admin panel, export all form responses as CSV with attachments. This should be successful.
    • As respondent 2, load the public form. Download the attachment from the form page. This should be successful.

After deployment tests

Backward compatibility tests

For the submission that was made earlier,

  • Run the attachment loading tests defined above

End-to-end tests for the new protocol

  • Now, make a new submission with an attachment. This should be successful.
  • Run the attachment loading tests defined above
  • As respondent 2, submit the form with the default attachment. This should be successful.
  • Run the attachment loading tests defined above again

Ensure that attachments up to 20MB are supported
(note: this is not testable on staging as a result of infra-level limitations. test this on production as a manual test once the new version is released)

  • Adjust the form to allow two attachments of up to 10MB each
  • Submit two 9MB attachments. This should be successful.
  • Run the attachment loading tests defined above again

@KenLSM KenLSM marked this pull request as draft April 23, 2024 14:13
@KenLSM KenLSM requested a review from justynoh April 23, 2024 14:40
@justynoh justynoh marked this pull request as ready for review April 29, 2024 05:34
@KenLSM
Copy link
Contributor Author

KenLSM commented May 2, 2024

@justynoh LGTM for the admin changes! I couldn't approve this PR myself

Copy link
Contributor

@justynoh justynoh left a comment

Choose a reason for hiding this comment

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

lgtm'ed by ken

@justynoh justynoh merged commit ed00a82 into develop May 3, 2024
34 checks passed
@justynoh justynoh deleted the fix/mrf-attachment-v2 branch May 3, 2024 02:17
@KenLSM KenLSM mentioned this pull request May 5, 2024
15 tasks
kathleenkhy added a commit that referenced this pull request May 6, 2024
* fix(deps): bump zod from 3.23.4 to 3.23.5 in /shared (#7301)

Bumps [zod](https://github.com/colinhacks/zod) from 3.23.4 to 3.23.5.
- [Release notes](https://github.com/colinhacks/zod/releases)
- [Changelog](https://github.com/colinhacks/zod/blob/master/CHANGELOG.md)
- [Commits](colinhacks/zod@v3.23.4...v3.23.5)

---
updated-dependencies:
- dependency-name: zod
  dependency-type: direct:production
  update-type: version-update:semver-patch
...

Signed-off-by: dependabot[bot] <support@github.com>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>

* fix(deps): bump type-fest from 4.18.0 to 4.18.1 in /shared (#7303)

Bumps [type-fest](https://github.com/sindresorhus/type-fest) from 4.18.0 to 4.18.1.
- [Release notes](https://github.com/sindresorhus/type-fest/releases)
- [Commits](sindresorhus/type-fest@v4.18.0...v4.18.1)

---
updated-dependencies:
- dependency-name: type-fest
  dependency-type: direct:production
  update-type: version-update:semver-patch
...

Signed-off-by: dependabot[bot] <support@github.com>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>

* fix(deps): bump ejs from 3.1.8 to 3.1.10 (#7304)

Bumps [ejs](https://github.com/mde/ejs) from 3.1.8 to 3.1.10.
- [Release notes](https://github.com/mde/ejs/releases)
- [Commits](mde/ejs@v3.1.8...v3.1.10)

---
updated-dependencies:
- dependency-name: ejs
  dependency-type: direct:production
...

Signed-off-by: dependabot[bot] <support@github.com>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>

* fix(mrf): attachment v2 (#7281)

* fix: add localhost domain into cors, used by attachment uploads

* fix: remove attachment contents out of encrypted fields, update attachment encryption to submission level

* chore: fix typo on throw -> throws

* feat: handle attachments for admin flow, backward compatibility

* chore: update chromium version for production Dockerfile

* chore: update error

---------

Co-authored-by: Justyn Oh <justynoh@gmail.com>

* chore: bump version to v6.119.0

---------

Signed-off-by: dependabot[bot] <support@github.com>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
Co-authored-by: Justyn Oh <justynoh@gmail.com>
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