diff --git a/app/server/lib/ActiveDoc.ts b/app/server/lib/ActiveDoc.ts index d56a6325..390aeb1b 100644 --- a/app/server/lib/ActiveDoc.ts +++ b/app/server/lib/ActiveDoc.ts @@ -680,10 +680,13 @@ export class ActiveDoc extends EventEmitter { * shut it down, and unlist it via the DocManager. A fresh ActiveDoc can be acquired via the * DocManager. */ - public async replace(source: DocReplacementOptions) { + public async replace(docSession: OptDocSession, source: DocReplacementOptions) { // During replacement, it is important for all hands to be off the document. So we // ask the shutdown method to do the replacement when the ActiveDoc is shutdown but // before a new one could be opened. + if (!await this._granularAccess.isOwner(docSession)) { + throw new ApiError('Only owners can replace a document.', 403); + } return this.shutdown({ afterShutdown: () => this._docManager.storageManager.replace(this.docName, source) }); diff --git a/app/server/lib/DocApi.ts b/app/server/lib/DocApi.ts index eff8cc4e..966838a0 100644 --- a/app/server/lib/DocApi.ts +++ b/app/server/lib/DocApi.ts @@ -393,10 +393,15 @@ export class DocWorkerApi { // TODO: look at download behavior if ActiveDoc is shutdown during call (cannot // use withDoc wrapper) this._app.get('/api/docs/:docId/download', canView, throttled(async (req, res) => { + // Support a dryRun flag to check if user has the right to download the + // full document. + const dryRun = isAffirmative(req.query.dryrun || req.query.dryRun); + const dryRunSuccess = () => res.status(200).json({dryRun: 'allowed'}); // We want to be have a way download broken docs that ActiveDoc may not be able // to load. So, if the user owns the document, we unconditionally let them // download. if (await this._isOwner(req)) { + if (dryRun) { dryRunSuccess(); return; } try { // We carefully avoid creating an ActiveDoc for the document being downloaded, // in case it is broken in some way. It is convenient to be able to download @@ -419,6 +424,7 @@ export class DocWorkerApi { if (!await activeDoc.canDownload(docSessionFromRequest(req))) { throw new ApiError('not authorized to download this document', 403); } + if (dryRun) { dryRunSuccess(); return; } return this._docWorker.downloadDoc(req, res, this._docManager.storageManager); } })); @@ -733,6 +739,18 @@ export class DocWorkerApi { const options: DocReplacementOptions = {}; if (req.body.sourceDocId) { options.sourceDocId = await this._confirmDocIdForRead(req, String(req.body.sourceDocId)); + // Make sure that if we wanted to download the full source, we would be allowed. + const result = await fetch(this._grist.getHomeUrl(req, `/api/docs/${options.sourceDocId}/download?dryrun=1`), { + method: 'GET', + headers: { + ...getTransitiveHeaders(req), + 'Content-Type': 'application/json', + } + }); + if (result.status !== 200) { + const jsonResult = await result.json(); + throw new ApiError(jsonResult.error, result.status); + } // We should make sure the source document has flushed recently. // It may not be served by the same worker, so work through the api. await fetch(this._grist.getHomeUrl(req, `/api/docs/${options.sourceDocId}/flush`), { @@ -746,7 +764,8 @@ export class DocWorkerApi { if (req.body.snapshotId) { options.snapshotId = String(req.body.snapshotId); } - await activeDoc.replace(options); + const docSession = docSessionFromRequest(req); + await activeDoc.replace(docSession, options); res.json(null); })); diff --git a/test/server/lib/DocApi.ts b/test/server/lib/DocApi.ts index c17ee7ff..e4e9a753 100644 --- a/test/server/lib/DocApi.ts +++ b/test/server/lib/DocApi.ts @@ -2285,9 +2285,11 @@ function testDocApi() { const ws1 = (await userApi.getOrgWorkspaces('current'))[0].id; const doc1 = await userApi.newDoc({name: 'testdoc1'}, ws1); const doc2 = await userApi.newDoc({name: 'testdoc2'}, ws1); - const doc3 = await userApi.newDoc({name: 'testdoc2'}, ws1); + const doc3 = await userApi.newDoc({name: 'testdoc3'}, ws1); + const doc4 = await userApi.newDoc({name: 'testdoc4'}, ws1); await userApi.updateDocPermissions(doc2, {users: {'kiwi@getgrist.com': 'editors'}}); await userApi.updateDocPermissions(doc3, {users: {'kiwi@getgrist.com': 'viewers'}}); + await userApi.updateDocPermissions(doc4, {users: {'kiwi@getgrist.com': 'owners'}}); try { // Put some material in doc3 let resp = await axios.post(`${serverUrl}/o/docs/api/docs/${doc3}/tables/Table1/data`, { @@ -2295,28 +2297,52 @@ function testDocApi() { }, chimpy); assert.equal(resp.status, 200); - // Kiwi can replace doc2 with doc3 + // Kiwi cannot replace doc2 with doc3, not an owner resp = await axios.post(`${serverUrl}/o/docs/api/docs/${doc2}/replace`, { sourceDocId: doc3 }, kiwi); - assert.equal(resp.status, 200); - resp = await axios.get(`${serverUrl}/api/docs/${doc2}/tables/Table1/data`, chimpy); - assert.equal(resp.data.A[0], 'Orange'); + assert.equal(resp.status, 403); + assert.match(resp.data.error, /Only owners can replace a document/); - // Kiwi can't replace doc1 with doc3, no write access to doc1 + // Kiwi can't replace doc1 with doc3, no access to doc1 resp = await axios.post(`${serverUrl}/o/docs/api/docs/${doc1}/replace`, { sourceDocId: doc3 }, kiwi); assert.equal(resp.status, 403); + assert.match(resp.data.error, /No view access/); // Kiwi can't replace doc2 with doc1, no read access to doc1 resp = await axios.post(`${serverUrl}/o/docs/api/docs/${doc2}/replace`, { sourceDocId: doc1 }, kiwi); assert.equal(resp.status, 403); + assert.match(resp.data.error, /access denied/); + + // Kiwi cannot replace a doc with material they have only partial read access to. + resp = await axios.post(`${serverUrl}/api/docs/${doc3}/apply`, [ + ['AddRecord', '_grist_ACLResources', -1, {tableId: 'Table1', colIds: 'A'}], + ['AddRecord', '_grist_ACLRules', null, { + resource: -1, aclFormula: 'user.Access not in [OWNER]', permissionsText: '-R', + }] + ], chimpy); + assert.equal(resp.status, 200); + resp = await axios.post(`${serverUrl}/o/docs/api/docs/${doc4}/replace`, { + sourceDocId: doc3 + }, kiwi); + assert.equal(resp.status, 403); + assert.match(resp.data.error, /not authorized/); + resp = await axios.post(`${serverUrl}/api/docs/${doc3}/tables/_grist_ACLRules/data/delete`, + [2], chimpy); + assert.equal(resp.status, 200); + resp = await axios.post(`${serverUrl}/o/docs/api/docs/${doc4}/replace`, { + sourceDocId: doc3 + }, kiwi); + assert.equal(resp.status, 200); } finally { await userApi.deleteDoc(doc1); await userApi.deleteDoc(doc2); + await userApi.deleteDoc(doc3); + await userApi.deleteDoc(doc4); } });