Skip to content

Commit c0f307c

Browse files
authored
Prevents documents being copied with external attachments (#1433)
## Context When external attachments are uploaded, they are placed in a folder in the external storage called a 'document pool', which is specific to the document it was uploaded. When a document is copied, the new copy has a different document pool. This means any external attachments in the document will be broken, and show as unavailable. ## Commit description This blocks the copying of documents with external attachments, until copy behaviour can be implemented in the future. ## Related issues #1021
1 parent 2f0d10d commit c0f307c

File tree

3 files changed

+71
-25
lines changed

3 files changed

+71
-25
lines changed

app/server/lib/DocApi.ts

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -530,11 +530,11 @@ export class DocWorkerApi {
530530
}));
531531

532532
// Returns the status of any current / pending attachment transfers
533-
this._app.get('/api/docs/:docId/attachments/transferStatus', isOwner, withDoc(async (activeDoc, req, res) => {
533+
this._app.get('/api/docs/:docId/attachments/transferStatus', canView, withDoc(async (activeDoc, req, res) => {
534534
res.json(await activeDoc.attachmentTransferStatus());
535535
}));
536536

537-
this._app.get('/api/docs/:docId/attachments/store', isOwner,
537+
this._app.get('/api/docs/:docId/attachments/store', canView,
538538
withDoc(async (activeDoc, req, res) => {
539539
const storeId = await activeDoc.getAttachmentStore();
540540
res.json({

app/server/lib/uploads.ts

Lines changed: 25 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,7 @@
11
import {ApiError} from 'app/common/ApiError';
22
import {InactivityTimer} from 'app/common/InactivityTimer';
33
import {FetchUrlOptions, FileUploadResult, UPLOAD_URL_PATH, UploadResult} from 'app/common/uploads';
4-
import {getUrlFromPrefix} from 'app/common/UserAPI';
4+
import {DocAttachmentsLocation, getUrlFromPrefix} from 'app/common/UserAPI';
55
import {getAuthorizedUserId, getTransitiveHeaders, getUserId, isSingleUserMode,
66
RequestWithLogin} from 'app/server/lib/Authorizer';
77
import {expressWrap} from 'app/server/lib/expressWrap';
@@ -426,9 +426,31 @@ export async function fetchDoc(
426426
// The backend needs to be well configured for this to work.
427427
const { selfPrefix, docWorker } = await getDocWorkerInfoOrSelfPrefix(docId, docWorkerMap, server.getTag());
428428
const docWorkerUrl = docWorker ? docWorker.internalUrl : getUrlFromPrefix(server.getHomeInternalUrl(), selfPrefix);
429+
const apiBaseUrl = docWorkerUrl.replace(/\/*$/, '/');
430+
431+
// Documents with external attachments can't be copied right now. Check status and alert the user.
432+
// Copying as a template is fine, as no attachments will be copied.
433+
if (!template) {
434+
const transferStatusResponse = await fetch(
435+
new URL(`/api/docs/${docId}/attachments/transferStatus`, apiBaseUrl).href,
436+
{
437+
headers: {
438+
...headers,
439+
'Content-Type': 'application/json',
440+
}
441+
}
442+
);
443+
if (!transferStatusResponse.ok) {
444+
throw new ApiError(await transferStatusResponse.text(), transferStatusResponse.status);
445+
}
446+
const attachmentsLocation: DocAttachmentsLocation = (await transferStatusResponse.json()).locationSummary;
447+
if (attachmentsLocation !== 'internal' && attachmentsLocation !== 'none') {
448+
throw new ApiError("Cannot copy a document with external attachments", 400);
449+
}
450+
}
451+
429452
// Download the document, in full or as a template.
430-
const url = new URL(`api/docs/${docId}/download?template=${Number(template)}`,
431-
docWorkerUrl.replace(/\/*$/, '/'));
453+
const url = new URL(`api/docs/${docId}/download?template=${Number(template)}`, apiBaseUrl);
432454
return _fetchURL(url.href, accessId, {headers});
433455
}
434456

test/server/lib/DocApi.ts

Lines changed: 44 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -2433,23 +2433,32 @@ function testDocApi(settings: {
24332433

24342434
});
24352435

2436+
async function addAttachmentsToDoc(docId: string, attachments: {name: string, contents: string}[],
2437+
user: AxiosRequestConfig = chimpy) {
2438+
const formData = new FormData();
2439+
for (const attachment of attachments) {
2440+
formData.append('upload', attachment.contents, attachment.name);
2441+
}
2442+
const resp = await axios.post(`${serverUrl}/api/docs/${docId}/attachments`, formData,
2443+
defaultsDeep({headers: formData.getHeaders()}, user));
2444+
assert.equal(resp.status, 200);
2445+
assert.equal(resp.data.length, attachments.length);
2446+
return resp;
2447+
}
2448+
24362449
describe('attachments', function () {
24372450
it("POST /docs/{did}/attachments adds attachments", async function () {
2438-
let formData = new FormData();
2439-
formData.append('upload', 'foobar', "hello.doc");
2440-
formData.append('upload', '123456', "world.jpg");
2441-
let resp = await axios.post(`${serverUrl}/api/docs/${docIds.TestDoc}/attachments`, formData,
2442-
defaultsDeep({headers: formData.getHeaders()}, chimpy));
2443-
assert.equal(resp.status, 200);
2444-
assert.deepEqual(resp.data, [1, 2]);
2451+
const uploadResp = await addAttachmentsToDoc(docIds.TestDoc, [
2452+
{ name: 'hello.doc', contents: 'foobar' },
2453+
{ name: 'world.jpg', contents: '123456' },
2454+
], chimpy);
2455+
assert.deepEqual(uploadResp.data, [1, 2]);
24452456

24462457
// Another upload gets the next number.
2447-
formData = new FormData();
2448-
formData.append('upload', 'abcdef', "hello.png");
2449-
resp = await axios.post(`${serverUrl}/api/docs/${docIds.TestDoc}/attachments`, formData,
2450-
defaultsDeep({headers: formData.getHeaders()}, chimpy));
2451-
assert.equal(resp.status, 200);
2452-
assert.deepEqual(resp.data, [3]);
2458+
const upload2Resp = await addAttachmentsToDoc(docIds.TestDoc, [
2459+
{ name: 'hello.png', contents: 'abcdef' },
2460+
], chimpy);
2461+
assert.deepEqual(upload2Resp.data, [3]);
24532462
});
24542463

24552464
it("GET /docs/{did}/attachments lists attachment metadata", async function () {
@@ -2753,13 +2762,11 @@ function testDocApi(settings: {
27532762
docId = await userApi.newDoc({name: 'TestDocExternalAttachments'}, wid);
27542763
docUrl = `${serverUrl}/api/docs/${docId}`;
27552764

2756-
const formData = new FormData();
2757-
formData.append('upload', 'foobar', "hello.doc");
2758-
formData.append('upload', '123456', "world.jpg");
2759-
formData.append('upload', 'foobar', "hello2.doc");
2760-
const resp = await axios.post(`${docUrl}/attachments`, formData,
2761-
defaultsDeep({headers: formData.getHeaders()}, chimpy));
2762-
assert.equal(resp.status, 200);
2765+
const resp = await addAttachmentsToDoc(docId, [
2766+
{ name: 'hello.doc', contents: 'foobar' },
2767+
{ name: 'world.jpg', contents: '123456' },
2768+
{ name: 'hello2.doc', contents: 'foobar' }
2769+
], chimpy);
27632770
assert.deepEqual(resp.data, [1, 2, 3]);
27642771
});
27652772

@@ -2804,6 +2811,23 @@ function testDocApi(settings: {
28042811
locationSummary: "internal",
28052812
});
28062813
});
2814+
2815+
it("POST /docs/{did}/copy fails when the document has external attachments", async function () {
2816+
const worker1 = await userApi.getWorkerAPI(docId);
2817+
await assert.isRejected(worker1.copyDoc(docId, undefined, 'copy'), /status 400/);
2818+
});
2819+
2820+
it("POST /docs/{did} with sourceDocId fails to copy a document with external attachments", async function () {
2821+
const chimpyWs = await userApi.newWorkspace({name: "Chimpy's Workspace"}, ORG_NAME);
2822+
const resp = await axios.post(`${serverUrl}/api/docs`, {
2823+
sourceDocumentId: docId,
2824+
documentName: 'copy of TestDocExternalAttachments',
2825+
asTemplate: false,
2826+
workspaceId: chimpyWs
2827+
}, chimpy);
2828+
assert.equal(resp.status, 400);
2829+
assert.match(resp.data.error, /external attachments/);
2830+
});
28072831
});
28082832
});
28092833

0 commit comments

Comments
 (0)