Refactored controllers, moved checkPermission to permission-checker.js middleware

lib/middleware/permission-checker.js

@@ -0,0 +1,29 @@
+'use strict';
+
+const MissingPermission = require('../missing-permission');
+const { ADMIN } = require('../permissions');
+
+module.exports = function(config, permission) {
+    if (!permission || !config.extendedPermissions) {
+        return (req, res, next) => next();
+    }
+    return (req, res, next) => {
+        if (
+            req.user &&
+            req.user.permissions &&
+            (req.user.permissions.indexOf(ADMIN) !== -1 ||
+                req.user.permissions.indexOf(permission) !== -1)
+        ) {
+            return next();
+        }
+        return res
+            .status(403)
+            .json(
+                new MissingPermission({
+                    permission,
+                    message: `You require ${permission} to perform this action`,
+                })
+            )
+            .end();
+    };
+};

lib/middleware/permission-checker.test.js

@@ -1,10 +1,10 @@
 'use strict';
 
 const test = require('ava');
-const store = require('./../test/fixtures/store');
+const store = require('../../test/fixtures/store');
-const { requirePermission } = require('./permissions');
+const checkPermission = require('./permission-checker');
 const supertest = require('supertest');
-const getApp = require('./app');
+const getApp = require('../app');
 
 const { EventEmitter } = require('events');
 const eventBus = new EventEmitter();
