diff --git a/packages/nodes-base/nodes/Confluence/actions/attachment/index.ts b/packages/nodes-base/nodes/Confluence/actions/attachment/index.ts index a186039d2b2..d91bc97aadf 100644 --- a/packages/nodes-base/nodes/Confluence/actions/attachment/index.ts +++ b/packages/nodes-base/nodes/Confluence/actions/attachment/index.ts @@ -2,8 +2,9 @@ import type { INodeProperties } from 'n8n-workflow'; import * as deleteAttachment from './delete.operation'; import * as getMany from './getMany.operation'; +import * as upload from './upload.operation'; -export { deleteAttachment as delete, getMany }; +export { deleteAttachment as delete, getMany, upload }; export const description: INodeProperties[] = [ { @@ -29,10 +30,17 @@ export const description: INodeProperties[] = [ description: 'List the attachments on a page, optionally downloading each file', action: 'Get many attachments', }, + { + name: 'Upload', + value: 'upload', + description: 'Upload a file as an attachment on a page', + action: 'Upload an attachment', + }, ], // Not the first option: the default must stay non-destructive default: 'getMany', }, ...deleteAttachment.description, ...getMany.description, + ...upload.description, ]; diff --git a/packages/nodes-base/nodes/Confluence/actions/attachment/upload.operation.ts b/packages/nodes-base/nodes/Confluence/actions/attachment/upload.operation.ts new file mode 100644 index 00000000000..b308ea2f950 --- /dev/null +++ b/packages/nodes-base/nodes/Confluence/actions/attachment/upload.operation.ts @@ -0,0 +1,79 @@ +import FormData from 'form-data'; +import type { IDataObject, IExecuteFunctions, INodeProperties } from 'n8n-workflow'; + +import { confluenceApiRequestUpload } from '../../transport'; +import { optionalSpaceRLC, pageRLC, resolvePageId } from '../common'; +import type { ConfluenceOperation } from '../router'; + +const showOnUpload = { resource: ['attachment'], operation: ['upload'] }; + +export const description: INodeProperties[] = [ + { + ...optionalSpaceRLC, + description: + 'Limits page selection and By Title lookups to one space. Leave empty or pick "All Spaces" to search across all spaces.', + displayOptions: { show: showOnUpload }, + }, + { + ...pageRLC, + description: 'The page to attach the file to', + displayOptions: { show: showOnUpload }, + }, + { + displayName: 'Input Binary Field', + name: 'binaryPropertyName', + type: 'string', + default: 'data', + required: true, + placeholder: 'e.g. data', + description: 'The name of the input binary field containing the file to upload', + hint: 'The file is loaded into memory in full before uploading, so very large files may be slow or use significant memory', + displayOptions: { show: showOnUpload }, + }, + { + displayName: 'Minor Edit', + name: 'minorEdit', + type: 'boolean', + default: false, + description: 'Whether to upload without notifying watchers of the page', + displayOptions: { show: showOnUpload }, + }, + { + displayName: 'Comment', + name: 'comment', + type: 'string', + default: '', + description: 'An optional comment to attach to this version of the file', + displayOptions: { show: showOnUpload }, + }, +]; + +export const execute: ConfluenceOperation = async function ( + this: IExecuteFunctions, + itemIndex: number, +) { + const pageId = await resolvePageId.call(this, itemIndex); + const binaryPropertyName = this.getNodeParameter('binaryPropertyName', itemIndex, 'data'); + const binaryData = this.helpers.assertBinaryData(itemIndex, binaryPropertyName); + const buffer = await this.helpers.getBinaryDataBuffer(itemIndex, binaryPropertyName); + + // Neither a plain cast nor a strict `=== true` compare is safe here: a plain + // cast reads the string "false" as truthy, and a strict compare reads the + // string "true" (also possible from an expression) as false. Check both. + const rawMinorEdit = this.getNodeParameter('minorEdit', itemIndex, false); + const minorEdit = rawMinorEdit === true || rawMinorEdit === 'true'; + const comment = String(this.getNodeParameter('comment', itemIndex, '')).trim(); + + const formData = new FormData(); + formData.append('file', buffer, { + contentType: binaryData.mimeType, + filename: binaryData.fileName ?? 'file', + }); + formData.append('minorEdit', String(minorEdit)); + if (comment !== '') formData.append('comment', comment); + + const endpoint = `/wiki/rest/api/content/${encodeURIComponent(pageId)}/child/attachment`; + const response = await confluenceApiRequestUpload.call(this, endpoint, formData); + const results = Array.isArray(response.results) ? (response.results as IDataObject[]) : []; + return results[0] ?? response; +}; diff --git a/packages/nodes-base/nodes/Confluence/actions/router.ts b/packages/nodes-base/nodes/Confluence/actions/router.ts index a1d69160350..60e281a06b0 100644 --- a/packages/nodes-base/nodes/Confluence/actions/router.ts +++ b/packages/nodes-base/nodes/Confluence/actions/router.ts @@ -40,6 +40,9 @@ export async function router(this: IExecuteFunctions): Promise { expect(operationOptions('attachment')).toEqual([ expect.objectContaining({ value: 'delete' }), expect.objectContaining({ value: 'getMany' }), + expect.objectContaining({ value: 'upload' }), ]); // Delete sorts first alphabetically; the default must stay non-destructive expect(operationProperty('attachment')?.default).toBe('getMany'); diff --git a/packages/nodes-base/nodes/Confluence/test/actions/attachment/upload.operation.test.ts b/packages/nodes-base/nodes/Confluence/test/actions/attachment/upload.operation.test.ts new file mode 100644 index 00000000000..4242c55a5a7 --- /dev/null +++ b/packages/nodes-base/nodes/Confluence/test/actions/attachment/upload.operation.test.ts @@ -0,0 +1,158 @@ +import type { IBinaryData } from 'n8n-workflow'; +import { NodeOperationError } from 'n8n-workflow'; + +import { description, execute } from '../../../actions/attachment/upload.operation'; +import { confluenceApiRequestUpload } from '../../../transport'; +import { mockExecuteCtx, testNode } from '../../shared'; + +vi.mock('../../../transport', () => ({ + CONFLUENCE_CREDENTIAL_NAME: 'confluenceCloudOAuth2Api', + confluenceApiRequestUpload: vi.fn(), +})); + +const apiRequestUpload = vi.mocked(confluenceApiRequestUpload); + +const ENDPOINT = '/wiki/rest/api/content/9/child/attachment'; + +const baseParams: Record = { + resource: 'attachment', + operation: 'upload', + page: { mode: 'id', value: '9' }, + binaryPropertyName: 'data', + minorEdit: false, + comment: '', +}; + +async function runUpload(overrides: Record = {}, binary?: Partial) { + const ctx = mockExecuteCtx({ ...baseParams, ...overrides }); + ctx.helpers.assertBinaryData.mockReturnValue({ + data: '', + mimeType: 'text/plain', + fileName: 'notes.txt', + ...binary, + }); + ctx.helpers.getBinaryDataBuffer.mockResolvedValue(Buffer.from('file-bytes')); + return await execute.call(ctx, 0); +} + +/** Reads a named multipart field's value out of a `form-data` body, or undefined if absent. */ +function readField(body: string, name: string): string | undefined { + const match = new RegExp(`name="${name}"\\r?\\n\\r?\\n([^\\r\\n]*)`).exec(body); + return match?.[1]; +} + +describe('attachment:upload', () => { + it('warns that large files are buffered fully in memory', () => { + const binaryProperty = description.find((p) => p.name === 'binaryPropertyName'); + expect(binaryProperty?.hint).toMatch(/memory/i); + }); + + beforeEach(() => { + vi.clearAllMocks(); + apiRequestUpload.mockResolvedValue({ results: [{ id: 'att1', title: 'notes.txt' }] }); + }); + + it('uploads the binary file and returns the first result', async () => { + const result = await runUpload(); + + expect(apiRequestUpload).toHaveBeenCalledTimes(1); + const [endpoint, formData] = apiRequestUpload.mock.calls[0]; + expect(endpoint).toBe(ENDPOINT); + expect(formData.getHeaders()['content-type']).toMatch(/^multipart\/form-data/); + expect(result).toEqual({ id: 'att1', title: 'notes.txt' }); + }); + + it('reads the binary data from the configured property, not always "data"', async () => { + const ctx = mockExecuteCtx({ ...baseParams, binaryPropertyName: 'file' }); + ctx.helpers.assertBinaryData.mockReturnValue({ + data: '', + mimeType: 'text/plain', + fileName: 'notes.txt', + }); + ctx.helpers.getBinaryDataBuffer.mockResolvedValue(Buffer.from('file-bytes')); + + await execute.call(ctx, 0); + + expect(ctx.helpers.assertBinaryData).toHaveBeenCalledWith(0, 'file'); + expect(ctx.helpers.getBinaryDataBuffer).toHaveBeenCalledWith(0, 'file'); + }); + + it.each([ + [true, 'true'], + [false, 'false'], + // An expression can hand back the string form of either value + ['true', 'true'], + ['false', 'false'], + ])( + 'sends minorEdit=%j as the string %j under its own form field', + async (minorEdit, expected) => { + await runUpload({ minorEdit }); + + const [, formData] = apiRequestUpload.mock.calls[0]; + expect(readField(formData.getBuffer().toString(), 'minorEdit')).toBe(expected); + }, + ); + + it('omits the comment field when blank', async () => { + await runUpload({ comment: ' ' }); + + const [, formData] = apiRequestUpload.mock.calls[0]; + expect(readField(formData.getBuffer().toString(), 'comment')).toBeUndefined(); + }); + + it('includes a trimmed comment field when provided', async () => { + await runUpload({ comment: ' release notes ' }); + + const [, formData] = apiRequestUpload.mock.calls[0]; + expect(readField(formData.getBuffer().toString(), 'comment')).toBe('release notes'); + }); + + it('falls back to the API response when no results array is present', async () => { + apiRequestUpload.mockResolvedValue({ id: 'att1' }); + + const result = await runUpload(); + + expect(result).toEqual({ id: 'att1' }); + }); + + it('falls back to the raw wrapper when results is an empty array', async () => { + apiRequestUpload.mockResolvedValue({ results: [] }); + + const result = await runUpload(); + + expect(result).toEqual({ results: [] }); + }); + + it('propagates the error when the configured binary property is missing', async () => { + const ctx = mockExecuteCtx({ ...baseParams, binaryPropertyName: 'missing' }); + const notFound = new NodeOperationError( + testNode, + "No binary data property 'missing' exists on item", + ); + ctx.helpers.assertBinaryData.mockImplementation(() => { + throw notFound; + }); + + await expect(execute.call(ctx, 0)).rejects.toBe(notFound); + expect(apiRequestUpload).not.toHaveBeenCalled(); + }); + + it("sends the binary data's mimeType as the file part's Content-Type", async () => { + await runUpload({}, { mimeType: 'application/pdf', fileName: 'report.pdf' }); + + const [, formData] = apiRequestUpload.mock.calls[0]; + expect(formData.getBuffer().toString()).toContain('Content-Type: application/pdf'); + }); + + it('falls back to a generic Content-Type when the binary data has no mimeType or usable filename', async () => { + const result = await runUpload( + {}, + { mimeType: undefined as unknown as string, fileName: undefined as unknown as string }, + ); + + expect(apiRequestUpload).toHaveBeenCalledTimes(1); + const [, formData] = apiRequestUpload.mock.calls[0]; + expect(formData.getBuffer().toString()).toContain('Content-Type: application/octet-stream'); + expect(result).toEqual({ id: 'att1', title: 'notes.txt' }); + }); +}); diff --git a/packages/nodes-base/nodes/Confluence/test/actions/router.test.ts b/packages/nodes-base/nodes/Confluence/test/actions/router.test.ts index 078ea013b1f..b4d7c59eb6d 100644 --- a/packages/nodes-base/nodes/Confluence/test/actions/router.test.ts +++ b/packages/nodes-base/nodes/Confluence/test/actions/router.test.ts @@ -2,15 +2,17 @@ import { NodeOperationError } from 'n8n-workflow'; import type { Mock } from 'vitest'; import { router } from '../../actions/router'; -import { confluenceApiRequest } from '../../transport'; +import { confluenceApiRequest, confluenceApiRequestUpload } from '../../transport'; import { mockExecuteCtx } from '../shared'; vi.mock('../../transport', async (importOriginal) => ({ ...(await importOriginal()), confluenceApiRequest: vi.fn(), + confluenceApiRequestUpload: vi.fn(), })); const apiRequest = confluenceApiRequest as unknown as Mock; +const apiRequestUpload = confluenceApiRequestUpload as unknown as Mock; const createParams: Record = { resource: 'page', @@ -97,6 +99,34 @@ describe('Confluence router', () => { expect(result).toEqual([[{ json: { id: 'a1', title: 'notes.txt' }, pairedItem: { item: 0 } }]]); }); + it('dispatches attachment:upload and returns the created attachment', async () => { + apiRequestUpload.mockResolvedValue({ results: [{ id: 'att1', title: 'notes.txt' }] }); + const ctx = mockExecuteCtx({ + resource: 'attachment', + operation: 'upload', + page: { mode: 'id', value: '9' }, + binaryPropertyName: 'data', + minorEdit: false, + comment: '', + }); + ctx.helpers.assertBinaryData.mockReturnValue({ + data: '', + mimeType: 'text/plain', + fileName: 'notes.txt', + }); + ctx.helpers.getBinaryDataBuffer.mockResolvedValue(Buffer.from('file-bytes')); + + const result = await router.call(ctx); + + expect(apiRequestUpload).toHaveBeenCalledWith( + '/wiki/rest/api/content/9/child/attachment', + expect.anything(), + ); + expect(result).toEqual([ + [{ json: { id: 'att1', title: 'notes.txt' }, pairedItem: { item: 0 } }], + ]); + }); + it('dispatches page:delete and returns the deletion report', async () => { const result = await router.call( mockExecuteCtx({ diff --git a/packages/nodes-base/nodes/Confluence/test/transport/index.test.ts b/packages/nodes-base/nodes/Confluence/test/transport/index.test.ts index 1ae435d1353..59484538302 100644 --- a/packages/nodes-base/nodes/Confluence/test/transport/index.test.ts +++ b/packages/nodes-base/nodes/Confluence/test/transport/index.test.ts @@ -1,3 +1,4 @@ +import FormData from 'form-data'; import type { IExecuteFunctions, INode, JsonObject } from 'n8n-workflow'; import { NodeApiError, NodeOperationError } from 'n8n-workflow'; import type { Mock, Mocked } from 'vitest'; @@ -5,7 +6,11 @@ import { mockDeep } from 'vitest-mock-extended'; import { clearAtlassianCloudIdCache } from '@utils/atlassian'; -import { confluenceApiRequest, confluenceApiRequestBinary } from '../../transport'; +import { + confluenceApiRequest, + confluenceApiRequestBinary, + confluenceApiRequestUpload, +} from '../../transport'; const accessibleResources = [ { id: 'cloud-1', url: 'https://example.atlassian.net', name: 'example' }, @@ -347,3 +352,100 @@ describe('confluenceApiRequestBinary', () => { expect(error?.httpCode).toBe('404'); }); }); + +describe('confluenceApiRequestUpload', () => { + let ctx: Mocked; + let mockHttpRequestWithAuthentication: Mock; + + beforeEach(() => { + vi.clearAllMocks(); + clearAtlassianCloudIdCache(); + ctx = mockDeep(); + mockHttpRequestWithAuthentication = vi.fn().mockResolvedValue(accessibleResources); + ctx.helpers.httpRequestWithAuthentication = mockHttpRequestWithAuthentication; + ctx.getNode.mockReturnValue({ + id: 'test-node', + name: 'Test Confluence Node', + type: 'n8n-nodes-base.confluence', + typeVersion: 1, + position: [0, 0], + parameters: {}, + }); + ctx.getCredentials.mockResolvedValue({ domain: 'https://example.atlassian.net/wiki' }); + }); + + it('PUTs the multipart body with the XSRF-bypass header, no json flag', async () => { + const formData = new FormData(); + formData.append('file', Buffer.from('bytes'), { filename: 'a.txt' }); + mockHttpRequestWithAuthentication + .mockResolvedValueOnce(accessibleResources) + .mockResolvedValueOnce({ results: [{ id: 'att1' }] }); + + const data = await confluenceApiRequestUpload.call( + ctx, + '/wiki/rest/api/content/9/child/attachment', + formData, + ); + + expect(mockHttpRequestWithAuthentication).toHaveBeenNthCalledWith( + 2, + 'confluenceCloudOAuth2Api', + expect.objectContaining({ + // PUT, not POST: the same endpoint's POST is create-only and 400s on + // a filename that already exists on the page, PUT upserts + method: 'PUT', + url: 'https://api.atlassian.com/ex/confluence/cloud-1/wiki/rest/api/content/9/child/attachment', + body: formData, + headers: { 'X-Atlassian-Token': 'nocheck' }, + }), + ); + expect(mockHttpRequestWithAuthentication.mock.calls[1][1]).not.toHaveProperty('json'); + expect(data).toEqual({ results: [{ id: 'att1' }] }); + }); + + it('wraps request failures in NodeApiError, keeping the status', async () => { + mockHttpRequestWithAuthentication + .mockResolvedValueOnce(accessibleResources) + .mockRejectedValueOnce({ message: 'boom', response: { status: 403 } }); + + const error = await confluenceApiRequestUpload + .call(ctx, '/wiki/rest/api/content/9/child/attachment', new FormData()) + .then(() => null) + .catch((thrown: NodeApiError) => thrown); + + expect(error).toBeInstanceOf(NodeApiError); + expect(error?.httpCode).toBe('403'); + }); + + it('surfaces the v1 scope-trap message instead of the generic status text', async () => { + mockHttpRequestWithAuthentication + .mockResolvedValueOnce(accessibleResources) + .mockRejectedValueOnce({ + message: 'Request failed with status code 401', + response: { status: 401, data: { message: 'scope does not match' } }, + }); + + const error = await confluenceApiRequestUpload + .call(ctx, '/wiki/rest/api/content/9/child/attachment', new FormData()) + .then(() => null) + .catch((thrown: NodeApiError) => thrown); + + expect(error).toBeInstanceOf(NodeApiError); + expect(error?.message).toBe('scope does not match'); + expect(error?.httpCode).toBe('401'); + }); + + it('throws a NodeOperationError naming the Site URL field when the credential lacks it', async () => { + ctx.getCredentials.mockResolvedValue({}); + + const promise = confluenceApiRequestUpload.call( + ctx, + '/wiki/rest/api/content/9/child/attachment', + new FormData(), + ); + + await expect(promise).rejects.toThrow(NodeOperationError); + await expect(promise).rejects.toThrow('Site URL'); + expect(mockHttpRequestWithAuthentication).not.toHaveBeenCalled(); + }); +}); diff --git a/packages/nodes-base/nodes/Confluence/transport/index.ts b/packages/nodes-base/nodes/Confluence/transport/index.ts index b7e325f8328..495535127a9 100644 --- a/packages/nodes-base/nodes/Confluence/transport/index.ts +++ b/packages/nodes-base/nodes/Confluence/transport/index.ts @@ -1,3 +1,4 @@ +import type FormData from 'form-data'; import type { IDataObject, IExecuteFunctions, @@ -174,3 +175,51 @@ export async function confluenceApiRequestBinary( if (typeof data === 'string') return Buffer.from(data); throw new NodeOperationError(this.getNode(), 'Confluence returned an unexpected binary response'); } + +/** + * Uploads a multipart body (e.g. a file) through the gateway. PUT, not POST: + * the same endpoint's POST is create-only and 400s on a filename that already + * exists on the page, while PUT upserts (creates if new, new version if the + * filename matches) so the delete+upload replace-a-file story becomes a single + * call. No `json: true` and no explicit Content-Type: `form-data` sets its own + * multipart boundary, and an explicit header would clobber it. + */ +export async function confluenceApiRequestUpload( + this: IExecuteFunctions, + endpoint: string, + formData: FormData, +): Promise { + const credentials = await this.getCredentials(CONFLUENCE_CREDENTIAL_NAME); + const siteUrl = credentials.domain; + if (typeof siteUrl !== 'string' || siteUrl === '') { + throw new NodeOperationError( + this.getNode(), + 'The Confluence credential is missing the Site URL field', + ); + } + const cloudId = await getAtlassianCloudId.call( + this, + CONFLUENCE_CREDENTIAL_NAME, + siteUrl, + 'confluence', + ); + + const options: IHttpRequestOptions = { + method: 'PUT', + url: `${getAtlassianApiBaseUrl('confluence', cloudId)}${endpoint}`, + body: formData, + // Bypasses XSRF checks on this v1 endpoint; without it the gateway answers + // 403 "XSRF check failed" before the request ever reaches Confluence. + headers: { 'X-Atlassian-Token': 'nocheck' }, + }; + + try { + return await this.helpers.httpRequestWithAuthentication.call( + this, + CONFLUENCE_CREDENTIAL_NAME, + options, + ); + } catch (error) { + throw toConfluenceApiError.call(this, error); + } +}