diff --git a/lib/app.js b/lib/app.js index 6b1600724a..24286f3f3c 100644 --- a/lib/app.js +++ b/lib/app.js @@ -46,7 +46,13 @@ module.exports = function(config) { }) ); - app.use(validator([])); + app.use( + validator({ + customValidators: { + isUrlFirendlyName: input => encodeURIComponent(input) === input, + }, + }) + ); if (publicFolder) { app.use(baseUriPath, express.static(publicFolder)); diff --git a/lib/routes/admin-api/feature.js b/lib/routes/admin-api/feature.js index ba3ef31e12..a7f565efc8 100644 --- a/lib/routes/admin-api/feature.js +++ b/lib/routes/admin-api/feature.js @@ -15,8 +15,6 @@ const ValidationError = require('../../error/validation-error.js'); const validateRequest = require('../../error/validate-request'); const extractUser = require('../../extract-user'); -const nameRegex = /^[0-9a-zA-Z\-._]+$/; - const handleErrors = (req, res, error) => { logger.warn('Error creating or updating feature', error); switch (error.constructor) { @@ -110,9 +108,7 @@ module.exports.router = function(config) { router.post('/validate', (req, res) => { req.checkBody('name', 'Name is required').notEmpty(); - req - .checkBody('name', 'Name must match format ^[0-9a-zA-Z\\.\\-\\_]+$') - .matches(/^[0-9a-zA-Z\\.\\-\\_]+$/i); + req.checkBody('name', 'Name must be URL friendly').isUrlFirendlyName(); validateRequest(req) .then(validateUniqueName) @@ -122,9 +118,8 @@ module.exports.router = function(config) { router.post('/', (req, res) => { req.checkBody('name', 'Name is required').notEmpty(); - req - .checkBody('name', `Name must match format ${nameRegex.source}`) - .matches(nameRegex); + req.checkBody('name', 'Name must be URL friendly').isUrlFirendlyName(); + const userName = extractUser(req); validateRequest(req) diff --git a/lib/routes/admin-api/feature.test.js b/lib/routes/admin-api/feature.test.js index 3cc428246f..c540f3ef48 100644 --- a/lib/routes/admin-api/feature.test.js +++ b/lib/routes/admin-api/feature.test.js @@ -96,7 +96,7 @@ test('should require at least one strategy when updating a feature toggle', t => .expect(400); }); -test('valid feature names pass validation', async t => { +test('valid feature names should pass validation', t => { t.plan(0); const { request, base } = getSetup(); @@ -123,3 +123,30 @@ test('valid feature names pass validation', async t => { ) ); }); + +test('invalid feature names should not pass validation', t => { + t.plan(0); + const { request, base } = getSetup(); + + const invalidNames = [ + 'some example', + 'some$example', + 'me&me', + ' ', + 'o2%ae', + ]; + + return Promise.all( + invalidNames.map(name => + request + .post(`${base}/api/admin/features`) + .send({ + name, + enabled: false, + strategies: [{ name: 'default' }], + }) + .set('Content-Type', 'application/json') + .expect(400) + ) + ); +});