chore(modernize): Use joi for url-friendly name validation

lib/error/name-invalid-error.js

@@ -1,14 +0,0 @@
-'use strict';
-
-class NameInvalidError extends Error {
-    constructor(message) {
-        super();
-        Error.captureStackTrace(this, this.constructor);
-
-        this.name = this.constructor.name;
-        this.message = message;
-        this.msg = message;
-    }
-}
-
-module.exports = NameInvalidError;

lib/routes/admin-api/feature-schema.js

@@ -1,22 +1,19 @@
 'use strict';
 
 const joi = require('joi');
+const { nameType } = require('./util');
+
+const nameSchema = joi.object().keys({ name: nameType });
 
 const strategiesSchema = joi.object().keys({
-    name: joi
+    name: nameType,
-        .string()
-        .regex(/^[a-zA-Z0-9\\.\\-]{3,100}$/)
-        .required(),
     parameters: joi.object(),
 });
 
 const featureShema = joi
     .object()
     .keys({
-        name: joi
+        name: nameType,
-            .string()
-            .regex(/^[a-zA-Z0-9\\.\\-]{3,100}$/)
-            .required(),
         enabled: joi.boolean().default(false),
         description: joi.string(),
         strategies: joi
@@ -27,4 +24,4 @@ const featureShema = joi
     })
     .options({ allowUnknown: false, stripUnknown: true });
 
-module.exports = { featureShema, strategiesSchema };
+module.exports = { featureShema, strategiesSchema, nameSchema };

lib/routes/admin-api/feature-schema.test.js

@@ -0,0 +1,27 @@
+'use strict';
+
+const { test } = require('ava');
+const { featureShema } = require('./feature-schema');
+const joi = require('joi');
+
+test('should require URL firendly name', t => {
+    const toggle = {
+        name: 'io`dasd',
+        enabled: false,
+        strategies: [{ name: 'default' }],
+    };
+
+    const { error } = joi.validate(toggle, featureShema);
+    t.deepEqual(error.details[0].message, '"name" must be URL friendly');
+});
+
+test('should be valid toggle name', t => {
+    const toggle = {
+        name: 'app.name',
+        enabled: false,
+        strategies: [{ name: 'default' }],
+    };
+
+    const { value } = joi.validate(toggle, featureShema);
+    t.deepEqual(value, toggle);
+});

lib/routes/admin-api/feature.js

@@ -9,10 +9,9 @@ const {
     FEATURE_ARCHIVED,
 } = require('../../event-type');
 const NameExistsError = require('../../error/name-exists-error');
-const NameInvalidError = require('../../error/name-invalid-error');
+const { handleErrors } = require('./util');
-const { isUrlFriendlyName, handleErrors } = require('./util');
 const extractUser = require('../../extract-user');
-const { featureShema } = require('./feature-schema');
+const { featureShema, nameSchema } = require('./feature-schema');
 const version = 1;
 
 class FeatureController extends Controller {
@@ -37,10 +36,8 @@ class FeatureController extends Controller {
 
     async getToggle(req, res) {
         try {
-            const featureName = req.params.featureName;
+            const name = req.params.featureName;
-            const feature = await this.featureToggleStore.getFeature(
+            const feature = await this.featureToggleStore.getFeature(name);
-                featureName
-            );
             res.json(feature).end();
         } catch (err) {
             res.status(404).json({ error: 'Could not find feature' });
@@ -51,7 +48,7 @@ class FeatureController extends Controller {
         const name = req.body.name;
 
         try {
-            await this.validateNameFormat(name);
+            await joi.validate({ name }, nameSchema);
             await this.validateUniqueName(name);
             res.status(201).end();
         } catch (error) {
@@ -76,19 +73,11 @@ class FeatureController extends Controller {
         throw new NameExistsError(msg);
     }
 
-    // TODO: this should be part of the schema validation!
-    validateNameFormat(name) {
-        if (!isUrlFriendlyName(name)) {
-            throw new NameInvalidError('Name must be URL friendly');
-        }
-    }
-
     async createToggle(req, res) {
         const toggleName = req.body.name;
         const userName = extractUser(req);
 
         try {
-            await this.validateNameFormat(toggleName);
             await this.validateUniqueName(toggleName);
             const featureToggle = await joi.validate(req.body, featureShema);
             await this.eventStore.store({

lib/routes/admin-api/feature.test.js

@@ -218,6 +218,8 @@ test('invalid feature names should have error msg', t => {
         .set('Content-Type', 'application/json')
         .expect(400)
         .expect(res => {
-            t.true(res.body[0].msg === 'Name must be URL friendly');
+            t.true(
+                res.body.details[0].message === '"name" must be URL friendly'
+            );
         });
 });

lib/routes/admin-api/strategy-schema.js

@@ -1,12 +1,10 @@
 'use strict';
 
 const joi = require('joi');
+const { nameType } = require('./util');
 
 const strategySchema = joi.object().keys({
-    name: joi
+    name: nameType,
-        .string()
-        .regex(/^[a-zA-Z0-9\\.\\-]{3,100}$/)
-        .required(),
     editable: joi.boolean().default(true),
     description: joi.string(),
     parameters: joi

lib/routes/admin-api/util.js

@@ -2,18 +2,45 @@
 
 const logger = require('../../logger')('/admin-api/util.js');
 
-const isUrlFriendlyName = input => encodeURIComponent(input) === input;
+const joi = require('joi');
+
+const customJoi = joi.extend(j => ({
+    base: j.string(),
+    name: 'string',
+    language: {
+        isUrlFriendly: 'must be URL friendly',
+    },
+    rules: [
+        {
+            name: 'isUrlFriendly',
+            validate(params, value, state, options) {
+                if (encodeURIComponent(value) !== value) {
+                    // Generate an error, state and options need to be passed
+                    return this.createError(
+                        'string.isUrlFriendly',
+                        { v: value },
+                        state,
+                        options
+                    );
+                }
+                return value; // Everything is OK
+            },
+        },
+    ],
+}));
+
+const nameType = customJoi
+    .string()
+    .isUrlFriendly()
+    .min(2)
+    .max(100)
+    .required();
 
 const handleErrors = (res, error) => {
     logger.warn(error.message);
     switch (error.name) {
         case 'NotFoundError':
             return res.status(404).end();
-        case 'NameInvalidError':
-            return res
-                .status(400)
-                .json([{ msg: error.message }])
-                .end();
         case 'NameExistsError':
             return res
                 .status(403)
@@ -30,4 +57,4 @@ const handleErrors = (res, error) => {
     }
 };
 
-module.exports = { isUrlFriendlyName, handleErrors };
+module.exports = { nameType, handleErrors };