From 332d3ce8b7e44827d5e01b1cccef5b9cf03b7728 Mon Sep 17 00:00:00 2001 From: Frank Chen Date: Tue, 8 Jun 2021 04:34:28 -0700 Subject: [PATCH] feat: Add webhook support for storage mode attachments (#1713) * feat: Add webhook support for storage mode attachments * fix tests post rebase * remove merge error --- .../__tests__/submission.server.model.spec.ts | 6 + src/app/models/field/attachmentField.ts | 23 +-- src/app/models/submission.server.model.ts | 5 + .../webhook/__tests__/webhook.service.spec.ts | 3 + src/app/modules/webhook/webhook.errors.ts | 14 ++ src/app/modules/webhook/webhook.service.ts | 172 +++++++++++------- .../settings-form.client.view.html | 11 +- .../directives/edit-form.client.directive.js | 5 +- .../settings-form.client.directive.js | 8 - src/types/submission.ts | 1 + 10 files changed, 141 insertions(+), 107 deletions(-) diff --git a/src/app/models/__tests__/submission.server.model.spec.ts b/src/app/models/__tests__/submission.server.model.spec.ts index f726745928..8ee24f5d5d 100644 --- a/src/app/models/__tests__/submission.server.model.spec.ts +++ b/src/app/models/__tests__/submission.server.model.spec.ts @@ -68,6 +68,7 @@ describe('Submission Model', () => { isRetryEnabled: true, webhookView: { data: { + attachmentDownloadUrls: {}, formId: String(form._id), submissionId: String(submission._id), encryptedContent: MOCK_ENCRYPTED_CONTENT, @@ -120,6 +121,7 @@ describe('Submission Model', () => { isRetryEnabled: false, webhookView: { data: { + attachmentDownloadUrls: {}, formId: String(form._id), submissionId: String(submission._id), encryptedContent: MOCK_ENCRYPTED_CONTENT, @@ -155,6 +157,7 @@ describe('Submission Model', () => { isRetryEnabled: false, webhookView: { data: { + attachmentDownloadUrls: {}, formId: String(form._id), submissionId: String(submission._id), encryptedContent: MOCK_ENCRYPTED_CONTENT, @@ -268,6 +271,7 @@ describe('Submission Model', () => { created: expect.any(Date), encryptedContent: MOCK_ENCRYPTED_CONTENT, verifiedContent: undefined, + attachmentDownloadUrls: {}, version: 1, }, }) @@ -294,6 +298,7 @@ describe('Submission Model', () => { // Assert expect(actualWebhookView).toEqual({ data: { + attachmentDownloadUrls: {}, formId: expect.any(String), submissionId: expect.any(String), created: expect.any(Date), @@ -336,6 +341,7 @@ describe('Submission Model', () => { // Assert expect(actualWebhookView).toEqual({ data: { + attachmentDownloadUrls: {}, formId: expect.any(String), submissionId: expect.any(String), created: expect.any(Date), diff --git a/src/app/models/field/attachmentField.ts b/src/app/models/field/attachmentField.ts index 49ee6eadfe..80fe25ca69 100644 --- a/src/app/models/field/attachmentField.ts +++ b/src/app/models/field/attachmentField.ts @@ -1,11 +1,6 @@ import { Document, Schema } from 'mongoose' -import { - AttachmentSize, - IAttachmentField, - IFormSchema, - ResponseMode, -} from '../../../types' +import { AttachmentSize, IAttachmentField, IFormSchema } from '../../../types' // Manual override since mongoose types don't have generics yet. interface IAttachmentFieldSchema extends IAttachmentField, Document { @@ -24,22 +19,6 @@ const createAttachmentFieldSchema = () => { }, }) - // Prevent attachments from being saved on a webhooked form. - AttachmentFieldSchema.pre( - 'validate', - function (next) { - const { webhook, responseMode } = this.parent() - - if (responseMode === ResponseMode.Encrypt && webhook?.url) { - return next( - Error('Attachments are not allowed when a form has a webhook url'), - ) - } - - return next() - }, - ) - return AttachmentFieldSchema } diff --git a/src/app/models/submission.server.model.ts b/src/app/models/submission.server.model.ts index 2e37daae76..8c949bbedc 100644 --- a/src/app/models/submission.server.model.ts +++ b/src/app/models/submission.server.model.ts @@ -187,6 +187,10 @@ EncryptSubmissionSchema.methods.getWebhookView = function ( const formId = this.populated('form') ? String(this.form._id) : String(this.form) + const attachmentRecords = Object.fromEntries( + this.attachmentMetadata ?? new Map(), + ) + const webhookData: WebhookData = { formId, submissionId: String(this._id), @@ -194,6 +198,7 @@ EncryptSubmissionSchema.methods.getWebhookView = function ( verifiedContent: this.verifiedContent, version: this.version, created: this.created, + attachmentDownloadUrls: attachmentRecords, } return { diff --git a/src/app/modules/webhook/__tests__/webhook.service.spec.ts b/src/app/modules/webhook/__tests__/webhook.service.spec.ts index c763f183aa..8ecb70678c 100644 --- a/src/app/modules/webhook/__tests__/webhook.service.spec.ts +++ b/src/app/modules/webhook/__tests__/webhook.service.spec.ts @@ -94,6 +94,9 @@ describe('webhook.service', () => { formId: MOCK_FORM_ID, submissionId: MOCK_SUBMISSION_ID, verifiedContent: 'mockVerifiedContent', + attachmentDownloadUrls: { + 'some-field-id': 'https://mock.s3.url/some/s3/url/timeout=3600', + }, version: 1, }, } diff --git a/src/app/modules/webhook/webhook.errors.ts b/src/app/modules/webhook/webhook.errors.ts index 426d32f170..6ecc52f3d7 100644 --- a/src/app/modules/webhook/webhook.errors.ts +++ b/src/app/modules/webhook/webhook.errors.ts @@ -12,6 +12,20 @@ export class WebhookValidationError extends ApplicationError { } } +/** + * Webhook failed to generate S3 presigned URLs for attachments + */ +export class WebhookFailedWithPresignedUrlGenerationError extends ApplicationError { + meta: { + originalError: unknown + } + + constructor(error: unknown, message = 'Presigned Url Generation failed') { + super(message) + this.meta = { originalError: error } + } +} + /** * Webhook returned non-200 status, but error is not instance of AxiosError */ diff --git a/src/app/modules/webhook/webhook.service.ts b/src/app/modules/webhook/webhook.service.ts index be3f7feb19..ce6e16b69f 100644 --- a/src/app/modules/webhook/webhook.service.ts +++ b/src/app/modules/webhook/webhook.service.ts @@ -1,4 +1,5 @@ import axios from 'axios' +import Bluebird from 'bluebird' import { get } from 'lodash' import mongoose from 'mongoose' import { errAsync, okAsync, ResultAsync } from 'neverthrow' @@ -9,6 +10,7 @@ import { IWebhookResponse, WebhookView, } from '../../../types' +import { aws as AwsConfig } from '../../config/config' import formsgSdk from '../../config/formsg-sdk' import { createLoggerWithLabel } from '../../config/logger' import { getEncryptSubmissionModel } from '../../models/submission.server.model' @@ -18,6 +20,7 @@ import { SubmissionNotFoundError } from '../submission/submission.errors' import { WebhookFailedWithAxiosError, + WebhookFailedWithPresignedUrlGenerationError, WebhookFailedWithUnknownError, WebhookPushToQueueError, WebhookValidationError, @@ -72,10 +75,35 @@ export const saveWebhookRecord = ( }) } +const createWebhookSubmissionView = ( + submissionWebhookView: WebhookView, +): Promise => { + // Generate S3 signed urls + const signedUrlPromises: Record> = {} + for (const key in submissionWebhookView.data.attachmentDownloadUrls) { + signedUrlPromises[key] = AwsConfig.s3.getSignedUrlPromise('getObject', { + Bucket: AwsConfig.attachmentS3Bucket, + Key: submissionWebhookView.data.attachmentDownloadUrls[key], + Expires: 60 * 60, // one hour expiry + }) + } + + return Bluebird.props(signedUrlPromises).then((signedUrls) => { + submissionWebhookView.data.attachmentDownloadUrls = signedUrls + return submissionWebhookView + }) +} + export const sendWebhook = ( webhookView: WebhookView, webhookUrl: string, -): ResultAsync => { +): ResultAsync< + IWebhookResponse, + | WebhookValidationError + | WebhookFailedWithAxiosError + | WebhookFailedWithPresignedUrlGenerationError + | WebhookFailedWithUnknownError +> => { const now = Date.now() const { submissionId, formId } = webhookView.data @@ -104,77 +132,93 @@ export const sendWebhook = ( return error instanceof WebhookValidationError ? error : new WebhookValidationError() - }) - .andThen(() => - ResultAsync.fromPromise( - axios.post(webhookUrl, webhookView, { - headers: { - 'X-FormSG-Signature': formsgSdk.webhooks.constructHeader({ - epoch: now, - submissionId, - formId, - signature, - }), - }, - maxRedirects: 0, - // Timeout after 10 seconds to allow for cold starts in receiver, - // e.g. Lambdas - timeout: 10 * 1000, - }), - (error) => { - logger.error({ - message: 'Webhook POST failed', - meta: { - ...logMeta, - isAxiosError: axios.isAxiosError(error), - status: get(error, 'response.status'), - }, - error, - }) - if (axios.isAxiosError(error)) { - return new WebhookFailedWithAxiosError(error) - } - return new WebhookFailedWithUnknownError(error) - }, - ), + }).andThen(() => { + return ResultAsync.fromPromise( + createWebhookSubmissionView(webhookView), + (error) => { + logger.error({ + message: 'S3 attachment presigned URL generation failed', + meta: logMeta, + error, + }) + return new WebhookFailedWithPresignedUrlGenerationError(error) + }, ) - .map((response) => { - // Capture response for logging purposes - logger.info({ - message: 'Webhook POST succeeded', - meta: { - ...logMeta, - status: get(response, 'status'), - }, + .andThen((submissionWebhookView) => + ResultAsync.fromPromise( + axios.post(webhookUrl, submissionWebhookView, { + headers: { + 'X-FormSG-Signature': formsgSdk.webhooks.constructHeader({ + epoch: now, + submissionId, + formId, + signature, + }), + }, + maxRedirects: 0, + // Timeout after 10 seconds to allow for cold starts in receiver, + // e.g. Lambdas + timeout: 10 * 1000, + }), + (error) => { + logger.error({ + message: 'Webhook POST failed', + meta: { + ...logMeta, + isAxiosError: axios.isAxiosError(error), + status: get(error, 'response.status'), + }, + error, + }) + if (axios.isAxiosError(error)) { + return new WebhookFailedWithAxiosError(error) + } + return new WebhookFailedWithUnknownError(error) + }, + ), + ) + .map((response) => { + // Capture response for logging purposes + logger.info({ + message: 'Webhook POST succeeded', + meta: { + ...logMeta, + status: get(response, 'status'), + }, + }) + return { + signature, + webhookUrl, + response: formatWebhookResponse(response), + } }) - return { - signature, - webhookUrl, - response: formatWebhookResponse(response), - } - }) - .orElse((error) => { - // Webhook was not posted - if (error instanceof WebhookValidationError) return errAsync(error) + .orElse((error) => { + // Webhook was not posted + if (error instanceof WebhookValidationError) return errAsync(error) + + // S3 pre-signed URL generation failed + if (error instanceof WebhookFailedWithPresignedUrlGenerationError) + return errAsync(error) + + // Webhook was posted but failed + if (error instanceof WebhookFailedWithUnknownError) { + return okAsync({ + signature, + webhookUrl, + // Not Axios error so no guarantee of having response. + // Hence allow formatting function to return default shape. + response: formatWebhookResponse(), + }) + } - // Webhook was posted but failed - if (error instanceof WebhookFailedWithUnknownError) { + const axiosError = error.meta.originalError return okAsync({ signature, webhookUrl, - // Not Axios error so no guarantee of having response. - // Hence allow formatting function to return default shape. - response: formatWebhookResponse(), + response: formatWebhookResponse(axiosError.response), }) - } - - const axiosError = error.meta.originalError - return okAsync({ - signature, - webhookUrl, - response: formatWebhookResponse(axiosError.response), }) - }) + }) } /** diff --git a/src/public/modules/forms/admin/directiveViews/settings-form.client.view.html b/src/public/modules/forms/admin/directiveViews/settings-form.client.view.html index 5b6f67c24e..7169ccec1c 100644 --- a/src/public/modules/forms/admin/directiveViews/settings-form.client.view.html +++ b/src/public/modules/forms/admin/directiveViews/settings-form.client.view.html @@ -379,15 +379,6 @@ (optional) -
- - Webhook is not available for forms with attachment fields. -
{ - return ( - $scope.myform.form_fields.filter( - (field) => field.fieldType == 'attachment', - ).length > 0 - ) - } - // Warning message when turning off SP with MyInfo fields $scope.myInfoSPWarning = () => { return ( diff --git a/src/types/submission.ts b/src/types/submission.ts index b1aaabee5c..3c2e640352 100644 --- a/src/types/submission.ts +++ b/src/types/submission.ts @@ -51,6 +51,7 @@ export interface WebhookData { verifiedContent: IEncryptedSubmissionSchema['verifiedContent'] version: IEncryptedSubmissionSchema['version'] created: IEncryptedSubmissionSchema['created'] + attachmentDownloadUrls: Record } export interface WebhookView {