Use full name instead of perms, 403 error message now includes expected permission

lib/missing-permission.js

@@ -0,0 +1,8 @@
+'use strict';
+
+module.exports = class MissingPermission {
+    constructor({ permission, message }) {
+        this.permission = permission;
+        this.message = message;
+    }
+};

lib/options.js

@@ -12,6 +12,7 @@ const DEFAULT_OPTIONS = {
     baseUriPath: process.env.BASE_URI_PATH || '',
     serverMetrics: true,
     enableLegacyRoutes: true,
+    extendedPermissions: false,
     publicFolder,
     enableRequestLogger: isDev(),
     secret: 'UNLEASH-SECRET',

lib/permissions.js

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

lib/permissions.test.js

@@ -2,7 +2,7 @@
 
 const test = require('ava');
 const store = require('./../test/fixtures/store');
-const { requirePerms } = require('./permissions');
+const { requirePermission } = require('./permissions');
 const supertest = require('supertest');
 const getApp = require('./app');
 
@@ -22,7 +22,7 @@ function getSetup(preRouterHook) {
 
             _app.get(
                 `${base}/protectedResource`,
-                requirePerms(['READ']),
+                requirePermission('READ'),
                 (req, res) => {
                     res.status(200)
                         .json({ message: 'OK' })

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

@@ -67,7 +67,7 @@ test('should revive toggle', t => {
     t.plan(0);
     const name = 'name1';
     const { request, base, archiveStore, perms } = getSetup();
-    perms.withPerms(UPDATE_FEATURE);
+    perms.withPermissions(UPDATE_FEATURE);
     archiveStore.addArchivedFeature({
         name,
         strategies: [{ name: 'default' }],
@@ -80,7 +80,7 @@ test('should create event when reviving toggle', async t => {
     t.plan(4);
     const name = 'name1';
     const { request, base, archiveStore, eventStore, perms } = getSetup();
-    perms.withPerms(UPDATE_FEATURE);
+    perms.withPermissions(UPDATE_FEATURE);
     archiveStore.addArchivedFeature({
         name,
         strategies: [{ name: 'default' }],

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

@@ -79,7 +79,7 @@ test('should add version numbers for /features', t => {
 test('should require at least one strategy when creating a feature toggle', t => {
     t.plan(0);
     const { request, base, perms } = getSetup();
-    perms.withPerms(CREATE_FEATURE);
+    perms.withPermissions(CREATE_FEATURE);
 
     return request
         .post(`${base}/api/admin/features`)
@@ -91,7 +91,7 @@ test('should require at least one strategy when creating a feature toggle', t =>
 test('should be allowed to use new toggle name', t => {
     t.plan(0);
     const { request, base, perms } = getSetup();
-    perms.withPerms(CREATE_FEATURE);
+    perms.withPermissions(CREATE_FEATURE);
 
     return request
         .post(`${base}/api/admin/features/validate`)
@@ -145,7 +145,7 @@ test('should not be allowed to reuse archived toggle name', t => {
 test('should require at least one strategy when updating a feature toggle', t => {
     t.plan(0);
     const { request, featureToggleStore, base, perms } = getSetup();
-    perms.withPerms(UPDATE_FEATURE);
+    perms.withPermissions(UPDATE_FEATURE);
     featureToggleStore.addFeature({
         name: 'ts',
         strategies: [{ name: 'default' }],
@@ -161,7 +161,7 @@ test('should require at least one strategy when updating a feature toggle', t =>
 test('valid feature names should pass validation', t => {
     t.plan(0);
     const { request, base, perms } = getSetup();
-    perms.withPerms(CREATE_FEATURE);
+    perms.withPermissions(CREATE_FEATURE);
 
     const validNames = [
         'com.example',
@@ -190,7 +190,7 @@ test('valid feature names should pass validation', t => {
 test('invalid feature names should not pass validation', t => {
     t.plan(0);
     const { request, base, perms } = getSetup();
-    perms.withPerms(CREATE_FEATURE);
+    perms.withPermissions(CREATE_FEATURE);
 
     const invalidNames = [
         'some example',
@@ -219,7 +219,7 @@ test('invalid feature names should not pass validation', t => {
 test('invalid feature names should have error msg', t => {
     t.plan(1);
     const { request, base, perms } = getSetup();
-    perms.withPerms(CREATE_FEATURE);
+    perms.withPermissions(CREATE_FEATURE);
 
     const name = 'ØÆ`';
 

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

@@ -133,7 +133,7 @@ test('should store application', t => {
     t.plan(0);
     const { request, perms } = getSetup();
     const appName = '123!23';
-    perms.withPerms(UPDATE_APPLICATION);
+    perms.withPermissions(UPDATE_APPLICATION);
 
     return request
         .post(`/api/admin/metrics/applications/${appName}`)

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

@@ -50,7 +50,7 @@ test('add version numbers for /stategies', t => {
 test('require a name when creating a new stratey', t => {
     t.plan(1);
     const { request, base, perms } = getSetup();
-    perms.withPerms(CREATE_STRATEGY);
+    perms.withPermissions(CREATE_STRATEGY);
 
     return request
         .post(`${base}/api/admin/strategies`)
@@ -64,7 +64,7 @@ test('require a name when creating a new stratey', t => {
 test('require parameters array when creating a new stratey', t => {
     t.plan(1);
     const { request, base, perms } = getSetup();
-    perms.withPerms(CREATE_STRATEGY);
+    perms.withPermissions(CREATE_STRATEGY);
 
     return request
         .post(`${base}/api/admin/strategies`)
@@ -78,7 +78,7 @@ test('require parameters array when creating a new stratey', t => {
 test('create a new stratey with empty parameters', t => {
     t.plan(0);
     const { request, base, perms } = getSetup();
-    perms.withPerms(CREATE_STRATEGY);
+    perms.withPermissions(CREATE_STRATEGY);
 
     return request
         .post(`${base}/api/admin/strategies`)
@@ -89,7 +89,7 @@ test('create a new stratey with empty parameters', t => {
 test('not be possible to override name', t => {
     t.plan(0);
     const { request, base, strategyStore, perms } = getSetup();
-    perms.withPerms(CREATE_STRATEGY);
+    perms.withPermissions(CREATE_STRATEGY);
     strategyStore.addStrategy({ name: 'Testing', parameters: [] });
 
     return request
@@ -102,7 +102,7 @@ test('update strategy', t => {
     t.plan(0);
     const name = 'AnotherStrat';
     const { request, base, strategyStore, perms } = getSetup();
-    perms.withPerms(UPDATE_STRATEGY);
+    perms.withPermissions(UPDATE_STRATEGY);
     strategyStore.addStrategy({ name, parameters: [] });
 
     return request
@@ -115,7 +115,7 @@ test('not update unknown strategy', t => {
     t.plan(0);
     const name = 'UnknownStrat';
     const { request, base, perms } = getSetup();
-    perms.withPerms(UPDATE_STRATEGY);
+    perms.withPermissions(UPDATE_STRATEGY);
 
     return request
         .put(`${base}/api/admin/strategies/${name}`)
@@ -127,7 +127,7 @@ test('validate format when updating strategy', t => {
     t.plan(0);
     const name = 'AnotherStrat';
     const { request, base, strategyStore, perms } = getSetup();
-    perms.withPerms(UPDATE_STRATEGY);
+    perms.withPermissions(UPDATE_STRATEGY);
     strategyStore.addStrategy({ name, parameters: [] });
 
     return request
@@ -140,7 +140,7 @@ test('editable=false will stop delete request', t => {
     t.plan(0);
     const name = 'default';
     const { request, base, perms } = getSetup();
-    perms.withPerms(DELETE_STRATEGY);
+    perms.withPermissions(DELETE_STRATEGY);
 
     return request.delete(`${base}/api/admin/strategies/${name}`).expect(500);
 });
@@ -149,7 +149,7 @@ test('editable=false will stop edit request', t => {
     t.plan(0);
     const name = 'default';
     const { request, base, perms } = getSetup();
-    perms.withPerms(UPDATE_STRATEGY);
+    perms.withPermissions(UPDATE_STRATEGY);
 
     return request
         .put(`${base}/api/admin/strategies/${name}`)
@@ -161,7 +161,7 @@ test('editable=true will allow delete request', t => {
     t.plan(0);
     const name = 'deleteStrat';
     const { request, base, strategyStore, perms } = getSetup();
-    perms.withPerms(DELETE_STRATEGY);
+    perms.withPermissions(DELETE_STRATEGY);
     strategyStore.addStrategy({ name, parameters: [] });
 
     return request
@@ -174,7 +174,7 @@ test('editable=true will allow edit request', t => {
     t.plan(0);
     const name = 'editStrat';
     const { request, base, strategyStore, perms } = getSetup();
-    perms.withPerms(UPDATE_STRATEGY);
+    perms.withPermissions(UPDATE_STRATEGY);
     strategyStore.addStrategy({ name, parameters: [] });
 
     return request

lib/routes/controller.js

@@ -1,7 +1,7 @@
 'use strict';
 
 const { Router } = require('express');
-const { requirePerms } = require('./../permissions');
+const { requirePermission } = require('./../permissions');
 /**
  * Base class for Controllers to standardize binding to express Router.
  */
@@ -12,30 +12,46 @@ class Controller {
         this.extendedPerms = extendedPerms;
     }
 
-    get(path, handler, ...perms) {
+    get(path, handler, permission) {
-        if (this.extendedPerms && perms.length > 0) {
+        if (this.extendedPerms && permission) {
-            this.app.get(path, requirePerms(perms), handler.bind(this));
+            this.app.get(
+                path,
+                requirePermission(permission),
+                handler.bind(this)
+            );
         }
         this.app.get(path, handler.bind(this));
     }
 
-    post(path, handler, ...perms) {
+    post(path, handler, permission) {
-        if (this.extendedPerms && perms.length > 0) {
+        if (this.extendedPerms && permission) {
-            this.app.post(path, requirePerms(perms), handler.bind(this));
+            this.app.post(
+                path,
+                requirePermission(permission),
+                handler.bind(this)
+            );
         }
         this.app.post(path, handler.bind(this));
     }
 
-    put(path, handler, ...perms) {
+    put(path, handler, permission) {
-        if (this.extendedPerms && perms.length > 0) {
+        if (this.extendedPerms && permission) {
-            this.app.put(path, requirePerms(perms), handler.bind(this));
+            this.app.put(
+                path,
+                requirePermission(permission),
+                handler.bind(this)
+            );
         }
         this.app.put(path, handler.bind(this));
     }
 
-    delete(path, handler, ...perms) {
+    delete(path, handler, permission) {
-        if (this.extendedPerms && perms.length > 0) {
+        if (this.extendedPerms && permission) {
-            this.app.delete(path, requirePerms(perms), handler.bind(this));
+            this.app.delete(
+                path,
+                requirePermission(permission),
+                handler.bind(this)
+            );
         }
         this.app.delete(path, handler.bind(this));
     }

test/fixtures/permissions.js

@@ -10,8 +10,8 @@ module.exports = () => {
                 next();
             });
         },
-        withPerms(...prms) {
+        withPermissions(...perms) {
-            _perms = prms;
+            _perms = perms;
         },
     };
 };