From ea4d11ba68439e47eb709717c246204656f4ba93 Mon Sep 17 00:00:00 2001 From: Andrew Rumble Date: Wed, 22 Jan 2025 10:56:42 +0000 Subject: [PATCH] Merge pull request #23020 from overleaf/ar-prevent-rootFolder-deletion [web] Prevent deletes on a project's rootFolder GitOrigin-RevId: 6d0506f207425f65d3de990a78bb1ea9b136ed1e --- .../src/Features/Errors/ErrorController.js | 6 +++ .../web/app/src/Features/Errors/Errors.js | 7 +++ .../ProjectEntityMongoUpdateHandler.js | 11 +++- services/web/locales/en.json | 1 + .../acceptance/src/ProjectStructureTests.mjs | 51 +++++++++++++++++-- 5 files changed, 70 insertions(+), 6 deletions(-) diff --git a/services/web/app/src/Features/Errors/ErrorController.js b/services/web/app/src/Features/Errors/ErrorController.js index a23b6521c7..b7f96a8b13 100644 --- a/services/web/app/src/Features/Errors/ErrorController.js +++ b/services/web/app/src/Features/Errors/ErrorController.js @@ -77,6 +77,12 @@ async function handleError(error, req, res, next) { res.status(400) plainTextResponse(res, error.message) } + } else if (error instanceof Errors.NonDeletableEntityError) { + req.logger.setLevel('warn') + if (shouldSendErrorResponse) { + res.status(422) + plainTextResponse(res, error.message) + } } else if (error instanceof Errors.SAMLSessionDataMissing) { req.logger.setLevel('warn') if (shouldSendErrorResponse) { diff --git a/services/web/app/src/Features/Errors/Errors.js b/services/web/app/src/Features/Errors/Errors.js index c00a1097d6..40076cea6e 100644 --- a/services/web/app/src/Features/Errors/Errors.js +++ b/services/web/app/src/Features/Errors/Errors.js @@ -268,6 +268,12 @@ class InvalidInstitutionalEmailError extends OError { } } +class NonDeletableEntityError extends OError { + get i18nKey() { + return 'non_deletable_entity' + } +} + module.exports = { OError, BackwardCompatibleError, @@ -318,4 +324,5 @@ module.exports = { AffiliationError, InvalidEmailError, InvalidInstitutionalEmailError, + NonDeletableEntityError, } diff --git a/services/web/app/src/Features/Project/ProjectEntityMongoUpdateHandler.js b/services/web/app/src/Features/Project/ProjectEntityMongoUpdateHandler.js index dcf1bd0471..31b3efaec8 100644 --- a/services/web/app/src/Features/Project/ProjectEntityMongoUpdateHandler.js +++ b/services/web/app/src/Features/Project/ProjectEntityMongoUpdateHandler.js @@ -375,11 +375,20 @@ async function moveEntity(projectId, entityId, destFolderId, entityType) { return { project, startPath, endPath, rev: entity.rev, changes } } -async function deleteEntity(projectId, entityId, entityType, callback) { +async function deleteEntity(projectId, entityId, entityType) { const project = await ProjectGetter.promises.getProjectWithoutLock( projectId, { name: true, rootFolder: true, overleaf: true, rootDoc_id: true } ) + if ( + entityType === 'folder' && + project.rootFolder.some( + rootFolder => rootFolder._id.toString() === entityId.toString() + ) + ) { + throw new Errors.NonDeletableEntityError('cannot delete root folder') + } + const deleteRootDoc = project.rootDoc_id && entityId && diff --git a/services/web/locales/en.json b/services/web/locales/en.json index 2931f6669c..597424e8b1 100644 --- a/services/web/locales/en.json +++ b/services/web/locales/en.json @@ -1352,6 +1352,7 @@ "no_symbols_found": "No symbols found", "no_thanks_cancel_now": "No thanks, I still want to cancel", "no_update_email": "No, update email", + "non_deletable_entity": "The specified entity may not be deleted", "normal": "Normal", "normally_x_price_per_month": "Normally __price__ per month", "normally_x_price_per_year": "Normally __price__ per year", diff --git a/services/web/test/acceptance/src/ProjectStructureTests.mjs b/services/web/test/acceptance/src/ProjectStructureTests.mjs index 022c16c294..a1f48a8448 100644 --- a/services/web/test/acceptance/src/ProjectStructureTests.mjs +++ b/services/web/test/acceptance/src/ProjectStructureTests.mjs @@ -1,4 +1,4 @@ -import { expect } from 'chai' +import chai, { expect } from 'chai' import mongodb from 'mongodb-legacy' import Path from 'node:path' import fs from 'node:fs' @@ -7,14 +7,14 @@ import ProjectGetter from '../../../app/src/Features/Project/ProjectGetter.js' import UserHelper from './helpers/User.mjs' import MockDocStoreApiClass from './mocks/MockDocstoreApi.mjs' import MockDocUpdaterApiClass from './mocks/MockDocUpdaterApi.mjs' -import { fileURLToPath } from 'node:url' +import chaiAsPromised from 'chai-as-promised' + +chai.use(chaiAsPromised) const User = UserHelper.promises const ObjectId = mongodb.ObjectId -const __dirname = fileURLToPath(new URL('.', import.meta.url)) - let MockDocStoreApi, MockDocUpdaterApi before(function () { @@ -76,7 +76,7 @@ describe('ProjectStructureChanges', function () { async function uploadExampleProject(owner, zipFilename, options = {}) { const zipFile = fs.createReadStream( - Path.resolve(Path.join(__dirname, '..', 'files', zipFilename)) + Path.resolve(Path.join(import.meta.dirname, '..', 'files', zipFilename)) ) const { response, body } = await owner.doRequest('POST', { @@ -209,6 +209,47 @@ describe('ProjectStructureChanges', function () { }) }) + describe('deleting folders', function () { + beforeEach(async function () { + const { projectId } = await createExampleProject(owner) + this.exampleProjectId = projectId + }) + describe('when the folder is the rootFolder', function () { + beforeEach(async function () { + const project = await ProjectGetter.promises.getProject( + this.exampleProjectId + ) + this.rootFolderId = project.rootFolder[0]._id + }) + + it('should fail with a 422 error', async function () { + await expect( + deleteItem(owner, this.exampleProjectId, 'folder', this.rootFolderId) + ) + .to.be.rejected.and.eventually.match(/status=422/) + .and.eventually.match(/body="cannot delete root folder"/) + }) + }) + + describe('when the folder is not the rootFolder', function () { + beforeEach(async function () { + const folderId = await createExampleFolder(owner, this.exampleProjectId) + this.exampleFolderId = folderId + }) + + it('should succeed', async function () { + await expect( + deleteItem( + owner, + this.exampleProjectId, + 'folder', + this.exampleFolderId + ) + ).to.be.fulfilled + }) + }) + }) + describe('deleting docs', function () { beforeEach(async function () { const { projectId } = await createExampleProject(owner)