fix: migrate all permissions to rbac (#782)

* fix: migrate all permissions to rbac
* fix: update migration guide

fixes #782

docs/deploy/migration-guide.md

@@ -5,6 +5,31 @@ title: Migration Guide
 
 Generally, the intention is that `unleash-server` should always provide support for clients one major version lower than the current one. This should make it possible to upgrade `unleash` gradually.
 
+## Upgrading from v3.x to v4.x
+
+(Work In Process!)
+
+### Role-based Access Control (RBAC)
+
+We have implemented RBAC in Unleash v4. This has totally changed the permission system in Unleash.
+
+**Required actions:** If you have implemented "custom authentication" for your users you will need to make changes to your integration:
+
+- _extendedPermissions_ option has been removed. You can no longer specify custom permission per-user basis. All "logged_in users" must belong to a "root" role. This can be "Admin", "Editor" or "Viewer". This is taken care of when you create new users via userService.
+- All "logged-in users" needs to be defined in Unleash and have a unique ID. This can be achieved by calling "createUser" on "userService".
+
+Code example:
+
+```js
+const user = userService.loginUserWithoutPassword(
+  'some@getunleash.io',
+  false, // autoCreateUser. Set to true if you want to create users on the fly.
+);
+
+// The user needs to be set on the current active session
+req.session.user = user;
+```
+
 ## Upgrading from v2.x to v3.x
 
 The notable change introduced in Unleash v3.x is a strict separation of API paths for client requests and admin requests. This makes it easier to implement different authentication mechanisms for the admin UI and all unleash-clients. You can read more about [securing unleash](https://github.com/Unleash/unleash/blob/master/docs/securing-unleash.md).

src/lib/app.test.js

@@ -14,13 +14,14 @@ const getApp = proxyquire('./app', {
 });
 
 test('should not throw when valid config', t => {
-    const app = getApp({ getLogger });
+    const app = getApp({ getLogger, stores: {} });
     t.true(typeof app.listen === 'function');
 });
 
 test('should call preHook', t => {
     let called = 0;
     getApp({
+        stores: {},
         getLogger,
         preHook: () => {
             called++;
@@ -32,6 +33,7 @@ test('should call preHook', t => {
 test('should call preRouterHook', t => {
     let called = 0;
     getApp({
+        stores: {},
         getLogger,
         preRouterHook: () => {
             called++;

src/lib/app.ts

@@ -30,7 +30,7 @@ module.exports = function(config, services = {}) {
     app.locals.baseUriPath = baseUriPath;
 
     if (typeof config.preHook === 'function') {
-        config.preHook(app);
+        config.preHook(app, config, services);
     }
 
     app.use(compression());

src/lib/db/access-store.ts

@@ -44,13 +44,13 @@ export class AccessStore {
             });
     }
 
-    async getPermissionsForUser(userId: Number): Promise<IUserPermission[]> {
+    async getPermissionsForUser(userId: number): Promise<IUserPermission[]> {
         const stopTimer = this.timer('getPermissionsForUser');
         const rows = await this.db
             .select('project', 'permission')
             .from<IUserPermission>(`${T.ROLE_PERMISSION} AS rp`)
             .leftJoin(`${T.ROLE_USER} AS ur`, 'ur.role_id', 'rp.role_id')
-            .where('user_id', '=', userId);
+            .where('ur.user_id', '=', userId);
         stopTimer();
         return rows;
     }

src/lib/middleware/api-token-middleware.ts

@@ -8,7 +8,7 @@ const apiAccessMiddleware = (
     const logger = config.getLogger('/middleware/api-token.ts');
     logger.info('Enabling api-token middleware');
 
-    if(!config.authentication.enableApiToken) {
+    if (!config.authentication.enableApiToken) {
         return (req, res, next) => next();
     }
 

src/lib/middleware/no-authentication.js

@@ -1,10 +1,15 @@
 'use strict';
 
+const { ADMIN } = require('../permissions');
 const User = require('../user');
 
 function noneAuthentication(basePath = '', app) {
     app.use(`${basePath}/api/admin/`, (req, res, next) => {
-        req.user = new User({ username: 'unknown' });
+        req.user = new User({
+            username: 'unknown',
+            isAPI: true,
+            permissions: [ADMIN],
+        });
         next();
     });
 }

src/lib/middleware/permission-checker.js

@@ -1,39 +0,0 @@
-'use strict';
-
-const NoAccessError = require('../error/no-access-error');
-const { ADMIN } = require('../permissions');
-const { isRbacEnabled } = require('../util/feature-enabled');
-
-module.exports = function(config, permission) {
-    if (!permission) {
-        return (req, res, next) => next();
-    }
-    if (isRbacEnabled(config)) {
-        return async (req, res, next) => {
-            if (await req.checkRbac(permission)) {
-                return next();
-            }
-            return res
-                .status(403)
-                .json(new NoAccessError(permission))
-                .end();
-        };
-    }
-    if (!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 NoAccessError(permission))
-            .end();
-    };
-};

src/lib/middleware/permission-checker.test.js

@@ -1,135 +0,0 @@
-'use strict';
-
-const test = require('ava');
-const supertest = require('supertest');
-const sinon = require('sinon');
-const { EventEmitter } = require('events');
-const store = require('../../test/fixtures/store');
-const checkPermission = require('./permission-checker');
-const getApp = require('../app');
-const getLogger = require('../../test/fixtures/no-logger');
-const { CREATE_PROJECT } = require('../permissions');
-
-const eventBus = new EventEmitter();
-
-function getSetup(preRouterHook) {
-    const base = `/random${Math.round(Math.random() * 1000)}`;
-    const stores = store.createStores();
-    const app = getApp({
-        baseUriPath: base,
-        stores,
-        eventBus,
-        getLogger,
-        preRouterHook(_app) {
-            preRouterHook(_app);
-
-            _app.get(
-                `${base}/protectedResource`,
-                checkPermission({ extendedPermissions: true }, 'READ'),
-                (req, res) => {
-                    res.status(200)
-                        .json({ message: 'OK' })
-                        .end();
-                },
-            );
-        },
-    });
-
-    return {
-        base,
-        request: supertest(app),
-    };
-}
-
-test('should return 403 when missing permission', t => {
-    t.plan(0);
-    const { base, request } = getSetup(() => {});
-
-    return request.get(`${base}/protectedResource`).expect(403);
-});
-
-test('should allow access with correct permissions', t => {
-    const { base, request } = getSetup(app => {
-        app.use((req, res, next) => {
-            req.user = { email: 'some@email.com', permissions: ['READ'] };
-            next();
-        });
-    });
-
-    return request
-        .get(`${base}/protectedResource`)
-        .expect(200)
-        .expect(res => {
-            t.is(res.body.message, 'OK');
-        });
-});
-
-test('should allow access with admin permissions', t => {
-    const { base, request } = getSetup(app => {
-        app.use((req, res, next) => {
-            req.user = { email: 'some@email.com', permissions: ['ADMIN'] };
-            next();
-        });
-    });
-
-    return request
-        .get(`${base}/protectedResource`)
-        .expect(200)
-        .expect(res => {
-            t.is(res.body.message, 'OK');
-        });
-});
-
-test('should call checkPermission if defined', async t => {
-    const config = {
-        experimental: {
-            rbac: true,
-        },
-    };
-
-    const func = checkPermission(config, CREATE_PROJECT);
-
-    const cb = sinon.fake();
-    const req = {
-        checkRbac: sinon.fake.returns(Promise.resolve(true)),
-    };
-
-    func(req, undefined, cb);
-
-    t.true(req.checkRbac.calledOnce);
-    t.is(req.checkRbac.firstArg, CREATE_PROJECT);
-});
-
-test('should call checkPermission if defined and give 403 response', async t => {
-    const config = {
-        experimental: {
-            rbac: true,
-        },
-    };
-
-    const func = checkPermission(config, CREATE_PROJECT);
-
-    const cb = sinon.fake();
-    const req = {
-        checkRbac: sinon.fake.returns(Promise.resolve(false)),
-    };
-
-    const fakeJson = sinon.fake.returns({
-        end: sinon.fake(),
-    });
-
-    const fakeStatus = sinon.fake.returns({
-        json: fakeJson,
-    });
-
-    const res = {
-        status: fakeStatus,
-    };
-
-    await func(req, res, cb);
-
-    t.true(req.checkRbac.calledOnce);
-    t.is(req.checkRbac.firstArg, CREATE_PROJECT);
-    t.false(cb.called);
-    t.is(fakeStatus.firstArg, 403);
-});

src/lib/middleware/rbac-middleware.test.ts

@@ -24,21 +24,7 @@ test.beforeEach(() => {
     };
 });
 
-test('should be disabled if rbac is disabled', t => {
+test('should add checkRbac to request', t => {
-    const accessService = {};
-
-    const func = rbacMiddleware(config, { accessService });
-
-    const cb = sinon.fake();
-
-    func(undefined, undefined, cb);
-
-    t.true(cb.calledOnce);
-});
-
-test('should add checkRbac to request if enabled', t => {
-    config.experimental.rbac = true;
-
     const accessService = {};
 
     const func = rbacMiddleware(config, { accessService });

src/lib/middleware/rbac-middleware.ts

@@ -6,13 +6,7 @@ import {
     ADMIN,
 } from '../permissions';
 
-import { isRbacEnabled } from '../util/feature-enabled';
-
 const rbacMiddleware = (config: any, { accessService }: any): any => {
-    if (!isRbacEnabled(config)) {
-        return (req, res, next) => next();
-    }
-
     const logger = config.getLogger('/middleware/rbac-middleware.js');
     logger.info('Enabling RBAC');
 
@@ -45,7 +39,7 @@ const rbacMiddleware = (config: any, { accessService }: any): any => {
                 const { featureName } = params;
                 projectId = await featureToggleStore.getProjectId(featureName);
             } else if (permission === CREATE_FEATURE) {
-                projectId = req.body.project;
+                projectId = req.body.project || 'default';
             }
 
             return accessService.hasPermission(user, permission, projectId);

src/lib/options.js

@@ -67,7 +67,6 @@ function defaultOptions() {
         unleashUrl: process.env.UNLEASH_URL || 'http://localhost:4242',
         serverMetrics: true,
         enableLegacyRoutes: false, // deprecated. Remove in v4,
-        extendedPermissions: false, // deprecated. Remove in v4,
         publicFolder,
         versionCheck: {
             url:
@@ -101,7 +100,6 @@ function defaultOptions() {
                 enabled: process.env.CLIENT_FEATURE_MEMOIZE || false,
                 maxAge: process.env.CLIENT_FEATURE_MAXAGE || 1000,
             },
-            rbac: false,
         },
         email: {
             host: process.env.EMAIL_HOST,

src/lib/routes/admin-api/api-token-controller.ts

@@ -8,11 +8,10 @@ import {
     UPDATE_API_TOKEN,
 } from '../../permissions';
 import { ApiTokenService } from '../../services/api-token-service';
-import { Logger, LogProvider } from '../../logger';
+import { Logger } from '../../logger';
 import { ApiTokenType } from '../../db/api-token-store';
 import { AccessService } from '../../services/access-service';
 import { IAuthRequest } from '../unleash-types';
-import { isRbacEnabled } from '../../util/feature-enabled';
 import User from '../../user';
 import { IUnleashConfig } from '../../types/core';
 
@@ -26,18 +25,12 @@ class ApiTokenController extends Controller {
 
     private accessService: AccessService;
 
-    private extendedPermissions: boolean;
-
-    private isRbacEnabled: boolean;
-
     private logger: Logger;
 
     constructor(config: IUnleashConfig, services: IServices) {
         super(config);
         this.apiTokenService = services.apiTokenService;
         this.accessService = services.accessService;
-        this.extendedPermissions = config.extendedPermissions;
-        this.isRbacEnabled = isRbacEnabled(config);
         this.logger = config.getLogger('api-token-controller.js');
 
         this.get('/', this.getAllApiTokens);
@@ -46,21 +39,17 @@ class ApiTokenController extends Controller {
         this.delete('/:token', this.deleteApiToken, DELETE_API_TOKEN);
     }
 
-    private isTokenAdmin(user: User) {
+    private async isTokenAdmin(user: User) {
-        if (this.isRbacEnabled) {
+        if (user.isAPI) {
-            return this.accessService.hasPermission(user, UPDATE_API_TOKEN);
+            return user.permissions.includes(ADMIN);
         }
-        if (this.extendedPermissions) {
+
-            return user.permissions.some(
+        return this.accessService.hasPermission(user, UPDATE_API_TOKEN);
-                t => t === UPDATE_API_TOKEN || t === ADMIN,
-            );
-        }
-        return true;
     }
 
     async getAllApiTokens(req: IAuthRequest, res: Response): Promise<void> {
         const { user } = req;
-        const isAdmin = this.isTokenAdmin(user);
+        const isAdmin = await this.isTokenAdmin(user);
 
         const tokens = await this.apiTokenService.getAllTokens();
 

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

@@ -8,7 +8,6 @@ const permissions = require('../../../test/fixtures/permissions');
 const getLogger = require('../../../test/fixtures/no-logger');
 const getApp = require('../../app');
 const { createServices } = require('../../services');
-const { UPDATE_FEATURE } = require('../../permissions');
 
 const eventBus = new EventEmitter();
 
@@ -20,8 +19,7 @@ function getSetup() {
         baseUriPath: base,
         stores,
         eventBus,
-        extendedPermissions: true,
+        preHook: perms.hook,
-        preRouterHook: perms.hook,
         getLogger,
     };
     const services = createServices(stores, config);
@@ -72,8 +70,7 @@ test('should get archived toggles via admin', t => {
 test('should revive toggle', t => {
     t.plan(0);
     const name = 'name1';
-    const { request, base, archiveStore, perms } = getSetup();
+    const { request, base, archiveStore } = getSetup();
-    perms.withPermissions(UPDATE_FEATURE);
     archiveStore.addArchivedFeature({
         name,
         strategies: [{ name: 'default' }],
@@ -88,14 +85,7 @@ test('should revive toggle', t => {
 test('should create event when reviving toggle', async t => {
     t.plan(6);
     const name = 'name1';
-    const {
+    const { request, base, featureToggleService, eventStore } = getSetup();
-        request,
-        base,
-        featureToggleService,
-        eventStore,
-        perms,
-    } = getSetup();
-    perms.withPermissions(UPDATE_FEATURE);
 
     await featureToggleService.addArchivedFeature({
         name,

src/lib/routes/admin-api/config.test.js

@@ -21,7 +21,6 @@ function getSetup() {
         baseUriPath: base,
         stores,
         eventBus,
-        extendedPermissions: false,
         ui: uiConfig,
         getLogger,
     });

src/lib/routes/admin-api/context.test.js

@@ -6,6 +6,7 @@ const { EventEmitter } = require('events');
 const store = require('../../../test/fixtures/store');
 const { createServices } = require('../../services');
 const getLogger = require('../../../test/fixtures/no-logger');
+const permissions = require('../../../test/fixtures/permissions');
 const getApp = require('../../app');
 
 const eventBus = new EventEmitter();
@@ -13,11 +14,12 @@ const eventBus = new EventEmitter();
 function getSetup() {
     const base = `/random${Math.round(Math.random() * 1000)}`;
     const stores = store.createStores();
+    const perms = permissions();
     const config = {
         baseUriPath: base,
         stores,
         eventBus,
-        extendedPermissions: false,
+        preHook: perms.hook,
         customContextFields: [{ name: 'tenantId' }],
         getLogger,
     };

src/lib/routes/admin-api/email.test.js

@@ -6,6 +6,7 @@ const { EventEmitter } = require('events');
 const store = require('../../../test/fixtures/store');
 const { createServices } = require('../../services');
 const getLogger = require('../../../test/fixtures/no-logger');
+const permissions = require('../../../test/fixtures/permissions');
 const getApp = require('../../app');
 
 const eventBus = new EventEmitter();
@@ -13,11 +14,12 @@ const eventBus = new EventEmitter();
 function getSetup() {
     const base = `/random${Math.round(Math.random() * 1000)}`;
     const stores = store.createStores();
+    const perms = permissions();
     const config = {
         baseUriPath: base,
         stores,
         eventBus,
-        extendedPermissions: false,
+        preHook: perms.hook,
         customContextFields: [{ name: 'tenantId' }],
         getLogger,
     };

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

@@ -8,11 +8,6 @@ const { createServices } = require('../../services');
 const permissions = require('../../../test/fixtures/permissions');
 const getLogger = require('../../../test/fixtures/no-logger');
 const getApp = require('../../app');
-const {
-    UPDATE_FEATURE,
-    CREATE_FEATURE,
-    DELETE_FEATURE,
-} = require('../../permissions');
 
 const eventBus = new EventEmitter();
 
@@ -24,8 +19,7 @@ function getSetup(databaseIsUp = true) {
         baseUriPath: base,
         stores,
         eventBus,
-        extendedPermissions: true,
+        preHook: perms.hook,
-        preRouterHook: perms.hook,
         getLogger,
     };
     const services = createServices(stores, config);
@@ -88,8 +82,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();
+    const { request, base } = getSetup();
-    perms.withPermissions(CREATE_FEATURE);
 
     return request
         .post(`${base}/api/admin/features`)
@@ -100,8 +93,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();
+    const { request, base } = getSetup();
-    perms.withPermissions(CREATE_FEATURE);
 
     return request
         .post(`${base}/api/admin/features/validate`)
@@ -112,8 +104,7 @@ test('should be allowed to use new toggle name', t => {
 
 test('should get unsupported media-type when posting as form-url-encoded', t => {
     t.plan(0);
-    const { request, base, perms } = getSetup();
+    const { request, base } = getSetup();
-    perms.withPermissions(CREATE_FEATURE);
 
     return request
         .post(`${base}/api/admin/features`)
@@ -126,8 +117,7 @@ test('should get unsupported media-type when posting as form-url-encoded', t =>
 
 test('should be allowed to have variants="null"', t => {
     t.plan(0);
-    const { request, base, perms } = getSetup();
+    const { request, base } = getSetup();
-    perms.withPermissions(CREATE_FEATURE);
 
     return request
         .post(`${base}/api/admin/features`)
@@ -185,8 +175,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();
+    const { request, featureToggleStore, base } = getSetup();
-    perms.withPermissions(UPDATE_FEATURE);
     featureToggleStore.createFeature({
         name: 'ts',
         strategies: [{ name: 'default' }],
@@ -201,8 +190,7 @@ test('should require at least one strategy when updating a feature toggle', t =>
 
 test('updating a feature toggle also requires application/json as content-type', t => {
     t.plan(0);
-    const { request, featureToggleStore, base, perms } = getSetup();
+    const { request, featureToggleStore, base } = getSetup();
-    perms.withPermissions(UPDATE_FEATURE);
     featureToggleStore.createFeature({
         name: 'ts',
         strategies: [{ name: 'default' }],
@@ -218,8 +206,7 @@ test('updating a feature toggle also requires application/json as content-type',
 
 test('valid feature names should pass validation', t => {
     t.plan(0);
-    const { request, base, perms } = getSetup();
+    const { request, base } = getSetup();
-    perms.withPermissions(CREATE_FEATURE);
 
     const validNames = [
         'com.example',
@@ -247,8 +234,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();
+    const { request, base } = getSetup();
-    perms.withPermissions(CREATE_FEATURE);
 
     const invalidNames = [
         'some example',
@@ -276,8 +262,7 @@ test('invalid feature names should not pass validation', t => {
 // Make sure current UI works. Should align on joi errors in future.
 test('invalid feature names should have error msg', t => {
     t.plan(1);
-    const { request, base, perms } = getSetup();
+    const { request, base } = getSetup();
-    perms.withPermissions(CREATE_FEATURE);
 
     const name = 'ØÆ`';
 
@@ -299,8 +284,7 @@ test('invalid feature names should have error msg', t => {
 
 test('should not allow variants with same name when creating feature flag', t => {
     t.plan(0);
-    const { request, base, perms } = getSetup();
+    const { request, base } = getSetup();
-    perms.withPermissions(CREATE_FEATURE);
 
     return request
         .post(`${base}/api/admin/features`)
@@ -319,8 +303,7 @@ test('should not allow variants with same name when creating feature flag', t =>
 
 test('should not allow variants with same name when updating feature flag', t => {
     t.plan(0);
-    const { request, featureToggleStore, base, perms } = getSetup();
+    const { request, featureToggleStore, base } = getSetup();
-    perms.withPermissions(UPDATE_FEATURE);
 
     featureToggleStore.createFeature({
         name: 'ts',
@@ -340,8 +323,7 @@ test('should not allow variants with same name when updating feature flag', t =>
 
 test('should toggle on', t => {
     t.plan(1);
-    const { request, featureToggleStore, base, perms } = getSetup();
+    const { request, featureToggleStore, base } = getSetup();
-    perms.withPermissions(UPDATE_FEATURE);
 
     featureToggleStore.createFeature({
         name: 'toggle.disabled',
@@ -361,8 +343,7 @@ test('should toggle on', t => {
 
 test('should toggle off', t => {
     t.plan(1);
-    const { request, featureToggleStore, base, perms } = getSetup();
+    const { request, featureToggleStore, base } = getSetup();
-    perms.withPermissions(UPDATE_FEATURE);
 
     featureToggleStore.createFeature({
         name: 'toggle.enabled',
@@ -382,8 +363,7 @@ test('should toggle off', t => {
 
 test('should toggle', t => {
     t.plan(1);
-    const { request, featureToggleStore, base, perms } = getSetup();
+    const { request, featureToggleStore, base } = getSetup();
-    perms.withPermissions(UPDATE_FEATURE);
 
     featureToggleStore.createFeature({
         name: 'toggle.disabled',
@@ -403,8 +383,7 @@ test('should toggle', t => {
 
 test('should be able to add tag for feature', t => {
     t.plan(0);
-    const { request, featureToggleStore, base, perms } = getSetup();
+    const { request, featureToggleStore, base } = getSetup();
-    perms.withPermissions(UPDATE_FEATURE);
     featureToggleStore.createFeature({
         name: 'toggle.disabled',
         enabled: false,
@@ -421,8 +400,7 @@ test('should be able to add tag for feature', t => {
 });
 test('should be able to get tags for feature', t => {
     t.plan(1);
-    const { request, featureToggleStore, base, perms } = getSetup();
+    const { request, featureToggleStore, base } = getSetup();
-    perms.withPermissions(UPDATE_FEATURE);
 
     featureToggleStore.createFeature({
         name: 'toggle.disabled',
@@ -445,8 +423,7 @@ test('should be able to get tags for feature', t => {
 
 test('Invalid tag for feature should be rejected', t => {
     t.plan(1);
-    const { request, featureToggleStore, base, perms } = getSetup();
+    const { request, featureToggleStore, base } = getSetup();
-    perms.withPermissions(UPDATE_FEATURE);
 
     featureToggleStore.createFeature({
         name: 'toggle.disabled',
@@ -469,8 +446,7 @@ test('Invalid tag for feature should be rejected', t => {
 
 test('Should be able to filter on tag', t => {
     t.plan(2);
-    const { request, featureToggleStore, base, perms } = getSetup();
+    const { request, featureToggleStore, base } = getSetup();
-    perms.withPermissions(UPDATE_FEATURE);
 
     featureToggleStore.createFeature({
         name: 'toggle.tagged',
@@ -499,8 +475,7 @@ test('Should be able to filter on tag', t => {
 
 test('Should be able to filter on name prefix', t => {
     t.plan(3);
-    const { request, featureToggleStore, base, perms } = getSetup();
+    const { request, featureToggleStore, base } = getSetup();
-    perms.withPermissions(UPDATE_FEATURE);
 
     featureToggleStore.createFeature({
         name: 'a_team.toggle',
@@ -531,8 +506,7 @@ test('Should be able to filter on name prefix', t => {
 
 test('Should be able to filter on project', t => {
     t.plan(3);
-    const { request, featureToggleStore, base, perms } = getSetup();
+    const { request, featureToggleStore, base } = getSetup();
-    perms.withPermissions(UPDATE_FEATURE);
 
     featureToggleStore.createFeature({
         name: 'a_team.toggle',
@@ -564,8 +538,7 @@ test('Should be able to filter on project', t => {
 });
 
 test('Tags should be included in archive events', async t => {
-    const { request, eventStore, featureToggleStore, base, perms } = getSetup();
+    const { request, eventStore, featureToggleStore, base } = getSetup();
-    perms.withPermissions(UPDATE_FEATURE, DELETE_FEATURE);
 
     featureToggleStore.createFeature({
         name: 'a_team.toggle',
@@ -587,8 +560,7 @@ test('Tags should be included in archive events', async t => {
 });
 
 test('Tags should be included in updated events', async t => {
-    const { request, eventStore, featureToggleStore, base, perms } = getSetup();
+    const { request, eventStore, featureToggleStore, base } = getSetup();
-    perms.withPermissions(UPDATE_FEATURE, DELETE_FEATURE);
 
     featureToggleStore.createFeature({
         name: 'a_team.toggle',
@@ -625,8 +597,7 @@ test('Trying to get features while database is down should yield 500', t => {
 test('should mark toggle as stale', t => {
     t.plan(1);
     const toggleName = 'toggle-stale';
-    const { request, featureToggleStore, base, perms } = getSetup();
+    const { request, featureToggleStore, base } = getSetup();
-    perms.withPermissions(UPDATE_FEATURE, DELETE_FEATURE);
     featureToggleStore.createFeature({
         name: toggleName,
         strategies: [{ name: 'default' }],
@@ -644,8 +615,7 @@ test('should mark toggle as stale', t => {
 test('should mark toggle as NOT stale', t => {
     t.plan(1);
     const toggleName = 'toggle-stale';
-    const { request, featureToggleStore, base, perms } = getSetup();
+    const { request, featureToggleStore, base } = getSetup();
-    perms.withPermissions(UPDATE_FEATURE, DELETE_FEATURE);
     featureToggleStore.createFeature({
         name: toggleName,
         strategies: [{ name: 'default' }],

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

@@ -7,7 +7,6 @@ const store = require('../../../test/fixtures/store');
 const permissions = require('../../../test/fixtures/permissions');
 const getLogger = require('../../../test/fixtures/no-logger');
 const getApp = require('../../app');
-const { UPDATE_APPLICATION } = require('../../permissions');
 const { createServices } = require('../../services');
 
 const eventBus = new EventEmitter();
@@ -18,7 +17,6 @@ function getSetup() {
     const config = {
         baseUriPath: '',
         eventBus,
-        extendedPermissions: true,
         preRouterHook: perms.hook,
         getLogger,
     };
@@ -135,9 +133,8 @@ test('should return applications', t => {
 
 test('should store application', t => {
     t.plan(0);
-    const { request, perms } = getSetup();
+    const { request } = getSetup();
     const appName = '123!23';
-    perms.withPermissions(UPDATE_APPLICATION);
 
     return request
         .post(`/api/admin/metrics/applications/${appName}`)
@@ -147,9 +144,8 @@ test('should store application', t => {
 
 test('should store application details wihtout strategies', t => {
     t.plan(0);
-    const { request, perms } = getSetup();
+    const { request } = getSetup();
     const appName = '123!23';
-    perms.withPermissions(UPDATE_APPLICATION);
 
     return request
         .post(`/api/admin/metrics/applications/${appName}`)
@@ -159,9 +155,8 @@ test('should store application details wihtout strategies', t => {
 
 test('should accept a delete call to unknown application', t => {
     t.plan(0);
-    const { request, perms } = getSetup();
+    const { request } = getSetup();
     const appName = 'unknown';
-    perms.withPermissions(UPDATE_APPLICATION);
 
     return request
         .delete(`/api/admin/metrics/applications/${appName}`)
@@ -170,10 +165,9 @@ test('should accept a delete call to unknown application', t => {
 
 test('should delete application', t => {
     t.plan(0);
-    const { request, stores, perms } = getSetup();
+    const { request, stores } = getSetup();
     const appName = 'deletable-test';
 
-    perms.withPermissions(UPDATE_APPLICATION);
     stores.clientApplicationsStore.upsert({ appName });
 
     return request

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

@@ -8,11 +8,6 @@ const permissions = require('../../../test/fixtures/permissions');
 const getLogger = require('../../../test/fixtures/no-logger');
 const getApp = require('../../app');
 const { createServices } = require('../../services');
-const {
-    DELETE_STRATEGY,
-    CREATE_STRATEGY,
-    UPDATE_STRATEGY,
-} = require('../../permissions');
 
 const eventBus = new EventEmitter();
 
@@ -25,7 +20,6 @@ function getSetup(databaseIsUp = true) {
         stores,
         eventBus,
         getLogger,
-        extendedPermissions: true,
         preRouterHook: perms.hook,
     };
     const services = createServices(stores, config);
@@ -58,8 +52,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();
+    const { request, base } = getSetup();
-    perms.withPermissions(CREATE_STRATEGY);
 
     return request
         .post(`${base}/api/admin/strategies`)
@@ -72,8 +65,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();
+    const { request, base } = getSetup();
-    perms.withPermissions(CREATE_STRATEGY);
 
     return request
         .post(`${base}/api/admin/strategies`)
@@ -89,8 +81,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();
+    const { request, base } = getSetup();
-    perms.withPermissions(CREATE_STRATEGY);
 
     return request
         .post(`${base}/api/admin/strategies`)
@@ -100,8 +91,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();
+    const { request, base, strategyStore } = getSetup();
-    perms.withPermissions(CREATE_STRATEGY);
     strategyStore.createStrategy({ name: 'Testing', parameters: [] });
 
     return request
@@ -113,8 +103,7 @@ test('not be possible to override name', t => {
 test('update strategy', t => {
     t.plan(0);
     const name = 'AnotherStrat';
-    const { request, base, strategyStore, perms } = getSetup();
+    const { request, base, strategyStore } = getSetup();
-    perms.withPermissions(UPDATE_STRATEGY);
     strategyStore.createStrategy({ name, parameters: [] });
 
     return request
@@ -126,8 +115,7 @@ test('update strategy', t => {
 test('not update unknown strategy', t => {
     t.plan(0);
     const name = 'UnknownStrat';
-    const { request, base, perms } = getSetup();
+    const { request, base } = getSetup();
-    perms.withPermissions(UPDATE_STRATEGY);
 
     return request
         .put(`${base}/api/admin/strategies/${name}`)
@@ -138,8 +126,7 @@ test('not update unknown strategy', t => {
 test('validate format when updating strategy', t => {
     t.plan(0);
     const name = 'AnotherStrat';
-    const { request, base, strategyStore, perms } = getSetup();
+    const { request, base, strategyStore } = getSetup();
-    perms.withPermissions(UPDATE_STRATEGY);
     strategyStore.createStrategy({ name, parameters: [] });
 
     return request
@@ -152,8 +139,7 @@ test('editable=false will stop delete request', t => {
     getLogger.setMuteError(true);
     t.plan(0);
     const name = 'default';
-    const { request, base, perms } = getSetup();
+    const { request, base } = getSetup();
-    perms.withPermissions(DELETE_STRATEGY);
 
     return request.delete(`${base}/api/admin/strategies/${name}`).expect(500);
 });
@@ -162,8 +148,7 @@ test('editable=false will stop edit request', t => {
     getLogger.setMuteError(true);
     t.plan(0);
     const name = 'default';
-    const { request, base, perms } = getSetup();
+    const { request, base } = getSetup();
-    perms.withPermissions(UPDATE_STRATEGY);
 
     return request
         .put(`${base}/api/admin/strategies/${name}`)
@@ -174,8 +159,7 @@ test('editable=false will stop edit request', t => {
 test('editable=true will allow delete request', t => {
     t.plan(0);
     const name = 'deleteStrat';
-    const { request, base, strategyStore, perms } = getSetup();
+    const { request, base, strategyStore } = getSetup();
-    perms.withPermissions(DELETE_STRATEGY);
     strategyStore.createStrategy({ name, parameters: [] });
 
     return request
@@ -187,8 +171,7 @@ test('editable=true will allow delete request', t => {
 test('editable=true will allow edit request', t => {
     t.plan(0);
     const name = 'editStrat';
-    const { request, base, strategyStore, perms } = getSetup();
+    const { request, base, strategyStore } = getSetup();
-    perms.withPermissions(UPDATE_STRATEGY);
     strategyStore.createStrategy({ name, parameters: [] });
 
     return request
@@ -200,8 +183,7 @@ test('editable=true will allow edit request', t => {
 test('deprecating a strategy works', async t => {
     t.plan(1);
     const name = 'editStrat';
-    const { request, base, strategyStore, perms } = getSetup();
+    const { request, base, strategyStore } = getSetup();
-    perms.withPermissions(UPDATE_STRATEGY);
     strategyStore.createStrategy({ name, parameters: [] });
 
     await request
@@ -217,8 +199,7 @@ test('deprecating a strategy works', async t => {
 
 test('deprecating a non-existent strategy yields 404', t => {
     t.plan(0);
-    const { request, base, perms } = getSetup();
+    const { request, base } = getSetup();
-    perms.withPermissions(UPDATE_STRATEGY);
     return request
         .post(`${base}/api/admin/strategies/non-existent-strategy/deprecate`)
         .set('Content-Type', 'application/json')
@@ -228,8 +209,7 @@ test('deprecating a non-existent strategy yields 404', t => {
 test('reactivating a strategy works', async t => {
     t.plan(1);
     const name = 'editStrat';
-    const { request, base, strategyStore, perms } = getSetup();
+    const { request, base, strategyStore } = getSetup();
-    perms.withPermissions(UPDATE_STRATEGY);
     strategyStore.createStrategy({ name, parameters: [] });
 
     await request
@@ -245,8 +225,7 @@ test('reactivating a strategy works', async t => {
 
 test('reactivating a non-existent strategy yields 404', t => {
     t.plan(0);
-    const { request, base, perms } = getSetup();
+    const { request, base } = getSetup();
-    perms.withPermissions(UPDATE_STRATEGY);
     return request
         .post(`${base}/api/admin/strategies/non-existent-strategy/reactivate`)
         .set('Content-Type', 'application/json')
@@ -254,8 +233,7 @@ test('reactivating a non-existent strategy yields 404', t => {
 });
 test("deprecating 'default' strategy will yield 403", t => {
     t.plan(0);
-    const { request, base, perms } = getSetup();
+    const { request, base } = getSetup();
-    perms.withPermissions(UPDATE_STRATEGY);
     return request
         .post(`${base}/api/admin/strategies/default/deprecate`)
         .set('Content-Type', 'application/json')

src/lib/routes/admin-api/tag.test.js

@@ -8,7 +8,6 @@ const permissions = require('../../../test/fixtures/permissions');
 const getLogger = require('../../../test/fixtures/no-logger');
 const getApp = require('../../app');
 const { createServices } = require('../../services');
-const { UPDATE_FEATURE } = require('../../permissions');
 
 const eventBus = new EventEmitter();
 
@@ -20,7 +19,6 @@ function getSetup(databaseIsUp = true) {
         baseUriPath: base,
         stores,
         eventBus,
-        extendedPermissions: true,
         preRouterHook: perms.hook,
         getLogger,
     };
@@ -85,8 +83,7 @@ test('trying to get non-existing tag by name and type should not be found', t =>
 });
 test('should be able to delete a tag', t => {
     t.plan(0);
-    const { request, base, tagStore, perms } = getSetup();
+    const { request, base, tagStore } = getSetup();
-    perms.withPermissions(UPDATE_FEATURE);
     tagStore.createTag({ type: 'simple', value: 'TeamRed' });
     return request
         .delete(`${base}/api/admin/tags/simple/TeamGreen`)

src/lib/routes/admin-api/user.js

@@ -13,11 +13,7 @@ class UserController extends Controller {
     getUser(req, res) {
         if (req.user) {
             const user = { ...req.user };
-            if (!this.config.extendedPermissions) {
+            delete user.permissions; // TODO: remove
-                delete user.permissions;
-            } else if (!Array.isArray(user.permissions)) {
-                user.permissions = [];
-            }
             return res
                 .status(200)
                 .json(user)

src/lib/routes/controller.js

@@ -1,8 +1,24 @@
 'use strict';
 
 const { Router } = require('express');
-const checkPermission = require('../middleware/permission-checker');
+const NoAccessError = require('../error/no-access-error');
 const requireContentType = require('../middleware/content_type_checker');
+
+const checkPermission = permission => {
+    return async (req, res, next) => {
+        if (!permission) {
+            return next();
+        }
+        if (await req.checkRbac(permission)) {
+            return next();
+        }
+        return res
+            .status(403)
+            .json(new NoAccessError(permission))
+            .end();
+    };
+};
+
 /**
  * Base class for Controllers to standardize binding to express Router.
  */
@@ -14,17 +30,13 @@ class Controller {
     }
 
     get(path, handler, permission) {
-        this.app.get(
+        this.app.get(path, checkPermission(permission), handler.bind(this));
-            path,
-            checkPermission(this.config, permission),
-            handler.bind(this),
-        );
     }
 
     post(path, handler, permission, ...acceptedContentTypes) {
         this.app.post(
             path,
-            checkPermission(this.config, permission),
+            checkPermission(permission),
             requireContentType(...acceptedContentTypes),
             handler.bind(this),
         );
@@ -33,24 +45,20 @@ class Controller {
     put(path, handler, permission, ...acceptedContentTypes) {
         this.app.put(
             path,
-            checkPermission(this.config, permission),
+            checkPermission(permission),
             requireContentType(...acceptedContentTypes),
             handler.bind(this),
         );
     }
 
     delete(path, handler, permission) {
-        this.app.delete(
+        this.app.delete(path, checkPermission(permission), handler.bind(this));
-            path,
-            checkPermission(this.config, permission),
-            handler.bind(this),
-        );
     }
 
     fileupload(path, filehandler, handler, permission) {
         this.app.post(
             path,
-            checkPermission(this.config, permission),
+            checkPermission(permission),
             filehandler,
             handler.bind(this),
         );

src/lib/services/access-service.ts

@@ -97,7 +97,7 @@ export class AccessService {
      * @param projectId 
      */
     async hasPermission(user: User, permission: string, projectId?: string): Promise<boolean> {
-        this.logger.info(`Checking permission=${permission}, userId=${user.id} projectId=${projectId}`)
+        this.logger.info(`Checking permission=${permission}, userId=${user.id} projectId=${projectId}`);
 
         const permissions = await this.store.getPermissionsForUser(user.id);
 

src/lib/services/project-service.ts

@@ -1,6 +1,5 @@
 import User from '../user';
 import { AccessService, RoleName } from './access-service';
-import { isRbacEnabled } from '../util/feature-enabled';
 
 const NameExistsError = require('../error/name-exists-error');
 const InvalidOperationError = require('../error/invalid-operation-error');
@@ -30,8 +29,6 @@ class ProjectService {
 
     private logger: any;
 
-    private rbacEnabled: boolean;
-
     constructor(
         { projectStore, eventStore, featureToggleStore },
         config: any,
@@ -42,7 +39,6 @@ class ProjectService {
         this.eventStore = eventStore;
         this.featureToggleStore = featureToggleStore;
         this.logger = config.getLogger('services/project-service.js');
-        this.rbacEnabled = isRbacEnabled(config);
     }
 
     async getProjects() {
@@ -59,9 +55,7 @@ class ProjectService {
 
         await this.projectStore.create(data);
 
-        if (this.rbacEnabled) {
+        await this.accessService.createDefaultProjectRoles(user, data.id);
-            await this.accessService.createDefaultProjectRoles(user, data.id);
-        }
 
         await this.eventStore.store({
             type: eventType.PROJECT_CREATED,
@@ -111,9 +105,7 @@ class ProjectService {
             data: { id },
         });
 
-        if (this.rbacEnabled) {
+        this.accessService.removeDefaultProjectRoles(user, id);
-            this.accessService.removeDefaultProjectRoles(user, id);
-        }
     }
 
     async validateId(id: string): Promise<boolean> {

src/lib/types/core.ts

@@ -7,7 +7,6 @@ interface IExperimentalFlags {
 export interface IUnleashConfig {
     getLogger: LogProvider;
     baseUriPath: string;
-    extendedPermissions?: boolean;
     experimental?: IExperimentalFlags;
     authentication: {
         enableApiToken: boolean;

src/lib/util/feature-enabled.ts

@@ -1,7 +0,0 @@
-import { IUnleashConfig } from '../types/core';
-
-export const isRbacEnabled = (config: IUnleashConfig): boolean => {
-    return config && config.experimental && config.experimental.rbac;
-};
-
-module.exports = { isRbacEnabled };

src/test/e2e/api/admin/api-token.e2e.test.ts

@@ -5,6 +5,7 @@ import getLogger from '../../../fixtures/no-logger';
 import { ApiTokenType, IApiToken } from '../../../../lib/db/api-token-store';
 import User from '../../../../lib/user';
 import { CREATE_API_TOKEN, CREATE_FEATURE } from '../../../../lib/permissions';
+import { RoleName } from '../../../../lib/services/access-service';
 
 let stores;
 let db;
@@ -184,16 +185,22 @@ test.serial('removes api token', async t => {
 
 test.serial('none-admins should only get client tokens', async t => {
     t.plan(2);
-    const user = new User({ email: 'custom-user@mail.com', permissions: [] });
 
-    const preHook = app => {
+    const email = 'custom-user@mail.com';
-        app.use('/api/', (req, res, next) => {
+
+    const preHook = (app, config, { userService, accessService }) => {
+        app.use('/api/admin/', async (req, res, next) => {
+            const role = await accessService.getRootRole(RoleName.REGULAR);
+            const user = await userService.createUser({
+                email,
+                rootRole: role.id,
+            });
             req.user = user;
             next();
         });
     };
 
-    const request = await setupAppWithCustomAuth(stores, preHook, true);
+    const request = await setupAppWithCustomAuth(stores, preHook);
 
     await stores.apiTokenStore.insert({
         username: 'test',
@@ -219,19 +226,21 @@ test.serial('none-admins should only get client tokens', async t => {
 
 test.serial('Only token-admins should be allowed to create token', async t => {
     t.plan(0);
-    const user = new User({
-        email: 'custom-user@mail.com',
-        permissions: [CREATE_FEATURE],
-    });
 
-    const preHook = app => {
+    const email = 'custom-user2@mail.com';
-        app.use('/api/', (req, res, next) => {
+
-            req.user = user;
+    const preHook = (app, config, { userService, accessService }) => {
+        app.use('/api/admin/', async (req, res, next) => {
+            const role = await accessService.getRootRole(RoleName.REGULAR);
+            req.user = await userService.createUser({
+                email,
+                rootRole: role.id,
+            });
             next();
         });
     };
 
-    const request = await setupAppWithCustomAuth(stores, preHook, true);
+    const request = await setupAppWithCustomAuth(stores, preHook);
 
     return request
         .post('/api/admin/api-tokens')
@@ -245,19 +254,20 @@ test.serial('Only token-admins should be allowed to create token', async t => {
 
 test.serial('Token-admin should be allowed to create token', async t => {
     t.plan(0);
-    const user = new User({
+    const email = 'custom-user3@mail.com';
-        email: 'custom-user@mail.com',
-        permissions: [CREATE_API_TOKEN],
-    });
 
-    const preHook = app => {
+    const preHook = (app, config, { userService, accessService }) => {
-        app.use('/api/', (req, res, next) => {
+        app.use('/api/admin/', async (req, res, next) => {
-            req.user = user;
+            const role = await accessService.getRootRole(RoleName.ADMIN);
+            req.user = await userService.createUser({
+                email,
+                rootRole: role.id,
+            });
             next();
         });
     };
 
-    const request = await setupAppWithCustomAuth(stores, preHook, true);
+    const request = await setupAppWithCustomAuth(stores, preHook);
 
     return request
         .post('/api/admin/api-tokens')

src/test/e2e/api/admin/feature.custom-auth.e2e.test.js

@@ -3,7 +3,6 @@
 const test = require('ava');
 const { setupAppWithCustomAuth } = require('../../helpers/test-helper');
 const AuthenticationRequired = require('../../../../lib/authentication-required');
-const User = require('../../../../lib/user');
 
 const dbInit = require('../../helpers/database-init');
 const getLogger = require('../../../fixtures/no-logger');
@@ -42,11 +41,11 @@ test.serial('should require authenticated user', async t => {
 
 test.serial('creates new feature toggle with createdBy', async t => {
     t.plan(1);
-    const user = new User({ email: 'custom-user@mail.com' });
+    const email = 'custom-user@mail.com';
 
-    const preHook = app => {
+    const preHook = (app, config, { userService }) => {
-        app.use('/api/admin/', (req, res, next) => {
+        app.use('/api/admin/', async (req, res, next) => {
-            req.user = user;
+            req.user = await userService.loginUserWithoutPassword(email, true);
             next();
         });
     };
@@ -63,6 +62,6 @@ test.serial('creates new feature toggle with createdBy', async t => {
         .expect(201);
 
     await request.get('/api/admin/events').expect(res => {
-        t.is(res.body.events[0].createdBy, user.email);
+        t.is(res.body.events[0].createdBy, email);
     });
 });

src/test/e2e/helpers/test-helper.js

@@ -11,18 +11,12 @@ const { createServices } = require('../../../lib/services');
 
 const eventBus = new EventEmitter();
 
-function createApp(
+function createApp(stores, adminAuthentication = 'none', preHook) {
-    stores,
-    adminAuthentication = 'none',
-    preHook,
-    extendedPermissions = false,
-) {
     const config = {
         stores,
         eventBus,
         preHook,
         adminAuthentication,
-        extendedPermissions,
         secret: 'super-secret',
         session: {
             db: true,
@@ -49,8 +43,8 @@ module.exports = {
         return supertest.agent(app);
     },
 
-    async setupAppWithCustomAuth(stores, preHook, extendedPermissions) {
+    async setupAppWithCustomAuth(stores, preHook) {
-        const app = createApp(stores, 'custom', preHook, extendedPermissions);
+        const app = createApp(stores, 'custom', preHook);
         return supertest.agent(app);
     },
 };

src/test/fixtures/permissions.js

@@ -1,17 +1,18 @@
 'use strict';
 
 module.exports = () => {
-    let _perms = [];
+    const _perms = ['ADMIN'];
     return {
         hook(app) {
             app.use((req, res, next) => {
-                if (req.user) req.user.permissions = _perms;
+                req.user = {
-                else req.user = { email: 'unknown', permissions: _perms };
+                    isAPI: true,
+                    id: 1,
+                    email: 'unknown',
+                    permissions: _perms,
+                };
                 next();
             });
         },
-        withPermissions(...perms) {
-            _perms = perms;
-        },
     };
 };