@@ -16,13 +16,12 @@ function getSetup(preRouterHook) {
         baseUriPath: base,
         stores,
         eventBus,
-        extendedPermissions: true,
         preRouterHook(_app) {
             preRouterHook(_app);
 
             _app.get(
                 `${base}/protectedResource`,
-                requirePermission('READ'),
+                checkPermission({ extendedPermissions: true }, 'READ'),
                 (req, res) => {
                     res.status(200)
                         .json({ message: 'OK' })

lib/permissions.js

@@ -1,7 +1,5 @@
 'use strict';
 
-const MissingPermission = require('./missing-permission');
-
 const ADMIN = 'ADMIN';
 const CREATE_FEATURE = 'CREATE_FEATURE';
 const UPDATE_FEATURE = 'UPDATE_FEATURE';
@@ -11,30 +9,7 @@ const UPDATE_STRATEGY = 'UPDATE_STRATEGY';
 const DELETE_STRATEGY = 'DELETE_STRATEGY';
 const UPDATE_APPLICATION = 'UPDATE_APPLICATION';
 
-function requirePermission(permission) {
-    return (req, res, next) => {
-        if (
-            req.user &&
-            req.user.permissions &&
-            (req.user.permissions.indexOf(ADMIN) !== -1 ||
-                req.user.permissions.indexOf(permission) !== -1)
-        ) {
-            return next();
-        }
-        return res
-            .status(403)
-            .json(
-                new MissingPermission({
-                    permission,
-                    message: `You require ${permission} to perform this action`,
-                })
-            )
-            .end();
-    };
-}
-
 module.exports = {
-    requirePermission,
     ADMIN,
     CREATE_FEATURE,
     UPDATE_FEATURE,

lib/routes/admin-api/archive.js

@@ -8,10 +8,10 @@ const extractUser = require('../../extract-user');
 const { UPDATE_FEATURE } = require('../../permissions');
 
 class ArchiveController extends Controller {
-    constructor(extendedPerms, { featureToggleStore, eventStore }) {
+    constructor(config) {
-        super(extendedPerms);
+        super(config);
-        this.featureToggleStore = featureToggleStore;
+        this.featureToggleStore = config.stores.featureToggleStore;
-        this.eventStore = eventStore;
+        this.eventStore = config.stores.eventStore;
 
         this.get('/features', this.getArchivedFeatures);
         this.post('/revive/:name', this.reviveFeatureToggle, UPDATE_FEATURE);

lib/routes/admin-api/event.js

@@ -6,9 +6,9 @@ const eventDiffer = require('../../event-differ');
 const version = 1;
 
 class EventController extends Controller {
-    constructor({ eventStore }) {
+    constructor(config) {
-        super();
+        super(config);
-        this.eventStore = eventStore;
+        this.eventStore = config.stores.eventStore;
         this.get('/', this.getEvents);
         this.get('/:name', this.getEventsForToggle);
     }

lib/routes/admin-api/feature.js

@@ -20,10 +20,10 @@ const { featureShema, nameSchema } = require('./feature-schema');
 const version = 1;
 
 class FeatureController extends Controller {
-    constructor(extendedPerms, { featureToggleStore, eventStore }) {
+    constructor(config) {
-        super(extendedPerms);
+        super(config);
-        this.featureToggleStore = featureToggleStore;
+        this.featureToggleStore = config.stores.featureToggleStore;
-        this.eventStore = eventStore;
+        this.eventStore = config.stores.eventStore;
 
         this.get('/', this.getAllToggles);
         this.post('/', this.createToggle, CREATE_FEATURE);

lib/routes/admin-api/index.js

@@ -11,21 +11,15 @@ const apiDef = require('./api-def.json');
 
 class AdminApi extends Controller {
     constructor(config) {
-        super();
+        super(config);
-
-        const stores = config.stores;
-        const perms = config.extendedPermissions;
 
         this.app.get('/', this.index);
-        this.app.use('/features', new FeatureController(perms, stores).router);
+        this.app.use('/features', new FeatureController(config).router);
-        this.app.use('/archive', new ArchiveController(perms, stores).router);
+        this.app.use('/archive', new ArchiveController(config).router);
-        this.app.use(
+        this.app.use('/strategies', new StrategyController(config).router);
-            '/strategies',
+        this.app.use('/events', new EventController(config).router);
-            new StrategyController(perms, stores).router
+        this.app.use('/metrics', new MetricsController(config).router);
-        );
+        this.app.use('/user', new UserController(config).router);
-        this.app.use('/events', new EventController(stores).router);
-        this.app.use('/metrics', new MetricsController(perms, stores).router);
-        this.app.use('/user', new UserController(perms).router);
     }
 
     index(req, res) {

lib/routes/admin-api/metrics.js

@@ -8,17 +8,16 @@ const schema = require('./metrics-schema');
 const { UPDATE_APPLICATION } = require('../../permissions');
 
 class MetricsController extends Controller {
-    constructor(
+    constructor(config) {
-        extendedPerms,
+        super(config);
-        {
+        const {
             clientMetricsStore,
             clientInstanceStore,
             clientApplicationsStore,
             strategyStore,
             featureToggleStore,
-        }
+        } = config.stores;
-    ) {
+
-        super(extendedPerms);
         this.metrics = new ClientMetrics(clientMetricsStore);
         this.clientInstanceStore = clientInstanceStore;
         this.clientApplicationsStore = clientApplicationsStore;

lib/routes/admin-api/strategy.js

@@ -16,10 +16,10 @@ const {
 const version = 1;
 
 class StrategyController extends Controller {
-    constructor(extendedPerms, { strategyStore, eventStore }) {
+    constructor(config) {
-        super(extendedPerms);
+        super(config);
-        this.strategyStore = strategyStore;
+        this.strategyStore = config.stores.strategyStore;
-        this.eventStore = eventStore;
+        this.eventStore = config.stores.eventStore;
 
         this.get('/', this.getAllStratgies);
         this.get('/:name', this.getStrategy);

lib/routes/admin-api/user.js

@@ -3,8 +3,8 @@
 const Controller = require('../controller');
 
 class UserController extends Controller {
-    constructor(perms) {
+    constructor(config) {
-        super(perms);
+        super(config);
         this.get('/', this.getUser);
         this.get('/logout', this.logout);
     }
@@ -12,8 +12,10 @@ class UserController extends Controller {
     getUser(req, res) {
         if (req.user) {
             const user = Object.assign({}, req.user);
-            if (!this.extendedPermissions) {
+            if (!this.config.extendedPermissions) {
                 delete user.permissions;
+            } else if (!Array.isArray(user.permissions)) {
+                user.permissions = [];
             }
             return res
                 .status(200)

lib/routes/controller.js

@@ -1,28 +1,21 @@
 'use strict';
 
 const { Router } = require('express');
-const { requirePermission } = require('./../permissions');
+const checkPermission = require('../middleware/permission-checker');
 /**
  * Base class for Controllers to standardize binding to express Router.
  */
 class Controller {
-    constructor(extendedPermissions) {
+    constructor(config) {
         const router = Router();
         this.app = router;
-        this.extendedPermissions = extendedPermissions;
+        this.config = config;
-    }
-
-    checkPermission(permission) {
-        if (this.extendedPermissions && permission) {
-            return requirePermission(permission);
-        }
-        return (res, req, next) => next();
     }
 
     get(path, handler, permission) {
         this.app.get(
             path,
-            this.checkPermission(permission),
+            checkPermission(this.config, permission),
             handler.bind(this)
         );
     }
@@ -30,7 +23,7 @@ class Controller {
     post(path, handler, permission) {
         this.app.post(
             path,
-            this.checkPermission(permission),
+            checkPermission(this.config, permission),
             handler.bind(this)
         );
     }
@@ -38,7 +31,7 @@ class Controller {
     put(path, handler, permission) {
         this.app.put(
             path,
-            this.checkPermission(permission),
+            checkPermission(this.config, permission),
             handler.bind(this)
         );
     }
@@ -46,7 +39,7 @@ class Controller {
     delete(path, handler, permission) {
         this.app.delete(
             path,
-            this.checkPermission(permission),
+            checkPermission(this.config, permission),
             handler.bind(this)
         );
     }