2
1
Fork 0
mirror of https://github.com/TryGhost/Ghost.git synced 2023-12-13 21:00:40 +01:00

feature: upload validation middleware (#7208)

no issue

- Source out validation logic into a upload validation middleware for all upload types (csv, image, subscribers). This unit can be later used for Ghost 1.0 as a pre validation core unit. 
- More usage of route tests than controller tests. These are use case tests, a use case only changes if the product changes
This commit is contained in:
Katharina Irrgang 2016-08-18 21:25:51 +02:00 committed by Hannah Wolfe
parent 3381449d78
commit 663b410fd4
23 changed files with 260 additions and 297 deletions

View file

@ -1,7 +1,6 @@
// # DB API
// API for DB operations
var _ = require('lodash'),
Promise = require('bluebird'),
var Promise = require('bluebird'),
exporter = require('../data/export'),
importer = require('../data/importer'),
backupDatabase = require('../data/migration').backupDatabase,
@ -9,8 +8,6 @@ var _ = require('lodash'),
errors = require('../errors'),
utils = require('./utils'),
pipeline = require('../utils/pipeline'),
i18n = require('../i18n'),
api = {},
docName = 'db',
db;
@ -62,31 +59,8 @@ db = {
*/
importContent: function (options) {
var tasks = [];
options = options || {};
function validate(options) {
options.name = options.originalname;
options.type = options.mimetype;
// Check if a file was provided
if (!utils.checkFileExists(options)) {
return Promise.reject(new errors.ValidationError(i18n.t('errors.api.db.selectFileToImport')));
}
// Check if the file is valid
if (!utils.checkFileIsValid(options, importer.getTypes(), importer.getExtensions())) {
return Promise.reject(new errors.UnsupportedMediaTypeError(
i18n.t('errors.api.db.unsupportedFile') +
_.reduce(importer.getExtensions(), function (memo, ext) {
return memo ? memo + ', ' + ext : ext;
})
));
}
return options;
}
function importContent(options) {
return importer.importFromFile(options)
.then(function () {
@ -96,7 +70,6 @@ db = {
}
tasks = [
validate,
utils.handlePermissions(docName, 'importContent'),
importContent
];

View file

@ -266,23 +266,6 @@ subscribers = {
var tasks = [];
options = options || {};
function validate(options) {
options.name = options.originalname;
options.type = options.mimetype;
// Check if a file was provided
if (!utils.checkFileExists(options)) {
return Promise.reject(new errors.ValidationError(i18n.t('errors.api.db.selectFileToImport')));
}
// Check for valid file type
if (!utils.checkFileIsValid(options, ['text/csv','application/csv'], ['.csv'])) {
return Promise.reject(new errors.ValidationError(i18n.t('errors.api.subscribers.selectValidFile')));
}
return options;
}
function importCSV(options) {
var filePath = options.path,
fulfilled = 0,
@ -326,7 +309,6 @@ subscribers = {
}
tasks = [
validate,
utils.handlePermissions(docName, 'add'),
importCSV
];

View file

@ -1,12 +1,7 @@
var config = require('../config'),
Promise = require('bluebird'),
var Promise = require('bluebird'),
fs = require('fs-extra'),
pUnlink = Promise.promisify(fs.unlink),
storage = require('../storage'),
errors = require('../errors'),
utils = require('./utils'),
i18n = require('../i18n'),
upload;
/**
@ -26,21 +21,6 @@ upload = {
add: Promise.method(function (options) {
var store = storage.getStorage();
// Public interface of the storage module's `save` method requires
// the file's name to be on the .name property.
options.name = options.originalname;
options.type = options.mimetype;
// Check if a file was provided
if (!utils.checkFileExists(options)) {
throw new errors.NoPermissionError(i18n.t('errors.api.upload.pleaseSelectImage'));
}
// Check if the file is valid
if (!utils.checkFileIsValid(options, config.uploads.contentTypes, config.uploads.extensions)) {
throw new errors.UnsupportedMediaTypeError(i18n.t('errors.api.upload.pleaseSelectValidImage'));
}
return store.save(options).finally(function () {
// Remove uploaded file from tmp location
return pUnlink(options.path);

View file

@ -225,10 +225,21 @@ ConfigManager.prototype.set = function (config) {
'signin', 'signout', 'signup', 'user', 'users', 'wp-admin', 'wp-login'],
protected: ['ghost', 'rss']
},
// used in middleware/validation/upload.js
// if we finish the data/importer logic, each type selects an importer
uploads: {
// Used by the upload API to limit uploads to images
extensions: ['.jpg', '.jpeg', '.gif', '.png', '.svg', '.svgz'],
contentTypes: ['image/jpeg', 'image/png', 'image/gif', 'image/svg+xml']
subscribers: {
extensions: ['.csv'],
contentTypes: ['text/csv','application/csv']
},
images: {
extensions: ['.jpg', '.jpeg', '.gif', '.png', '.svg', '.svgz'],
contentTypes: ['image/jpeg', 'image/png', 'image/gif', 'image/svg+xml']
},
db: {
extensions: ['.json'],
contentTypes: ['application/octet-stream', 'application/json']
}
},
deprecatedItems: ['updateCheck', 'mail.fromaddress'],
// create a hash for cache busting assets

View file

@ -8,8 +8,8 @@ var _ = require('lodash'),
ImageHandler = {
type: 'images',
extensions: config.uploads.extensions,
types: config.uploads.contentTypes,
extensions: config.uploads.images.extensions,
contentTypes: config.uploads.images.contentTypes,
directories: ['images', 'content'],
loadFile: function (files, baseDir) {

View file

@ -8,7 +8,7 @@ var _ = require('lodash'),
JSONHandler = {
type: 'data',
extensions: ['.json'],
types: ['application/octet-stream', 'application/json'],
contentTypes: ['application/octet-stream', 'application/json'],
directories: [],
loadFile: function (files, startDir) {

View file

@ -83,7 +83,7 @@ processMarkdownFile = function (filename, content) {
MarkdownHandler = {
type: 'data',
extensions: ['.md', '.markdown'],
types: ['application/octet-stream', 'text/plain'],
contentTypes: ['application/octet-stream', 'text/plain'],
directories: [],
loadFile: function (files, startDir) {

View file

@ -25,7 +25,7 @@ var _ = require('lodash'),
defaults = {
extensions: ['.zip'],
types: ['application/zip', 'application/x-zip-compressed'],
contentTypes: ['application/zip', 'application/x-zip-compressed'],
directories: []
};
@ -55,8 +55,8 @@ _.extend(ImportManager.prototype, {
* Get an array of all the mime types for which we have handlers
* @returns {string[]}
*/
getTypes: function () {
return _.flatten(_.union(_.map(this.handlers, 'types'), defaults.types));
getContentTypes: function () {
return _.flatten(_.union(_.map(this.handlers, 'contentTypes'), defaults.contentTypes));
},
/**
* Get an array of directories for which we have handlers

View file

@ -30,6 +30,7 @@ var bodyParser = require('body-parser'),
maintenance = require('./maintenance'),
versionMatch = require('./api/version-match'),
cors = require('./cors'),
validation = require('./validation'),
netjet = require('netjet'),
labs = require('./labs'),
helpers = require('../helpers'),
@ -42,6 +43,7 @@ var bodyParser = require('body-parser'),
middleware = {
upload: multer({dest: tmpdir()}),
validation: validation,
cacheControl: cacheControl,
spamPrevention: spamPrevention,
oauth: oauth,

View file

@ -0,0 +1 @@
exports.upload = require('./upload');

View file

@ -0,0 +1,30 @@
var apiUtils = require('../../api/utils'),
errors = require('../../errors'),
config = require('../../config'),
i18n = require('../../i18n');
module.exports = function upload(options) {
var type = options.type;
// if we finish the data/importer logic, we forward the request to the specified importer
return function (req, res, next) {
var extensions = (config.uploads[type] && config.uploads[type].extensions) || [],
contentTypes = (config.uploads[type] && config.uploads[type].contentTypes) || [];
req.file = req.file || {};
req.file.name = req.file.originalname;
req.file.type = req.file.mimetype;
// Check if a file was provided
if (!apiUtils.checkFileExists(req.file)) {
return next(new errors.NoPermissionError(i18n.t('errors.api.' + type + '.missingFile')));
}
// Check if the file is valid
if (!apiUtils.checkFileIsValid(req.file, contentTypes, extensions)) {
return next(new errors.UnsupportedMediaTypeError(i18n.t('errors.api.' + type + '.invalidFile', {extensions: extensions})));
}
next();
};
};

View file

@ -82,6 +82,7 @@ apiRoutes = function apiRoutes(middleware) {
middleware.api.labs.subscribers,
authenticatePrivate,
middleware.upload.single('subscribersfile'),
middleware.validation.upload({type: 'subscribers'}),
api.http(api.subscribers.importCSV)
);
router.get('/subscribers/:id', middleware.api.labs.subscribers, authenticatePrivate, api.http(api.subscribers.read));
@ -109,7 +110,12 @@ apiRoutes = function apiRoutes(middleware) {
// ## DB
router.get('/db', authenticatePrivate, api.http(api.db.exportContent));
router.post('/db', authenticatePrivate, middleware.upload.single('importfile'), api.http(api.db.importContent));
router.post('/db',
authenticatePrivate,
middleware.upload.single('importfile'),
middleware.validation.upload({type: 'db'}),
api.http(api.db.importContent)
);
router.del('/db', authenticatePrivate, api.http(api.db.deleteAllContent));
// ## Mail
@ -138,7 +144,13 @@ apiRoutes = function apiRoutes(middleware) {
router.post('/authentication/revoke', authenticatePrivate, api.http(api.authentication.revoke));
// ## Uploads
router.post('/uploads', authenticatePrivate, middleware.upload.single('uploadimage'), api.http(api.uploads.add));
// @TODO: rename endpoint to /images/upload (or similar)
router.post('/uploads',
authenticatePrivate,
middleware.upload.single('uploadimage'),
middleware.validation.upload({type: 'images'}),
api.http(api.uploads.add)
);
// API Router middleware
router.use(middleware.api.errorHandler);

View file

@ -309,10 +309,10 @@
"invalidKey": "Invalid key"
},
"db": {
"missingFile": "Please select a database file to import.",
"invalidFile": "Unsupported file. Please try any of the following formats: {extensions}",
"noPermissionToExportData": "You do not have permission to export data (no rights).",
"noPermissionToImportData": "You do not have permission to import data (no rights).",
"selectFileToImport": "Please select a file to import.",
"unsupportedFile": "Unsupported file. Please try any of the following formats: "
"noPermissionToImportData": "You do not have permission to import data (no rights)."
},
"mail": {
"noPermissionToSendEmail": "You do not have permission to send mail.",
@ -346,9 +346,10 @@
"unknownSlugType": "Unknown slug type '{type}'."
},
"subscribers": {
"missingFile": "Please select a csv.",
"invalidFile": "Please select a valid CSV file to import.",
"subscriberNotFound": "Subscriber not found.",
"subscriberAlreadyExists": "Email address is already subscribed.",
"selectValidFile": "Please select a valid CSV file to import."
"subscriberAlreadyExists": "Email address is already subscribed."
},
"tags": {
"tagNotFound": "Tag not found."
@ -359,9 +360,9 @@
"themeDoesNotExist": "Theme does not exist.",
"invalidRequest": "Invalid request."
},
"upload": {
"pleaseSelectImage": "Please select an image.",
"pleaseSelectValidImage": "Please select a valid image."
"images": {
"missingFile": "Please select an image.",
"invalidFile": "Please select a valid image."
},
"users": {
"userNotFound": "User not found.",

View file

@ -1,5 +1,6 @@
var supertest = require('supertest'),
should = require('should'),
path = require('path'),
testUtils = require('../../../utils'),
ghost = require('../../../../../core'),
request;
@ -61,4 +62,33 @@ describe('DB API', function () {
done();
});
});
it('import should fail without file', function (done) {
request.post(testUtils.API.getApiQuery('db/'))
.set('Authorization', 'Bearer ' + accesstoken)
.expect('Content-Type', /json/)
.expect(403)
.end(function (err) {
if (err) {
return done(err);
}
done();
});
});
it('import should fail with unsupported file', function (done) {
request.post(testUtils.API.getApiQuery('db/'))
.set('Authorization', 'Bearer ' + accesstoken)
.expect('Content-Type', /json/)
.attach('importfile', path.join(__dirname, '/../../../utils/fixtures/csv/single-column-with-header.csv'))
.expect(415)
.end(function (err) {
if (err) {
return done(err);
}
done();
});
});
});

View file

@ -0,0 +1,134 @@
var testUtils = require('../../../utils'),
/*jshint unused:false*/
should = require('should'),
path = require('path'),
fs = require('fs-extra'),
supertest = require('supertest'),
ghost = require('../../../../../core'),
config = require('../../../../../core/server/config'),
request;
describe('Upload API', function () {
var accesstoken = '',
images = [];
before(function (done) {
// starting ghost automatically populates the db
// TODO: prevent db init, and manage bringing up the DB with fixtures ourselves
ghost().then(function (ghostServer) {
request = supertest.agent(ghostServer.rootApp);
}).then(function () {
return testUtils.doAuth(request);
}).then(function (token) {
accesstoken = token;
done();
}).catch(done);
});
after(function (done) {
images.forEach(function (image) {
fs.removeSync(config.paths.appRoot + image);
});
testUtils.clearData().then(function () {
done();
}).catch(done);
});
describe('success cases', function () {
it('valid png', function (done) {
request.post(testUtils.API.getApiQuery('uploads'))
.set('Authorization', 'Bearer ' + accesstoken)
.expect('Content-Type', /json/)
.attach('uploadimage', path.join(__dirname, '/../../../utils/fixtures/images/ghost-logo.png'))
.expect(200)
.end(function (err, res) {
if (err) {
return done(err);
}
images.push(res.body);
done();
});
});
it('valid jpg', function (done) {
request.post(testUtils.API.getApiQuery('uploads'))
.set('Authorization', 'Bearer ' + accesstoken)
.expect('Content-Type', /json/)
.attach('uploadimage', path.join(__dirname, '/../../../utils/fixtures/images/ghosticon.jpg'))
.expect(200)
.end(function (err, res) {
if (err) {
return done(err);
}
images.push(res.body);
done();
});
});
it('valid gif', function (done) {
request.post(testUtils.API.getApiQuery('uploads'))
.set('Authorization', 'Bearer ' + accesstoken)
.expect('Content-Type', /json/)
.attach('uploadimage', path.join(__dirname, '/../../../utils/fixtures/images/loadingcat.gif'))
.expect(200)
.end(function (err, res) {
if (err) {
return done(err);
}
images.push(res.body);
done();
});
});
});
describe('error cases', function () {
it('import should fail without file', function (done) {
request.post(testUtils.API.getApiQuery('uploads'))
.set('Authorization', 'Bearer ' + accesstoken)
.expect('Content-Type', /json/)
.expect(403)
.end(function (err) {
if (err) {
return done(err);
}
done();
});
});
it('import should fail with unsupported file', function (done) {
request.post(testUtils.API.getApiQuery('uploads'))
.set('Authorization', 'Bearer ' + accesstoken)
.expect('Content-Type', /json/)
.attach('uploadimage', path.join(__dirname, '/../../../utils/fixtures/csv/single-column-with-header.csv'))
.expect(415)
.end(function (err) {
if (err) {
return done(err);
}
done();
});
});
it('incorrect extension', function (done) {
request.post(testUtils.API.getApiQuery('uploads'))
.set('Authorization', 'Bearer ' + accesstoken)
.set('content-type', 'image/png')
.expect('Content-Type', /json/)
.attach('uploadimage', path.join(__dirname, '/../../../utils/fixtures/images/ghost-logo.pngx'))
.expect(415)
.end(function (err) {
if (err) {
return done(err);
}
done();
});
});
});
});

View file

@ -110,22 +110,4 @@ describe('DB API', function () {
error.errorType.should.eql('NoPermissionError');
});
});
it('import content should fail without file & with unsupported file', function () {
return dbAPI.importContent(testUtils.context.admin).then(function () {
throw new Error('Import content is not failed without file.');
}, function (error) {
error.errorType.should.eql('ValidationError');
var context = _.extend(testUtils.context.admin, {
originalname: 'myFile.docx', path: '/my/path/myFile.docx', mimetype: 'application/docx'
});
return dbAPI.importContent(context);
}).then(function () {
throw new Error('Import content is not failed with unsupported.');
}, function (error) {
error.errorType.should.eql('UnsupportedMediaTypeError');
});
});
});

View file

@ -302,18 +302,5 @@ describe('Subscribers API', function () {
done();
});
});
it('throws an invalid file error', function (done) {
stub.returns(false);
SubscribersAPI.importCSV(_.merge(testUtils.context.internal, {path: '/somewhere'}))
.then(function () {
done(new Error('we expected an error here!'));
})
.catch(function (err) {
err.message.should.eql('Please select a valid CSV file to import.');
done();
});
});
});
});

