fix: add middleware verifying content type

- By default only accepts 'application/json'
- Routes that need different content-type, can call post or put with
  additional arguments, one per content-type you need to support.

lib/middleware/content_type_checker.js

@@ -0,0 +1,30 @@
+const DEFAULT_ACCEPTED_CONTENT_TYPE = 'application/json';
+
+/**
+ * Builds an express middleware checking the content-type header
+ * returning 415 if the header is not either `application/json` or in the array
+ * passed into the function of valid content-types
+ * @param {String} acceptedContentTypes
+ * @returns {function(Request, Response, NextFunction): void}
+ */
+function requireContentType(...acceptedContentTypes) {
+    return (req, res, next) => {
+        const contentType = req.header('Content-Type');
+        if (
+            Array.isArray(acceptedContentTypes) &&
+            acceptedContentTypes.length > 0
+        ) {
+            if (acceptedContentTypes.includes(contentType)) {
+                next();
+            } else {
+                res.status(415).end();
+            }
+        } else if (DEFAULT_ACCEPTED_CONTENT_TYPE === contentType) {
+            next();
+        } else {
+            res.status(415).end();
+        }
+    };
+}
+
+module.exports = requireContentType;

lib/middleware/content_type_checker.test.js

@@ -0,0 +1,63 @@
+const test = require('ava');
+
+const requireContentType = require('./content_type_checker');
+
+const mockRequest = contentType => ({
+    header: name => {
+        if (name === 'Content-Type') {
+            return contentType;
+        }
+        return '';
+    },
+});
+
+const returns415 = t => {
+    return {
+        status: code => {
+            t.is(415, code);
+            return {
+                end: t.pass,
+            };
+        },
+    };
+};
+
+const expectNoCall = t => {
+    return {
+        status: () => {
+            return {
+                end: t.fail,
+            };
+        },
+    };
+};
+
+test('Content-type middleware should by default only support application/json', t => {
+    const middleware = requireContentType();
+    middleware(mockRequest('application/json'), expectNoCall(t), t.pass);
+    middleware(mockRequest('text/plain'), returns415(t), t.fail);
+});
+
+test('Content-type middleware should allow adding custom supported types', t => {
+    const middleware = requireContentType('application/yaml');
+    middleware(mockRequest('application/yaml'), expectNoCall(t), t.pass);
+    middleware(mockRequest('text/html'), returns415(t), t.fail);
+    middleware(mockRequest('text/plain'), returns415(t), t.fail);
+});
+
+test('adding custom supported types no longer supports default type', t => {
+    const middleware = requireContentType('application/yaml');
+    middleware(mockRequest('application/json'), returns415(t), t.fail);
+});
+
+test('Should be able to add multiple content-types supported', t => {
+    const middleware = requireContentType(
+        'application/json',
+        'application/yaml',
+        'form/multipart',
+    );
+    middleware(mockRequest('application/json'), expectNoCall(t), t.pass);
+    middleware(mockRequest('application/yaml'), expectNoCall(t), t.pass);
+    middleware(mockRequest('form/multipart'), expectNoCall(t), t.pass);
+    middleware(mockRequest('text/plain'), returns415(t), t.fail);
+});

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

@@ -79,7 +79,10 @@ test('should revive toggle', t => {
         strategies: [{ name: 'default' }],
     });
 
