From fccdb2524057d6aff2a186e1df5f3e0bbeb3f115 Mon Sep 17 00:00:00 2001 From: Spoffy Date: Fri, 7 Feb 2025 22:27:55 +0000 Subject: [PATCH 1/5] Prevents documents being copied with external attachments --- app/server/lib/uploads.ts | 25 +++++++++++++-- test/server/lib/DocApi.ts | 65 ++++++++++++++++++++++++++------------- 2 files changed, 66 insertions(+), 24 deletions(-) diff --git a/app/server/lib/uploads.ts b/app/server/lib/uploads.ts index 08c3585207..a8720632f9 100644 --- a/app/server/lib/uploads.ts +++ b/app/server/lib/uploads.ts @@ -1,7 +1,7 @@ import {ApiError} from 'app/common/ApiError'; import {InactivityTimer} from 'app/common/InactivityTimer'; import {FetchUrlOptions, FileUploadResult, UPLOAD_URL_PATH, UploadResult} from 'app/common/uploads'; -import {getUrlFromPrefix} from 'app/common/UserAPI'; +import {DocAttachmentsLocation, getUrlFromPrefix} from 'app/common/UserAPI'; import {getAuthorizedUserId, getTransitiveHeaders, getUserId, isSingleUserMode, RequestWithLogin} from 'app/server/lib/Authorizer'; import {expressWrap} from 'app/server/lib/expressWrap'; @@ -426,9 +426,28 @@ export async function fetchDoc( // The backend needs to be well configured for this to work. const { selfPrefix, docWorker } = await getDocWorkerInfoOrSelfPrefix(docId, docWorkerMap, server.getTag()); const docWorkerUrl = docWorker ? docWorker.internalUrl : getUrlFromPrefix(server.getHomeInternalUrl(), selfPrefix); + const apiBaseUrl = docWorkerUrl.replace(/\/*$/, '/'); + + // Documents with external attachments can't be copied right now. Check status and alert the user. + const transferStatusResponse = await fetch( + new URL(`/api/docs/${docId}/attachments/transferStatus`, apiBaseUrl).href, + { + headers: { + ...headers, + 'Content-Type': 'application/json', + } + } + ); + if (!transferStatusResponse.ok) { + throw new ApiError(await transferStatusResponse.text(), transferStatusResponse.status); + } + const attachmentsLocation: DocAttachmentsLocation = (await transferStatusResponse.json()).locationSummary; + if (attachmentsLocation !== 'internal' && attachmentsLocation !== 'none') { + throw new ApiError("Cannot copy a document with external attachments", 400); + } + // Download the document, in full or as a template. - const url = new URL(`api/docs/${docId}/download?template=${Number(template)}`, - docWorkerUrl.replace(/\/*$/, '/')); + const url = new URL(`api/docs/${docId}/download?template=${Number(template)}`, apiBaseUrl); return _fetchURL(url.href, accessId, {headers}); } diff --git a/test/server/lib/DocApi.ts b/test/server/lib/DocApi.ts index e72b1abf13..15d494f0cf 100644 --- a/test/server/lib/DocApi.ts +++ b/test/server/lib/DocApi.ts @@ -2433,23 +2433,32 @@ function testDocApi(settings: { }); + async function addAttachmentsToDoc(docId: string, attachments: {name: string, contents: string}[], + user: AxiosRequestConfig = chimpy) { + const formData = new FormData(); + for (const attachment of attachments) { + formData.append('upload', attachment.contents, attachment.name); + } + const resp = await axios.post(`${serverUrl}/api/docs/${docId}/attachments`, formData, + defaultsDeep({headers: formData.getHeaders()}, user)); + assert.equal(resp.status, 200); + assert.equal(resp.data.length, attachments.length); + return resp; + } + describe('attachments', function () { it("POST /docs/{did}/attachments adds attachments", async function () { - let formData = new FormData(); - formData.append('upload', 'foobar', "hello.doc"); - formData.append('upload', '123456', "world.jpg"); - let resp = await axios.post(`${serverUrl}/api/docs/${docIds.TestDoc}/attachments`, formData, - defaultsDeep({headers: formData.getHeaders()}, chimpy)); - assert.equal(resp.status, 200); - assert.deepEqual(resp.data, [1, 2]); + const uploadResp = await addAttachmentsToDoc(docIds.TestDoc, [ + { name: 'hello.doc', contents: 'foobar' }, + { name: 'world.jpg', contents: '123456' }, + ], chimpy); + assert.deepEqual(uploadResp.data, [1, 2]); // Another upload gets the next number. - formData = new FormData(); - formData.append('upload', 'abcdef', "hello.png"); - resp = await axios.post(`${serverUrl}/api/docs/${docIds.TestDoc}/attachments`, formData, - defaultsDeep({headers: formData.getHeaders()}, chimpy)); - assert.equal(resp.status, 200); - assert.deepEqual(resp.data, [3]); + const upload2Resp = await addAttachmentsToDoc(docIds.TestDoc, [ + { name: 'hello.png', contents: 'abcdef' }, + ], chimpy); + assert.deepEqual(upload2Resp.data, [3]); }); it("GET /docs/{did}/attachments lists attachment metadata", async function () { @@ -2753,14 +2762,11 @@ function testDocApi(settings: { docId = await userApi.newDoc({name: 'TestDocExternalAttachments'}, wid); docUrl = `${serverUrl}/api/docs/${docId}`; - const formData = new FormData(); - formData.append('upload', 'foobar', "hello.doc"); - formData.append('upload', '123456', "world.jpg"); - formData.append('upload', 'foobar', "hello2.doc"); - const resp = await axios.post(`${docUrl}/attachments`, formData, - defaultsDeep({headers: formData.getHeaders()}, chimpy)); - assert.equal(resp.status, 200); - assert.deepEqual(resp.data, [1, 2, 3]); + await addAttachmentsToDoc(docId, [ + { name: 'hello.doc', contents: 'foobar' }, + { name: 'world.jpg', contents: '123456' }, + { name: 'hello2.doc', contents: 'foobar' } + ], chimpy); }); after(async () => { @@ -2804,6 +2810,23 @@ function testDocApi(settings: { locationSummary: "internal", }); }); + + it("POST /docs/{did}/copy fails when the document has external attachments", async function () { + const worker1 = await userApi.getWorkerAPI(docId); + await assert.isRejected(worker1.copyDoc(docId, undefined, 'copy'), /status 400/); + }); + + it("POST /docs/{did} with sourceDocId fails to copy a document with external attachments", async function () { + const chimpyWs = await userApi.newWorkspace({name: "Chimpy's Workspace"}, ORG_NAME); + const resp = await axios.post(`${serverUrl}/api/docs`, { + sourceDocumentId: docId, + documentName: 'copy of TestDocExternalAttachments', + asTemplate: false, + workspaceId: chimpyWs + }, chimpy); + assert.equal(resp.status, 400); + assert.match(resp.data.error, /external attachments/); + }); }); }); From e372766c593f967a8ec25c9f4d7e6f26a7729cb6 Mon Sep 17 00:00:00 2001 From: Spoffy Date: Tue, 11 Feb 2025 14:48:55 +0000 Subject: [PATCH 2/5] Enables copying if copying as a template --- app/server/lib/uploads.ts | 27 +++++++++++++++------------ 1 file changed, 15 insertions(+), 12 deletions(-) diff --git a/app/server/lib/uploads.ts b/app/server/lib/uploads.ts index a8720632f9..1f187459dc 100644 --- a/app/server/lib/uploads.ts +++ b/app/server/lib/uploads.ts @@ -429,21 +429,24 @@ export async function fetchDoc( const apiBaseUrl = docWorkerUrl.replace(/\/*$/, '/'); // Documents with external attachments can't be copied right now. Check status and alert the user. - const transferStatusResponse = await fetch( + // Copying as a template is fine, as no attachments will be copied. + if (!template) { + const transferStatusResponse = await fetch( new URL(`/api/docs/${docId}/attachments/transferStatus`, apiBaseUrl).href, - { - headers: { - ...headers, - 'Content-Type': 'application/json', + { + headers: { + ...headers, + 'Content-Type': 'application/json', + } } + ); + if (!transferStatusResponse.ok) { + throw new ApiError(await transferStatusResponse.text(), transferStatusResponse.status); + } + const attachmentsLocation: DocAttachmentsLocation = (await transferStatusResponse.json()).locationSummary; + if (attachmentsLocation !== 'internal' && attachmentsLocation !== 'none') { + throw new ApiError("Cannot copy a document with external attachments", 400); } - ); - if (!transferStatusResponse.ok) { - throw new ApiError(await transferStatusResponse.text(), transferStatusResponse.status); - } - const attachmentsLocation: DocAttachmentsLocation = (await transferStatusResponse.json()).locationSummary; - if (attachmentsLocation !== 'internal' && attachmentsLocation !== 'none') { - throw new ApiError("Cannot copy a document with external attachments", 400); } // Download the document, in full or as a template. From dbcb50bf95a3770cf3bb5ad80d51989f1c451f88 Mon Sep 17 00:00:00 2001 From: Spoffy Date: Tue, 11 Feb 2025 16:58:44 +0000 Subject: [PATCH 3/5] Fixes permissions on the transferStatus endpoint for copying --- app/server/lib/DocApi.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/server/lib/DocApi.ts b/app/server/lib/DocApi.ts index 443a9614ac..266a671da9 100644 --- a/app/server/lib/DocApi.ts +++ b/app/server/lib/DocApi.ts @@ -534,7 +534,7 @@ export class DocWorkerApi { })); // Returns the status of any current / pending attachment transfers - this._app.get('/api/docs/:docId/attachments/transferStatus', isOwner, withDoc(async (activeDoc, req, res) => { + this._app.get('/api/docs/:docId/attachments/transferStatus', canView, withDoc(async (activeDoc, req, res) => { const locationSummary = await activeDoc.attachmentLocationSummary(); res.json({ status: activeDoc.attachmentTransferStatus(), From ffe679bf3b3f4e0428096f538bf54c7205f12821 Mon Sep 17 00:00:00 2001 From: Spoffy Date: Wed, 12 Feb 2025 20:43:54 +0000 Subject: [PATCH 4/5] Changes GET attachment store endpoint to canView --- app/server/lib/DocApi.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/server/lib/DocApi.ts b/app/server/lib/DocApi.ts index 266a671da9..209d88629b 100644 --- a/app/server/lib/DocApi.ts +++ b/app/server/lib/DocApi.ts @@ -542,7 +542,7 @@ export class DocWorkerApi { }); })); - this._app.get('/api/docs/:docId/attachments/store', isOwner, + this._app.get('/api/docs/:docId/attachments/store', canView, withDoc(async (activeDoc, req, res) => { const storeId = await activeDoc.getAttachmentStore(); res.json({ From 43b7607b5cdefaa47dcf0dfd6257831dc62bfb55 Mon Sep 17 00:00:00 2001 From: Spoffy Date: Mon, 17 Feb 2025 16:44:51 +0000 Subject: [PATCH 5/5] Adds additional assert to test --- test/server/lib/DocApi.ts | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/test/server/lib/DocApi.ts b/test/server/lib/DocApi.ts index 15d494f0cf..06622b75d5 100644 --- a/test/server/lib/DocApi.ts +++ b/test/server/lib/DocApi.ts @@ -2762,11 +2762,12 @@ function testDocApi(settings: { docId = await userApi.newDoc({name: 'TestDocExternalAttachments'}, wid); docUrl = `${serverUrl}/api/docs/${docId}`; - await addAttachmentsToDoc(docId, [ + const resp = await addAttachmentsToDoc(docId, [ { name: 'hello.doc', contents: 'foobar' }, { name: 'world.jpg', contents: '123456' }, { name: 'hello2.doc', contents: 'foobar' } ], chimpy); + assert.deepEqual(resp.data, [1, 2, 3]); }); after(async () => {