From 2a751a4789f1316883cc06f44ab75390f6b5740a Mon Sep 17 00:00:00 2001
From: Ivar <ivarconr@gmail.com>
Date: Thu, 2 Nov 2017 10:30:14 +0100
Subject: [PATCH] Simplofy name validator

closes #271
---
 lib/app.js                           |  8 +++++++-
 lib/routes/admin-api/feature.js      | 11 +++--------
 lib/routes/admin-api/feature.test.js | 29 +++++++++++++++++++++++++++-
 3 files changed, 38 insertions(+), 10 deletions(-)

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)
+        )
+    );
+});