From 64b10573e7fbb14cb024fe97b3b8e852746f28be Mon Sep 17 00:00:00 2001 From: Frank Chen Date: Tue, 8 Jun 2021 03:06:23 +0000 Subject: [PATCH 1/3] feat: Add webhook support for storage mode attachments --- .../__tests__/submission.server.model.spec.ts | 1 + src/app/models/field/attachmentField.ts | 23 +-- src/app/models/submission.server.model.ts | 5 + .../webhook/__tests__/webhook.service.spec.ts | 2 + src/app/modules/webhook/webhook.errors.ts | 14 ++ src/app/modules/webhook/webhook.service.ts | 137 ++++++++++++++---- .../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, 132 insertions(+), 75 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..efcaa704ef 100644 --- a/src/app/models/__tests__/submission.server.model.spec.ts +++ b/src/app/models/__tests__/submission.server.model.spec.ts @@ -268,6 +268,7 @@ describe('Submission Model', () => { created: expect.any(Date), encryptedContent: MOCK_ENCRYPTED_CONTENT, verifiedContent: undefined, + attachmentDownloadUrls: {}, version: 1, }, }) 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..0c5deaff90 100644 --- a/src/app/modules/webhook/__tests__/webhook.service.spec.ts +++ b/src/app/modules/webhook/__tests__/webhook.service.spec.ts @@ -4,6 +4,7 @@ import mongoose from 'mongoose' import { ok, okAsync } from 'neverthrow' import { mocked } from 'ts-jest/utils' +import { aws as AwsConfig } from 'src/app/config/config' import formsgSdk from 'src/app/config/formsg-sdk' import { getEncryptSubmissionModel } from 'src/app/models/submission.server.model' import { WebhookValidationError } from 'src/app/modules/webhook/webhook.errors' @@ -94,6 +95,7 @@ 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..5bc7e40a91 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,11 +75,37 @@ 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 rawSubmissionWebhookView = submission.getWebhookView() const { submissionId, formId } = webhookView.data const signature = formsgSdk.webhooks.generateSignature({ @@ -104,7 +133,18 @@ export const sendWebhook = ( return error instanceof WebhookValidationError ? error : new WebhookValidationError() - }) + }).andThen(() => { + return ResultAsync.fromPromise( + createWebhookSubmissionView(rawSubmissionWebhookView), + (error) => { + logger.error({ + message: 'S3 attachment presigned URL generation failed', + meta: logMeta, + error, + }) + return new WebhookFailedWithPresignedUrlGenerationError(error) + }, + ) .andThen(() => ResultAsync.fromPromise( axios.post(webhookUrl, webhookView, { @@ -138,43 +178,76 @@ export const sendWebhook = ( }, ), ) - .map((response) => { - // Capture response for logging purposes - logger.info({ - message: 'Webhook POST succeeded', - meta: { - ...logMeta, - status: get(response, 'status'), - }, + .andThen((submissionWebhookView) => { + return ResultAsync.fromPromise( + axios.post(webhookUrl, submissionWebhookView, { + headers: { + 'X-FormSG-Signature': formsgSdk.webhooks.constructHeader({ + epoch: now, + submissionId, + formId, + signature, + }), + }, + maxRedirects: 0, + }), + (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) + }, + ) }) - return { - signature, - webhookUrl, - response: formatWebhookResponse(response), - } - }) - .orElse((error) => { - // Webhook was not posted - if (error instanceof WebhookValidationError) return errAsync(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), + } + }) + .orElse((error) => { + // Webhook was not posted + if (error instanceof WebhookValidationError) return errAsync(error) + 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 { From f0f0ca228b7223b8315aea810f4947eb8f3bba3a Mon Sep 17 00:00:00 2001 From: Frank Chen Date: Tue, 8 Jun 2021 04:12:13 +0000 Subject: [PATCH 2/3] fix tests post rebase --- .../__tests__/submission.server.model.spec.ts | 5 ++ .../webhook/__tests__/webhook.service.spec.ts | 5 +- src/app/modules/webhook/webhook.service.ts | 69 +++++++++---------- 3 files changed, 42 insertions(+), 37 deletions(-) diff --git a/src/app/models/__tests__/submission.server.model.spec.ts b/src/app/models/__tests__/submission.server.model.spec.ts index efcaa704ef..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, @@ -295,6 +298,7 @@ describe('Submission Model', () => { // Assert expect(actualWebhookView).toEqual({ data: { + attachmentDownloadUrls: {}, formId: expect.any(String), submissionId: expect.any(String), created: expect.any(Date), @@ -337,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/modules/webhook/__tests__/webhook.service.spec.ts b/src/app/modules/webhook/__tests__/webhook.service.spec.ts index 0c5deaff90..8ecb70678c 100644 --- a/src/app/modules/webhook/__tests__/webhook.service.spec.ts +++ b/src/app/modules/webhook/__tests__/webhook.service.spec.ts @@ -4,7 +4,6 @@ import mongoose from 'mongoose' import { ok, okAsync } from 'neverthrow' import { mocked } from 'ts-jest/utils' -import { aws as AwsConfig } from 'src/app/config/config' import formsgSdk from 'src/app/config/formsg-sdk' import { getEncryptSubmissionModel } from 'src/app/models/submission.server.model' import { WebhookValidationError } from 'src/app/modules/webhook/webhook.errors' @@ -95,7 +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'}, + attachmentDownloadUrls: { + 'some-field-id': 'https://mock.s3.url/some/s3/url/timeout=3600', + }, version: 1, }, } diff --git a/src/app/modules/webhook/webhook.service.ts b/src/app/modules/webhook/webhook.service.ts index 5bc7e40a91..ac7a46173f 100644 --- a/src/app/modules/webhook/webhook.service.ts +++ b/src/app/modules/webhook/webhook.service.ts @@ -98,14 +98,13 @@ export const sendWebhook = ( webhookView: WebhookView, webhookUrl: string, ): ResultAsync< - IWebhookResponse, + IWebhookResponse, | WebhookValidationError | WebhookFailedWithAxiosError | WebhookFailedWithPresignedUrlGenerationError | WebhookFailedWithUnknownError > => { const now = Date.now() - const rawSubmissionWebhookView = submission.getWebhookView() const { submissionId, formId } = webhookView.data const signature = formsgSdk.webhooks.generateSignature({ @@ -135,7 +134,7 @@ export const sendWebhook = ( : new WebhookValidationError() }).andThen(() => { return ResultAsync.fromPromise( - createWebhookSubmissionView(rawSubmissionWebhookView), + createWebhookSubmissionView(webhookView), (error) => { logger.error({ message: 'S3 attachment presigned URL generation failed', @@ -145,39 +144,39 @@ export const sendWebhook = ( return new WebhookFailedWithPresignedUrlGenerationError(error) }, ) - .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'), + .andThen((submissionWebhookView) => + ResultAsync.fromPromise( + axios.post(webhookUrl, submissionWebhookView, { + headers: { + 'X-FormSG-Signature': formsgSdk.webhooks.constructHeader({ + epoch: now, + submissionId, + formId, + signature, + }), }, - error, - }) - if (axios.isAxiosError(error)) { - return new WebhookFailedWithAxiosError(error) - } - return new WebhookFailedWithUnknownError(error) - }, - ), - ) + 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((submissionWebhookView) => { return ResultAsync.fromPromise( axios.post(webhookUrl, submissionWebhookView, { From 9c80633f1dcea811b3170c97871951bff3374230 Mon Sep 17 00:00:00 2001 From: Frank Chen Date: Tue, 8 Jun 2021 06:18:11 +0000 Subject: [PATCH 3/3] remove merge error --- src/app/modules/webhook/webhook.service.ts | 32 ++-------------------- 1 file changed, 2 insertions(+), 30 deletions(-) diff --git a/src/app/modules/webhook/webhook.service.ts b/src/app/modules/webhook/webhook.service.ts index ac7a46173f..ce6e16b69f 100644 --- a/src/app/modules/webhook/webhook.service.ts +++ b/src/app/modules/webhook/webhook.service.ts @@ -177,36 +177,6 @@ export const sendWebhook = ( }, ), ) - .andThen((submissionWebhookView) => { - return ResultAsync.fromPromise( - axios.post(webhookUrl, submissionWebhookView, { - headers: { - 'X-FormSG-Signature': formsgSdk.webhooks.constructHeader({ - epoch: now, - submissionId, - formId, - signature, - }), - }, - maxRedirects: 0, - }), - (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({ @@ -225,6 +195,8 @@ export const sendWebhook = ( .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)