View file

@ -1,162 +0,0 @@
var tmp = require('tmp'),
should = require('should'),
Promise = require('bluebird'),
configUtils = require('../../utils/configUtils'),
extname = require('path').extname,
// Stuff we are testing
UploadAPI = require('../../../server/api/upload'),
uploadimage = {
originalname: '',
mimetype: '',
path: ''
};
function setupAndTestUpload(filename, mimeType) {
return new Promise(function (resolve, reject) {
// create a temp file (the uploaded file)
tmp.file({keep: true}, function (err, path/*, fd, cleanupFile*/) {
if (err) {
return reject(err);
}
uploadimage.path = path;
uploadimage.originalname = filename;
uploadimage.mimetype = mimeType;
// create a temp directory (the directory that the API saves the file to)
tmp.dir({keep: true, unsafeCleanup: true}, function (err, path, cleanupDir) {
if (err) {
return reject(err);
}
configUtils.set({
paths: {
contentPath: path
}
});
UploadAPI.add(uploadimage)
.then(resolve)
.catch(reject)
.finally(function () {
// remove the temporary directory (file is unlinked by the API)
cleanupDir();
});
});
});
});
}
function testResult(filename, result) {
var base = filename,
ext = extname(base),
regex;
if (ext) {
base = base.split(ext)[0];
}
regex = new RegExp('^/content/images/[0-9]{4}/[0-9]{2}/' + base + '.*' + ext + '$');
regex.test(result).should.be.true();
}
describe('Upload API', function () {
afterEach(function () {
uploadimage = {
originalname: '',
mimetype: 'application/octet-stream',
path: ''
};
configUtils.restore();
});
should.exist(UploadAPI);
describe('invalid file extension and mime-type', function () {
it('should return 415 for invalid file extension', function (done) {
setupAndTestUpload('test.invalid', 'application/octet-stream').then(function () {
done(new Error('Upload succeeded with invalid extension and mime-type'));
}).catch(function (err) {
should.exist(err);
err.statusCode.should.equal(415);
err.errorType.should.equal('UnsupportedMediaTypeError');
done();
}).catch(done);
});
});
describe('valid extension but invalid type', function () {
it('should return 415 for mime-type', function (done) {
setupAndTestUpload('test.jpg', 'application/octet-stream').then(function () {
done(new Error('Upload succeeded with invalid mime-type'));
}).catch(function (err) {
should.exist(err);
err.statusCode.should.equal(415);
err.errorType.should.equal('UnsupportedMediaTypeError');
done();
}).catch(done);
});
});
describe('valid file', function () {
it('can upload jpg', function (done) {
var filename = 'test.jpg';
setupAndTestUpload(filename, 'image/jpeg').then(function (url) {
testResult(filename, url);
done();
}).catch(done);
});
it('cannot upload jpg with incorrect extension', function (done) {
setupAndTestUpload('invalid.xjpg', 'image/jpeg').then(function () {
done(new Error('Upload succeeded with invalid extension'));
}).catch(function (err) {
should.exist(err);
err.statusCode.should.equal(415);
err.errorType.should.equal('UnsupportedMediaTypeError');
done();
}).catch(done);
});
it('can upload png', function (done) {
var filename = 'test.png';
setupAndTestUpload(filename, 'image/png').then(function (url) {
testResult(filename, url);
done();
}).catch(done);
});
it('can upload gif', function (done) {
var filename = 'test.gif';
setupAndTestUpload(filename, 'image/gif').then(function (url) {
testResult(filename, url);
done();
}).catch(done);
});
});
describe('missing file', function () {
it('throws an error if no file is provided', function (done) {
UploadAPI.add({}).then(function () {
done(new Error('Upload succeeded with invalid extension'));
}).catch(function (err) {
should.exist(err);
err.statusCode.should.equal(403);
err.errorType.should.equal('NoPermissionError');
done();
}).catch(done);
});
});
});

