Skip to content

Commit c769f06

Browse files
authored
PLU-337: Allow attachments in postman (#821)
## Problem Users want to upload a fixed file when sending email via Postman. ## Solution Allow uploads at Postman step for users to send a fixed file with every email. Note: attachments are available for use in all Postman steps of the same Pipe **Features**: - Upload fixed file when sending email via Postman **Improvements**: - Improve attachment selector to be consistent with other variable selectors - Improve `toPrettyDateString` function to convert `iso` and `ms` inputs to date strings - Improve `FileUpload` component to show as an IconButton or with text ## Before & After Screenshots **BEFORE**: <img width="858" alt="Screenshot 2024-12-16 at 1 43 29 PM" src="https://github.com/user-attachments/assets/292a5a27-69b2-4058-8c92-5f186f7bc5d3" /> **AFTER**: <img width="862" alt="Screenshot 2025-03-11 at 10 13 49 PM" src="https://github.com/user-attachments/assets/82fc5f68-5b9b-4ac2-aaa9-ebf777f3bc00" /> <img width="862" alt="Screenshot 2025-03-11 at 10 14 02 PM" src="https://github.com/user-attachments/assets/8d5cc665-7ddb-4a0a-8e75-d29e19ff1044" /> <img width="959" alt="Screenshot 2025-03-11 at 10 14 29 PM" src="https://github.com/user-attachments/assets/a8a4b1d5-4195-4e5f-9b10-c7d5dc747674" /> ## Tests - [x] Existing Pipes with variable attachments are populated and checked correctly - [x] Existing Pipes can handle uploaded attachments and send emails correctly with existing variable attachments - [x] New Pipes can handle both FormSG, LetterSG and uploaded attachments - [x] When deleting a file in a Pipe, file should be deleted from all Steps in the Pipe (ensures that Pipe does not fail) - [x] When deleting a Pipe, files uploaded to the Pipe should be deleted from S3 ## Deploy Notes - Note: infra updates required to add permissions for Malware Protection and `GetObjectTagging` **New scripts**: - `packages/backend/src/graphql/__tests__/mutations/delete-from-s3.itest.ts` : tests for functions to delete from s3 - `packages/backend/src/graphql/__tests__/mutations/flow.mock.ts` : mocks to create mock flow and step - `packages/backend/src/graphql/__tests__/mutations/generate-presigned-url.itest.ts` : tests for function to generate pre-signed URL to upload files to S3 - `packages/backend/src/graphql/mutations/delete-from-s3.ts`: function to delete files from S3 and corresponding steps - `packages/backend/src/graphql/mutations/generate-presigned-url.ts`: function to generate pre-signed URL for uploading file to S3 - `packages/frontend/src/components/AttachmentSuggestions/components/Checkbox.tsx`: component to display attachments as checkboxes - `packages/frontend/src/components/AttachmentSuggestions/components/Suggestions.tsx`: component to display attachments in same format as variables - `packages/frontend/src/components/AttachmentSuggestions/components/TagList.tsx`: component to display selected files as a Tag, with close button to remove - `packages/frontend/src/components/AttachmentSuggestions/hooks/useAttachmentSelection.ts`: hook to manage attachment selection - `packages/frontend/src/components/AttachmentSuggestions/index.tsx`: component for suggesting attachment variables or upload fixed attachments - `packages/frontend/src/components/AttachmentSuggestions/style.ts`: to manage styles for AttachmentSuggestions -`packages/frontend/src/components/AttachmentSuggestions/utils.ts`: helpers for attachments including file type and size validation, formatters to file size display, reformatting files to correct format for saving - `packages/frontend/src/components/SuggestionsWrapper/index.tsx`: generic wrapper for suggestions panel - `packages/frontend/src/graphql/mutations/delete-from-s3.ts`: to invoke DeleteFromS3 - `packages/frontend/src/graphql/mutations/generate-presigned-url.ts`: to call - `packages/frontend/src/hooks/useS3Operations.ts`: hook to perform upload and delete from S3 - `packages/frontend/src/theme/components/Checkbox.ts`: set style for Checkbox **New dependencies**: - `@aws-sdk/s3-request-presigner` : to generate pre-signed URL for uploading attachments to S3
1 parent 5df3814 commit c769f06

File tree

40 files changed

+2292
-93
lines changed

40 files changed

+2292
-93
lines changed

package-lock.json

Lines changed: 33 additions & 0 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

packages/backend/package.json

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -31,6 +31,7 @@
3131
"@apollo/server": "4.9.4",
3232
"@aws-sdk/client-dynamodb": "3.460.0",
3333
"@aws-sdk/client-s3": "3.369.0",
34+
"@aws-sdk/s3-request-presigner": "3.369.0",
3435
"@bull-board/express": "5.17.0",
3536
"@graphql-tools/schema": "10.0.0",
3637
"@launchdarkly/node-server-sdk": "9.0.4",

packages/backend/src/apps/postman/actions/send-transactional-email/index.ts

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -75,7 +75,11 @@ const action: IRawAction = {
7575
result.data.attachments?.map(async (attachment) => {
7676
// We verify the flowId here to ensure that the attachment is from the same flow and not
7777
// maliciously/ manually injected by another user who does not have access to this attachment
78-
const obj = await getObjectFromS3Id(attachment, { flowId: $.flow.id })
78+
const obj = await getObjectFromS3Id(
79+
attachment,
80+
{ flowId: $.flow.id },
81+
$,
82+
)
7983
return { fileName: obj.name, data: obj.data }
8084
}),
8185
)

packages/backend/src/apps/postman/common/parameters.ts

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -80,8 +80,8 @@ export const transactionalEmailFields: IField[] = [
8080
label: 'Attachments',
8181
key: 'attachments',
8282
description:
83-
'Check supported file types [here](https://guide.postman.gov.sg/email-api-guide/programmatic-email-api/send-email-api/attachments#list-of-supported-attachment-file-types).',
84-
type: 'multiselect' as const,
83+
'Check supported file types [here](https://guide.postman.gov.sg/email-api-guide/programmatic-email-api/send-email-api/attachments#list-of-supported-attachment-file-types).\nPlease note that the maximum file size for each file is 2MB, and the total size of all attachments cannot exceed 10MB.',
84+
type: 'attachment' as const,
8585
required: false,
8686
variables: true,
8787
variableTypes: ['file'],
Lines changed: 114 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,114 @@
1+
import { randomUUID } from 'crypto'
2+
import { beforeEach, describe, expect, it, vi } from 'vitest'
3+
4+
import { ForbiddenError } from '@/errors/graphql-errors'
5+
import deleteUploadedFile from '@/graphql/mutations/delete-uploaded-file'
6+
import Flow from '@/models/flow'
7+
import Context from '@/types/express/context'
8+
9+
import { generateMockContext } from './tiles/table.mock'
10+
import { generateMockFlow, generateMockStep } from './flow.mock'
11+
12+
const mockFlowId = '8c2a70d1-e78b-431e-9069-a4d8f97883f6'
13+
const mockBucket = 'test-bucket'
14+
const mockObjectName = 'some-file.jpg'
15+
const mockObjectKey = `${mockFlowId}s/${mockObjectName}`
16+
const mockS3Id = `s3:${mockBucket}:${mockFlowId}/${randomUUID()}/${mockObjectName}`
17+
18+
const mocks = vi.hoisted(() => ({
19+
s3Client: {
20+
send: vi.fn(() => ({
21+
$metadata: {
22+
httpStatusCode: 200,
23+
},
24+
})),
25+
},
26+
parseS3Id: vi.fn(() => ({
27+
bucket: mockBucket,
28+
objectKey: mockObjectKey,
29+
objectName: mockObjectName,
30+
})),
31+
PutObjectCommand: vi.fn().mockImplementation((input) => ({ input })),
32+
}))
33+
34+
vi.mock('@aws-sdk/client-s3', () => ({
35+
S3Client: function () {
36+
return mocks.s3Client
37+
},
38+
PutObjectCommand: mocks.PutObjectCommand,
39+
GetObjectCommand: vi.fn(),
40+
DeleteObjectsCommand: vi.fn(),
41+
}))
42+
43+
function createMockS3Id(mockFileName: string) {
44+
return `s3:${mockBucket}:${mockFlowId}/${randomUUID()}/${mockFileName}.txt`
45+
}
46+
47+
async function createMockStep(
48+
context: Context,
49+
flowId: string,
50+
position: number,
51+
params: Record<string, any>,
52+
config: Record<string, any> = {},
53+
) {
54+
return generateMockStep(
55+
context,
56+
'sendTransactionalEmail',
57+
'postman',
58+
'action',
59+
flowId,
60+
position,
61+
params,
62+
config,
63+
)
64+
}
65+
66+
describe('deleteFromS3', () => {
67+
let context: Context
68+
beforeEach(async () => {
69+
vi.clearAllMocks()
70+
context = await generateMockContext()
71+
})
72+
73+
it('should successfully delete an object when user owns the flow', async () => {
74+
await generateMockFlow(context, mockFlowId)
75+
76+
await expect(
77+
deleteUploadedFile(null, { id: mockS3Id }, context),
78+
).resolves.toBe(true)
79+
})
80+
81+
it('should delete object from all other steps within a flow', async () => {
82+
const fileToDelete = createMockS3Id('test_1')
83+
await generateMockFlow(context, mockFlowId)
84+
await createMockStep(context, mockFlowId, 2, {
85+
body: 'Test body',
86+
attachments: [fileToDelete],
87+
})
88+
await createMockStep(context, mockFlowId, 2, {
89+
body: 'Test body',
90+
attachments: [fileToDelete, createMockS3Id('test_2')],
91+
})
92+
93+
await deleteUploadedFile(null, { id: fileToDelete }, context)
94+
const postDeleteSteps = await context.currentUser
95+
.$relatedQuery('steps')
96+
.where({ flow_id: mockFlowId })
97+
98+
postDeleteSteps.forEach((step) => {
99+
expect(step.parameters.attachments).not.toContain(fileToDelete)
100+
})
101+
})
102+
103+
it('should throw an error if user does not have access to the flow', async () => {
104+
vi.fn()
105+
.mockRejectedValue(Flow.hasAccess)
106+
.mockRejectedValue(
107+
new ForbiddenError('You do not have access to this flow'),
108+
)
109+
110+
await expect(
111+
deleteUploadedFile(null, { id: mockS3Id }, context),
112+
).rejects.toThrow(ForbiddenError)
113+
})
114+
})
Lines changed: 72 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,72 @@
1+
import Flow from '@/models/flow'
2+
import Step from '@/models/step'
3+
import Context from '@/types/express/context'
4+
5+
export async function generateMockFlow(
6+
context: Context,
7+
id: string,
8+
config?: Record<string, any>,
9+
) {
10+
await Flow.query().insert({
11+
id,
12+
name: 'Test Flow',
13+
userId: context.currentUser.id,
14+
config: config ?? {},
15+
})
16+
}
17+
18+
export async function generateMockStep(
19+
context: Context,
20+
key:
21+
| 'delayFor'
22+
| 'everyHour'
23+
| 'getCellValues'
24+
| 'NULL'
25+
| 'findMessage'
26+
| 'ifThen'
27+
| 'everyDay'
28+
| 'getTableRow'
29+
| 'createTableRow'
30+
| 'sendTransactionalEmail'
31+
| 'onlyContinueIf'
32+
| 'createTileRow'
33+
| 'dateTime'
34+
| 'httpRequest'
35+
| 'everyMonth'
36+
| 'createLetter'
37+
| 'updateSingleRow'
38+
| 'catchRawWebhook'
39+
| 'newSubmission'
40+
| 'findSingleRow'
41+
| 'performCalculation',
42+
appKey:
43+
| null
44+
| 'slack'
45+
| 'm365-excel'
46+
| 'custom-api'
47+
| 'webhook'
48+
| 'delay'
49+
| 'lettersg'
50+
| 'formatter'
51+
| 'tiles'
52+
| 'scheduler'
53+
| 'postman'
54+
| 'formsg'
55+
| 'calculator'
56+
| 'toolbox',
57+
type: 'action' | 'trigger',
58+
flowId: string,
59+
position: number,
60+
parameters?: Record<string, any>,
61+
config?: Record<string, any>,
62+
) {
63+
await Step.query().insert({
64+
key,
65+
appKey,
66+
type,
67+
flowId,
68+
position,
69+
parameters: parameters ?? {},
70+
config: config ?? {},
71+
})
72+
}
Lines changed: 107 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,107 @@
1+
import { beforeEach, describe, expect, it, vi } from 'vitest'
2+
3+
import { ForbiddenError } from '@/errors/graphql-errors'
4+
import { generateMockContext } from '@/graphql/__tests__/mutations/tiles/table.mock'
5+
import generatePresignedUrl from '@/graphql/mutations/generate-presigned-url'
6+
import Flow from '@/models/flow'
7+
import Context from '@/types/express/context'
8+
9+
const VALID_PARAMS = {
10+
flowId: '193040de-c818-4a0c-90f9-1dcfb1963f53',
11+
filename: 'test.txt',
12+
fileType: 'text/plain',
13+
size: 100,
14+
updatedAt: new Date().toISOString(),
15+
manualUpload: true,
16+
}
17+
18+
const mocks = vi.hoisted(() => ({
19+
getSignedUrl: vi.fn(),
20+
}))
21+
22+
vi.mock('@aws-sdk/s3-request-presigner', () => ({
23+
getSignedUrl: mocks.getSignedUrl,
24+
}))
25+
26+
describe('generatePresignedUrl', () => {
27+
let context: Context
28+
beforeEach(async () => {
29+
context = await generateMockContext()
30+
})
31+
32+
it('should generate a presigned url', async () => {
33+
const expectedUrl = 'https://presigned-url.example.com'
34+
mocks.getSignedUrl.mockResolvedValueOnce(expectedUrl)
35+
36+
await Flow.query().insert({
37+
id: VALID_PARAMS.flowId,
38+
name: 'Test Flow',
39+
userId: context.currentUser.id,
40+
})
41+
42+
const result = await generatePresignedUrl(
43+
null,
44+
{ input: VALID_PARAMS },
45+
context,
46+
)
47+
const expectedKeys = ['url', 's3Id']
48+
expect(Object.keys(result)).toEqual(expectedKeys)
49+
expect(result.s3Id).toContain(VALID_PARAMS.flowId)
50+
})
51+
52+
it('should throw an error if the user does not have access to the flow', async () => {
53+
vi.fn()
54+
.mockRejectedValue(Flow.hasAccess)
55+
.mockRejectedValue(
56+
new ForbiddenError('You do not have access to this flow'),
57+
)
58+
59+
await expect(
60+
generatePresignedUrl(null, { input: VALID_PARAMS }, context),
61+
).rejects.toThrow(ForbiddenError)
62+
})
63+
64+
it('should throw an error if the file size is too large', async () => {
65+
const expectedUrl = 'https://presigned-url.example.com'
66+
mocks.getSignedUrl.mockResolvedValueOnce(expectedUrl)
67+
68+
await Flow.query().insert({
69+
id: VALID_PARAMS.flowId,
70+
name: 'Test Flow',
71+
userId: context.currentUser.id,
72+
})
73+
74+
const tooLargeParams = { ...VALID_PARAMS, size: 2 * 1024 * 1024 + 1 }
75+
await expect(
76+
generatePresignedUrl(null, { input: tooLargeParams }, context),
77+
).rejects.toThrow('Size of attachment exceeds 2MB')
78+
})
79+
80+
it.each([
81+
'application/octet-stream',
82+
'application/x-executable',
83+
'application/x-shockwave-flash',
84+
'application/x-msdownload',
85+
])(
86+
'should throw an error if the file type is not supported: %s',
87+
async (fileType) => {
88+
const expectedUrl = 'https://presigned-url.example.com'
89+
mocks.getSignedUrl.mockResolvedValueOnce(expectedUrl)
90+
91+
await Flow.query().insert({
92+
id: VALID_PARAMS.flowId,
93+
name: 'Test Flow',
94+
userId: context.currentUser.id,
95+
})
96+
97+
const unsupportedParams = {
98+
...VALID_PARAMS,
99+
fileType,
100+
}
101+
102+
await expect(
103+
generatePresignedUrl(null, { input: unsupportedParams }, context),
104+
).rejects.toThrow('Unsupported file type')
105+
},
106+
)
107+
})

0 commit comments

Comments
 (0)