-    return request.post(`${base}/api/admin/archive/revive/${name}`).expect(200);
+    return request
+        .post(`${base}/api/admin/archive/revive/${name}`)
+        .set('Content-Type', 'application/json')
+        .expect(200);
 });
 
 test('should create event when reviving toggle', async t => {
@@ -108,7 +111,9 @@ test('should create event when reviving toggle', async t => {
         'test@test.com',
     );
 
-    await request.post(`${base}/api/admin/archive/revive/${name}`);
+    await request
+        .post(`${base}/api/admin/archive/revive/${name}`)
+        .set('Content-Type', 'application/json');
 
     const events = await eventStore.getEvents();
     t.is(events.length, 3);

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

@@ -110,6 +110,20 @@ test('should be allowed to use new toggle name', t => {
         .expect(200);
 });
 
+test('should get unsupported media-type when posting as form-url-encoded', t => {
+    t.plan(0);
+    const { request, base, perms } = getSetup();
+    perms.withPermissions(CREATE_FEATURE);
+
+    return request
+        .post(`${base}/api/admin/features`)
+        .type('form')
+        .send({ name: 'new.name' })
+        .send({ enabled: true })
+        .send({ strategies: [{ name: 'default' }] })
+        .expect(415);
+});
+
 test('should be allowed to have variants="null"', t => {
     t.plan(0);
     const { request, base, perms } = getSetup();
@@ -185,6 +199,23 @@ test('should require at least one strategy when updating a feature toggle', t =>
         .expect(400);
 });
 
+test('updating a feature toggle also requires application/json as content-type', t => {
+    t.plan(0);
+    const { request, featureToggleStore, base, perms } = getSetup();
+    perms.withPermissions(UPDATE_FEATURE);
+    featureToggleStore.createFeature({
+        name: 'ts',
+        strategies: [{ name: 'default' }],
+    });
+
+    return request
+        .put(`${base}/api/admin/features/ts`)
+        .type('form')
+        .send({ name: 'ts' })
+        .send({ strategies: [{ name: 'default' }] })
+        .expect(415);
+});
+
 test('valid feature names should pass validation', t => {
     t.plan(0);
     const { request, base, perms } = getSetup();
@@ -320,6 +351,7 @@ test('should toggle on', t => {
 
     return request
         .post(`${base}/api/admin/features/toggle.disabled/toggle/on`)
+        .set('Content-Type', 'application/json')
         .expect('Content-Type', /json/)
         .expect(200)
         .expect(res => {
@@ -340,6 +372,7 @@ test('should toggle off', t => {
 
     return request
         .post(`${base}/api/admin/features/toggle.enabled/toggle/off`)
+        .set('Content-Type', 'application/json')
         .expect('Content-Type', /json/)
         .expect(200)
         .expect(res => {
@@ -360,6 +393,7 @@ test('should toggle', t => {
 
     return request
         .post(`${base}/api/admin/features/toggle.disabled/toggle`)
+        .set('Content-Type', 'application/json')
         .expect('Content-Type', /json/)
         .expect(200)
         .expect(res => {

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

@@ -206,6 +206,7 @@ test('deprecating a strategy works', async t => {
 
     await request
         .post(`${base}/api/admin/strategies/${name}/deprecate`)
+        .set('Content-Type', 'application/json')
         .send()
         .expect(200);
     return request
@@ -220,6 +221,7 @@ test('deprecating a non-existent strategy yields 404', t => {
     perms.withPermissions(UPDATE_STRATEGY);
     return request
         .post(`${base}/api/admin/strategies/non-existent-strategy/deprecate`)
+        .set('Content-Type', 'application/json')
         .expect(404);
 });
 
@@ -232,6 +234,7 @@ test('reactivating a strategy works', async t => {
 
     await request
         .post(`${base}/api/admin/strategies/${name}/reactivate`)
+        .set('Content-Type', 'application/json')
         .send()
         .expect(200);
     return request
@@ -246,6 +249,7 @@ test('reactivating a non-existent strategy yields 404', t => {
     perms.withPermissions(UPDATE_STRATEGY);
     return request
         .post(`${base}/api/admin/strategies/non-existent-strategy/reactivate`)
+        .set('Content-Type', 'application/json')
         .expect(404);
 });
 test(`deprecating 'default' strategy will yield 403`, t => {
@@ -254,5 +258,6 @@ test(`deprecating 'default' strategy will yield 403`, t => {
     perms.withPermissions(UPDATE_STRATEGY);
     return request
         .post(`${base}/api/admin/strategies/default/deprecate`)
+        .set('Content-Type', 'application/json')
         .expect(403);
 });

lib/routes/client-api/register.test.js

@@ -65,7 +65,10 @@ test('should register client without sdkVersin', t => {
 test('should require appName field', t => {
     t.plan(0);
     const { request } = getSetup();
-    return request.post('/api/client/register').expect(400);
+    return request
+        .post('/api/client/register')
+        .set('Content-Type', 'application/json')
+        .expect(400);
 });
 
 test('should require strategies field', t => {

lib/routes/controller.js

@@ -2,6 +2,7 @@
 
 const { Router } = require('express');
 const checkPermission = require('../middleware/permission-checker');
+const requireContentType = require('../middleware/content_type_checker');
 /**
  * Base class for Controllers to standardize binding to express Router.
  */
@@ -20,18 +21,20 @@ class Controller {
         );
     }
 
-    post(path, handler, permission) {
+    post(path, handler, permission, ...acceptedContentTypes) {
         this.app.post(
             path,
             checkPermission(this.config, permission),
+            requireContentType(...acceptedContentTypes),
             handler.bind(this),
         );
     }
 
-    put(path, handler, permission) {
+    put(path, handler, permission, ...acceptedContentTypes) {
         this.app.put(
             path,
             checkPermission(this.config, permission),
+            requireContentType(...acceptedContentTypes),
             handler.bind(this),
         );
     }

package.json

@@ -128,12 +128,10 @@
   },
   "lint-staged": {
     "*.js": [
-      "eslint --fix",
-      "git add"
+      "eslint --fix"
     ],
     "*.{json,css,md}": [
-      "prettier --write",
-      "git add"
+      "prettier --write"
     ]
   },
   "husky": {

test/e2e/api/admin/strategy.e2e.test.js

@@ -130,6 +130,7 @@ test.serial('deprecating a strategy works', async t => {
         .expect(201);
     await request
         .post(`/api/admin/strategies/${name}/deprecate`)
+        .set('Content-Type', 'application/json')
         .send()
         .expect(200);
     await request
@@ -149,6 +150,7 @@ test.serial('can reactivate a deprecated strategy', async t => {
         .expect(201);
     await request
         .post(`/api/admin/strategies/${name}/deprecate`)
+        .set('Content-Type', 'application/json')
         .send()
         .expect(200);
     await request
@@ -158,6 +160,7 @@ test.serial('can reactivate a deprecated strategy', async t => {
         .expect(res => t.is(res.body.deprecated, true));
     await request
         .post(`/api/admin/strategies/${name}/reactivate`)
+        .set('Content-Type', 'application/json')
         .send()
         .expect(200);
     await request
@@ -170,7 +173,10 @@ test.serial('can reactivate a deprecated strategy', async t => {
 test.serial('cannot deprecate default strategy', async t => {
     t.plan(0);
     const request = await setupApp(stores);
-    await request.post('/api/admin/strategies/default/deprecate').expect(403);
+    await request
+        .post('/api/admin/strategies/default/deprecate')
+        .set('Content-Type', 'application/json')
+        .expect(403);
 });
 
 test.serial('can update a exiting strategy with deprecated', async t => {