View file

@ -45,12 +45,12 @@ describe('Importer', function () {
});
it('gets the correct types', function () {
ImportManager.getTypes().should.be.instanceof(Array).and.have.lengthOf(10);
ImportManager.getTypes().should.containEql('application/octet-stream');
ImportManager.getTypes().should.containEql('application/json');
ImportManager.getTypes().should.containEql('application/zip');
ImportManager.getTypes().should.containEql('application/x-zip-compressed');
ImportManager.getTypes().should.containEql('text/plain');
ImportManager.getContentTypes().should.be.instanceof(Array).and.have.lengthOf(10);
ImportManager.getContentTypes().should.containEql('application/octet-stream');
ImportManager.getContentTypes().should.containEql('application/json');
ImportManager.getContentTypes().should.containEql('application/zip');
ImportManager.getContentTypes().should.containEql('application/x-zip-compressed');
ImportManager.getContentTypes().should.containEql('text/plain');
});
it('gets the correct directories', function () {
@ -305,9 +305,9 @@ describe('Importer', function () {
JSONHandler.type.should.eql('data');
JSONHandler.extensions.should.be.instanceof(Array).and.have.lengthOf(1);
JSONHandler.extensions.should.containEql('.json');
JSONHandler.types.should.be.instanceof(Array).and.have.lengthOf(2);
JSONHandler.types.should.containEql('application/octet-stream');
JSONHandler.types.should.containEql('application/json');
JSONHandler.contentTypes.should.be.instanceof(Array).and.have.lengthOf(2);
JSONHandler.contentTypes.should.containEql('application/octet-stream');
JSONHandler.contentTypes.should.containEql('application/json');
JSONHandler.loadFile.should.be.instanceof(Function);
});
@ -350,11 +350,11 @@ describe('Importer', function () {
ImageHandler.extensions.should.containEql('.png');
ImageHandler.extensions.should.containEql('.svg');
ImageHandler.extensions.should.containEql('.svgz');
ImageHandler.types.should.be.instanceof(Array).and.have.lengthOf(4);
ImageHandler.types.should.containEql('image/jpeg');
ImageHandler.types.should.containEql('image/png');
ImageHandler.types.should.containEql('image/gif');
ImageHandler.types.should.containEql('image/svg+xml');
ImageHandler.contentTypes.should.be.instanceof(Array).and.have.lengthOf(4);
ImageHandler.contentTypes.should.containEql('image/jpeg');
ImageHandler.contentTypes.should.containEql('image/png');
ImageHandler.contentTypes.should.containEql('image/gif');
ImageHandler.contentTypes.should.containEql('image/svg+xml');
ImageHandler.loadFile.should.be.instanceof(Function);
});
@ -487,9 +487,9 @@ describe('Importer', function () {
MarkdownHandler.extensions.should.be.instanceof(Array).and.have.lengthOf(2);
MarkdownHandler.extensions.should.containEql('.md');
MarkdownHandler.extensions.should.containEql('.markdown');
MarkdownHandler.types.should.be.instanceof(Array).and.have.lengthOf(2);
MarkdownHandler.types.should.containEql('application/octet-stream');
MarkdownHandler.types.should.containEql('text/plain');
MarkdownHandler.contentTypes.should.be.instanceof(Array).and.have.lengthOf(2);
MarkdownHandler.contentTypes.should.containEql('application/octet-stream');
MarkdownHandler.contentTypes.should.containEql('text/plain');
MarkdownHandler.loadFile.should.be.instanceof(Function);
});

Binary file not shown.

After

Width:  |  Height:  |  Size: 8.4 KiB

Binary file not shown.

After

Width:  |  Height:  |  Size: 8.4 KiB

Binary file not shown.

After

Width:  |  Height:  |  Size: 2.4 KiB

Binary file not shown.

After

Width:  |  Height:  |  Size: 20 KiB