From f870721fca7acd0e7af003c9753afa34163b531a Mon Sep 17 00:00:00 2001 From: Julian Lam Date: Thu, 8 Oct 2020 12:00:06 -0400 Subject: [PATCH] feat(writeapi): file deletion route --- public/openapi/write.yaml | 33 ++++++++++++++++++ public/src/admin/manage/uploads.js | 12 +++---- src/controllers/write/files.js | 11 ++++++ src/controllers/write/index.js | 1 + src/middleware/assert.js | 56 +++++++++++++++++++++++------- src/routes/write/files.js | 16 +++++++++ src/routes/write/index.js | 1 + src/socket.io/admin/uploads.js | 4 +++ 8 files changed, 115 insertions(+), 19 deletions(-) create mode 100644 src/controllers/write/files.js create mode 100644 src/routes/write/files.js diff --git a/public/openapi/write.yaml b/public/openapi/write.yaml index 4bd7e2fed9e6..2a16be8a7154 100644 --- a/public/openapi/write.yaml +++ b/public/openapi/write.yaml @@ -1048,6 +1048,39 @@ paths: response: type: object properties: {} + /files: + delete: + tags: + - files + summary: delete uploaded file + description: This operation deletes a file uploaded to NodeBB + requestBody: + required: true + content: + application/json: + schema: + type: object + properties: + path: + type: string + description: Path to the file (relative to the configured `upload_path`) + required: + - path + example: + path: files/uploaded_file.jpg + responses: + '200': + description: File deleted + content: + application/json: + schema: + type: object + properties: + status: + $ref: '#/components/schemas/Status' + response: + type: object + properties: {} components: schemas: Status: diff --git a/public/src/admin/manage/uploads.js b/public/src/admin/manage/uploads.js index 98da2ef557e0..997ce9c91aac 100644 --- a/public/src/admin/manage/uploads.js +++ b/public/src/admin/manage/uploads.js @@ -1,7 +1,7 @@ 'use strict'; -define('admin/manage/uploads', ['uploader'], function (uploader) { +define('admin/manage/uploads', ['uploader', 'api'], function (uploader, api) { var Uploads = {}; Uploads.init = function () { @@ -21,12 +21,12 @@ define('admin/manage/uploads', ['uploader'], function (uploader) { if (!ok) { return; } - socket.emit('admin.uploads.delete', file.attr('data-path'), function (err) { - if (err) { - return app.alertError(err.message); - } + + api.del('/files', { + path: file.attr('data-path'), + }, () => { file.remove(); - }); + }, 'default'); }); }); }; diff --git a/src/controllers/write/files.js b/src/controllers/write/files.js new file mode 100644 index 000000000000..564424f5cd6a --- /dev/null +++ b/src/controllers/write/files.js @@ -0,0 +1,11 @@ +'use strict'; + +const fs = require('fs').promises; +const helpers = require('../helpers'); + +const Files = module.exports; + +Files.delete = async (req, res) => { + await fs.unlink(res.locals.cleanedPath); + helpers.formatApiResponse(200, res); +}; diff --git a/src/controllers/write/index.js b/src/controllers/write/index.js index 7bf5776b1c8e..66d65987806b 100644 --- a/src/controllers/write/index.js +++ b/src/controllers/write/index.js @@ -8,3 +8,4 @@ Write.categories = require('./categories'); Write.topics = require('./topics'); Write.posts = require('./posts'); Write.admin = require('./admin'); +Write.files = require('./files'); diff --git a/src/middleware/assert.js b/src/middleware/assert.js index ef7d79938678..abb22cfd9391 100644 --- a/src/middleware/assert.js +++ b/src/middleware/assert.js @@ -5,44 +5,74 @@ * payload and throw an error otherwise. */ +const fs = require('fs'); +const fsPromises = fs.promises; +const path = require('path'); + +const nconf = require('nconf'); + const user = require('../user'); const groups = require('../groups'); const topics = require('../topics'); const posts = require('../posts'); -const helpers = require('../controllers/helpers'); +const helpers = require('./helpers'); +const controllerHelpers = require('../controllers/helpers'); module.exports = function (middleware) { - middleware.assertUser = async (req, res, next) => { + middleware.assertUser = helpers.try(async (req, res, next) => { if (!await user.exists(req.params.uid)) { - return helpers.formatApiResponse(404, res, new Error('[[error:no-user]]')); + return controllerHelpers.formatApiResponse(404, res, new Error('[[error:no-user]]')); } next(); - }; + }); - middleware.assertGroup = async (req, res, next) => { + middleware.assertGroup = helpers.try(async (req, res, next) => { const name = await groups.getGroupNameByGroupSlug(req.params.slug); if (!name || !await groups.exists(name)) { - return helpers.formatApiResponse(404, res, new Error('[[error:no-group]]')); + return controllerHelpers.formatApiResponse(404, res, new Error('[[error:no-group]]')); } next(); - }; + }); - middleware.assertTopic = async (req, res, next) => { + middleware.assertTopic = helpers.try(async (req, res, next) => { if (!await topics.exists(req.params.tid)) { - return helpers.formatApiResponse(404, res, new Error('[[error:no-topic]]')); + return controllerHelpers.formatApiResponse(404, res, new Error('[[error:no-topic]]')); } next(); - }; + }); - middleware.assertPost = async (req, res, next) => { + middleware.assertPost = helpers.try(async (req, res, next) => { if (!await posts.exists(req.params.pid)) { - return helpers.formatApiResponse(404, res, new Error('[[error:no-topic]]')); + return controllerHelpers.formatApiResponse(404, res, new Error('[[error:no-topic]]')); + } + + next(); + }); + + middleware.assertPath = helpers.try(async (req, res, next) => { + // file: URL support + if (req.body.path.startsWith('file:///')) { + req.body.path = new URL(req.body.path).pathname; + } + + // Checks file exists and is within bounds of upload_path + const pathToFile = path.join(nconf.get('upload_path'), req.body.path); + res.locals.cleanedPath = pathToFile; + + if (!pathToFile.startsWith(nconf.get('upload_path'))) { + return controllerHelpers.formatApiResponse(403, res, new Error('[[error:invalid-path]]')); + } + + try { + await fsPromises.access(pathToFile, fs.constants.F_OK); + } catch (e) { + return controllerHelpers.formatApiResponse(404, res, new Error('[[error:invalid-path]]')); } next(); - }; + }); }; diff --git a/src/routes/write/files.js b/src/routes/write/files.js new file mode 100644 index 000000000000..f6e4e58f56c3 --- /dev/null +++ b/src/routes/write/files.js @@ -0,0 +1,16 @@ +'use strict'; + +const router = require('express').Router(); +const middleware = require('../../middleware'); +const controllers = require('../../controllers'); +const routeHelpers = require('../helpers'); + +const setupApiRoute = routeHelpers.setupApiRoute; + +module.exports = function () { + const middlewares = [middleware.authenticate]; + + setupApiRoute(router, '/', middleware, [...middlewares, middleware.checkRequired.bind(null, ['path']), middleware.assertPath], 'delete', controllers.write.files.delete); + + return router; +}; diff --git a/src/routes/write/index.js b/src/routes/write/index.js index 58bcc2ef20ac..c0764f65086b 100644 --- a/src/routes/write/index.js +++ b/src/routes/write/index.js @@ -26,6 +26,7 @@ Write.reload = (params) => { router.use('/api/v3/topics', require('./topics')()); router.use('/api/v3/posts', require('./posts')()); router.use('/api/v3/admin', require('./admin')()); + router.use('/api/v3/files', require('./files')()); router.get('/api/v3/ping', function (req, res) { helpers.formatApiResponse(200, res, { diff --git a/src/socket.io/admin/uploads.js b/src/socket.io/admin/uploads.js index d34edba503f8..32e1e334d337 100644 --- a/src/socket.io/admin/uploads.js +++ b/src/socket.io/admin/uploads.js @@ -4,9 +4,13 @@ const fs = require('fs'); const path = require('path'); const nconf = require('nconf'); +const sockets = require('..'); + const Uploads = module.exports; Uploads.delete = function (socket, pathToFile, callback) { + sockets.warnDeprecated(socket, 'DELETE /api/v3/files'); + pathToFile = path.join(nconf.get('upload_path'), pathToFile); if (!pathToFile.startsWith(nconf.get('upload_path'))) { return callback(new Error('[[error:invalid-path]]'));