From e1fbe9d01393e9256c6008a59bbac79dab69fd58 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ivar=20Conradi=20=C3=98sthus?= Date: Thu, 11 Mar 2021 22:51:58 +0100 Subject: [PATCH] feat: Default roles and RBAC permission checker. (#735) This PR Introduces first steps towards RBAC according to our specifications. Rbac will assume users to exist in the Unleash user table with a unique id. This is required to make correct mappings between users and roles. --- .eslintrc | 3 +- package.json | 2 +- src/lib/app.ts | 7 +- src/lib/db/access-store.ts | 163 +++++++ src/lib/db/feature-toggle-store.js | 8 + src/lib/db/index.js | 4 + src/lib/db/project-store.js | 7 +- src/lib/db/user-store.js | 20 + src/lib/error/no-access-error.ts | 26 ++ src/lib/middleware/permission-checker.js | 26 +- src/lib/middleware/permission-checker.test.js | 56 +++ src/lib/middleware/rbac-middleware.test.ts | 257 +++++++++++ src/lib/middleware/rbac-middleware.ts | 58 +++ src/lib/middleware/simple-authentication.js | 4 +- src/lib/missing-permission.js | 8 - src/lib/options.js | 1 + src/lib/permissions.js | 4 + src/lib/routes/admin-api/feature.js | 2 +- src/lib/routes/admin-api/util.js | 5 + src/lib/routes/index.ts | 5 +- src/lib/services/access-service.ts | 223 ++++++++++ src/lib/services/index.js | 11 +- src/lib/services/project-service.js | 93 ---- src/lib/services/project-service.ts | 197 ++++++++ src/lib/user.js | 42 -- src/lib/user.test.js | 2 +- src/lib/user.ts | 79 ++++ src/lib/util/feature-enabled.ts | 13 + src/migrations/.eslintrc | 16 + src/migrations/20210217195834-rbac-tables.js | 81 ++++ src/test/e2e/helpers/database-init.js | 1 + .../e2e/services/access-service.e2e.test.js | 420 ++++++++++++++++++ .../e2e/services/project-service.e2e.test.js | 236 +++++++++- 33 files changed, 1900 insertions(+), 180 deletions(-) create mode 100644 src/lib/db/access-store.ts create mode 100644 src/lib/error/no-access-error.ts create mode 100644 src/lib/middleware/rbac-middleware.test.ts create mode 100644 src/lib/middleware/rbac-middleware.ts delete mode 100644 src/lib/missing-permission.js create mode 100644 src/lib/services/access-service.ts delete mode 100644 src/lib/services/project-service.js create mode 100644 src/lib/services/project-service.ts delete mode 100644 src/lib/user.js create mode 100644 src/lib/user.ts create mode 100644 src/lib/util/feature-enabled.ts create mode 100644 src/migrations/.eslintrc create mode 100644 src/migrations/20210217195834-rbac-tables.js create mode 100644 src/test/e2e/services/access-service.e2e.test.js diff --git a/.eslintrc b/.eslintrc index 1d12ab0bf5..9c473e41ce 100644 --- a/.eslintrc +++ b/.eslintrc @@ -16,9 +16,10 @@ "root": true, "rules": { "@typescript-eslint/no-var-requires": 0, - "@typescript-eslint/indent": 0, + "@typescript-eslint/indent": ["error", 4], "@typescript-eslint/naming-convention": 0, "@typescript-eslint/space-before-function-paren": 0, + "import/prefer-default-export": 0, "import/no-unresolved": 0, "class-methods-use-this": [0], "prettier/prettier": ["error"], diff --git a/package.json b/package.json index 7fa8e9f728..6592dd660d 100644 --- a/package.json +++ b/package.json @@ -164,7 +164,7 @@ "trailingComma": "all", "overrides": [ { - "files": "*.{json,yaml,yml,md,ts}", + "files": "*.{json,yaml,yml,md}", "options": { "tabWidth": 2 } diff --git a/src/lib/app.ts b/src/lib/app.ts index 6a8f9eb9c9..ec0425af66 100644 --- a/src/lib/app.ts +++ b/src/lib/app.ts @@ -1,3 +1,6 @@ +import { responseTimeMetrics } from './middleware/response-time-metrics'; +import rbacMiddleware from './middleware/rbac-middleware'; + const express = require('express'); const compression = require('compression'); @@ -7,7 +10,7 @@ const path = require('path'); const errorHandler = require('errorhandler'); const IndexRouter = require('./routes'); const unleashDbSession = require('./middleware/session-db'); -import { responseTimeMetrics } from './middleware/response-time-metrics'; + const requestLogger = require('./middleware/request-logger'); const simpleAuthentication = require('./middleware/simple-authentication'); const noAuthentication = require('./middleware/no-authentication'); @@ -57,6 +60,8 @@ module.exports = function(config, services = {}) { config.preRouterHook(app); } + app.use(baseUriPath, rbacMiddleware(config, services)); + // Setup API routes app.use(`${baseUriPath}/`, new IndexRouter(config, services).router); diff --git a/src/lib/db/access-store.ts b/src/lib/db/access-store.ts new file mode 100644 index 0000000000..6a36a2ff5c --- /dev/null +++ b/src/lib/db/access-store.ts @@ -0,0 +1,163 @@ +import { EventEmitter } from 'events'; +import Knex from 'knex'; +import metricsHelper from '../metrics-helper'; +import { DB_TIME } from '../events'; + +const T = { + ROLE_USER: 'role_user', + ROLES: 'roles', + ROLE_PERMISSION: 'role_permission', +}; + +export interface IUserPermission { + project?: string; + permission: string; +} + +export interface IRole { + id: number; + name: string; + description?: string; + type: string; + project?: string; +} + +export class AccessStore { + private logger: Function; + + private timer: Function; + + private db: Knex; + + constructor(db: Knex, eventBus: EventEmitter, getLogger: Function) { + this.db = db; + this.logger = getLogger('access-store.js'); + this.timer = (action: string) => + metricsHelper.wrapTimer(eventBus, DB_TIME, { + store: 'access-store', + action, + }); + } + + async getPermissionsForUser(userId: Number): Promise { + const stopTimer = this.timer('getPermissionsForUser'); + const rows = await this.db + .select('project', 'permission') + .from(`${T.ROLE_PERMISSION} AS rp`) + .leftJoin(`${T.ROLE_USER} AS ur`, 'ur.role_id', 'rp.role_id') + .where('user_id', '=', userId); + stopTimer(); + return rows; + } + + async getPermissionsForRole(roleId: number): Promise { + const stopTimer = this.timer('getPermissionsForRole'); + const rows = await this.db + .select('project', 'permission') + .from(`${T.ROLE_PERMISSION}`) + .where('role_id', '=', roleId); + stopTimer(); + return rows; + } + + async getRoles(): Promise { + return this.db + .select(['id', 'name', 'type', 'description']) + .from(T.ROLES); + } + + async getRoleWithId(id: number): Promise { + return this.db + .select(['id', 'name', 'type', 'description']) + .where('id', id) + .first() + .from(T.ROLES); + } + + async getRolesForProject(projectId: string): Promise { + return this.db + .select(['id', 'name', 'type', 'project', 'description']) + .from(T.ROLES) + .where('project', projectId) + .andWhere('type', 'project'); + } + + async removeRolesForProject(projectId: string): Promise { + return this.db(T.ROLES) + .where({ + project: projectId, + }) + .delete(); + } + + async getRolesForUserId(userId: number): Promise { + return this.db + .select(['id', 'name', 'type', 'project', 'description']) + .from(T.ROLES) + .innerJoin(`${T.ROLE_USER} as ru`, 'ru.role_id', 'id') + .where('ru.user_id', '=', userId); + } + + async getUserIdsForRole(roleId: number): Promise { + const rows = await this.db + .select(['user_id']) + .from(T.ROLE_USER) + .where('role_id', roleId); + return rows.map(r => r.user_id); + } + + async addUserToRole(userId: number, roleId: number): Promise { + return this.db(T.ROLE_USER).insert({ + user_id: userId, + role_id: roleId, + }); + } + + async removeUserFromRole(userId: number, roleId: number): Promise { + return this.db(T.ROLE_USER) + .where({ + user_id: userId, + role_id: roleId, + }) + .delete(); + } + + async createRole( + name: string, + type: string, + project?: string, + description?: string, + ): Promise { + const [id] = await this.db(T.ROLES) + .insert({ name, description, type, project }) + .returning('id'); + return { id, name, description, type, project }; + } + + async addPermissionsToRole( + role_id: number, + permissions: string[], + projectId?: string, + ): Promise { + const rows = permissions.map(permission => ({ + role_id, + project: projectId, + permission, + })); + return this.db.batchInsert(T.ROLE_PERMISSION, rows); + } + + async removePermissionFromRole( + roleId: number, + permission: string, + projectId?: string, + ): Promise { + return this.db(T.ROLE_PERMISSION) + .where({ + role_id: roleId, + permission, + project: projectId, + }) + .delete(); + } +} diff --git a/src/lib/db/feature-toggle-store.js b/src/lib/db/feature-toggle-store.js index 19f491bc2a..d15c19ba02 100644 --- a/src/lib/db/feature-toggle-store.js +++ b/src/lib/db/feature-toggle-store.js @@ -98,6 +98,14 @@ class FeatureToggleStore { .then(this.rowToFeature); } + async getProjectId(name) { + return this.db + .first(['project']) + .from(TABLE) + .where({ name }) + .then(r => (r ? r.project : undefined)); + } + async hasFeature(name) { return this.db .first('name', 'archived') diff --git a/src/lib/db/index.js b/src/lib/db/index.js index caf941e24c..316efb74fb 100644 --- a/src/lib/db/index.js +++ b/src/lib/db/index.js @@ -1,5 +1,8 @@ 'use strict'; +// eslint-disable-next-line +import { AccessStore } from './access-store'; + const { createDb } = require('./db-pool'); const EventStore = require('./event-store'); const FeatureToggleStore = require('./feature-toggle-store'); @@ -51,5 +54,6 @@ module.exports.createStores = (config, eventBus) => { tagStore: new TagStore(db, eventBus, getLogger), tagTypeStore: new TagTypeStore(db, eventBus, getLogger), addonStore: new AddonStore(db, eventBus, getLogger), + accessStore: new AccessStore(db, eventBus, getLogger), }; }; diff --git a/src/lib/db/project-store.js b/src/lib/db/project-store.js index c18bfd5f93..1f36abf2cf 100644 --- a/src/lib/db/project-store.js +++ b/src/lib/db/project-store.js @@ -51,11 +51,10 @@ class ProjectStore { } async create(project) { - await this.db(TABLE) + const [id] = await this.db(TABLE) .insert(this.fieldToRow(project)) - .catch(err => - this.logger.error('Could not insert project, error: ', err), - ); + .returning('id'); + return { ...project, id }; } async update(data) { diff --git a/src/lib/db/user-store.js b/src/lib/db/user-store.js index 651e3c7e71..fce3a7c2d8 100644 --- a/src/lib/db/user-store.js +++ b/src/lib/db/user-store.js @@ -19,6 +19,8 @@ const USER_COLUMNS = [ 'created_at', ]; +const USER_COLUMNS_PUBLIC = ['id', 'name', 'username', 'email', 'image_url']; + const emptify = value => { if (!value) { return undefined; @@ -105,6 +107,24 @@ class UserStore { return users.map(rowToUser); } + async search(query) { + const users = await this.db + .select(USER_COLUMNS_PUBLIC) + .from(TABLE) + .where('name', 'ILIKE', `%${query}%`) + .orWhere('username', 'ILIKE', `${query}%`) + .orWhere('email', 'ILIKE', `${query}%`); + return users.map(rowToUser); + } + + async getAllWithId(userIdList) { + const users = await this.db + .select(USER_COLUMNS_PUBLIC) + .from(TABLE) + .whereIn('id', userIdList); + return users.map(rowToUser); + } + async get(idQuery) { const row = await this.buildSelectUser(idQuery).first(USER_COLUMNS); return rowToUser(row); diff --git a/src/lib/error/no-access-error.ts b/src/lib/error/no-access-error.ts new file mode 100644 index 0000000000..2034576c55 --- /dev/null +++ b/src/lib/error/no-access-error.ts @@ -0,0 +1,26 @@ +class NoAccessError extends Error { + permission: string; + + name: string; + + message: string; + + constructor(permission: string) { + super(); + Error.captureStackTrace(this, this.constructor); + + this.name = this.constructor.name; + this.permission = permission; + this.message = `You need permission=${permission} to perform this action`; + } + + toJSON(): any { + return { + permission: this.permission, + message: this.message, + }; + } +} + +export default NoAccessError; +module.exports = NoAccessError; diff --git a/src/lib/middleware/permission-checker.js b/src/lib/middleware/permission-checker.js index 9c67a752f7..2aa331189b 100644 --- a/src/lib/middleware/permission-checker.js +++ b/src/lib/middleware/permission-checker.js @@ -1,10 +1,25 @@ 'use strict'; -const MissingPermission = require('../missing-permission'); +const NoAccessError = require('../error/no-access-error'); const { ADMIN } = require('../permissions'); +const { isRbacEnabled } = require('../util/feature-enabled'); module.exports = function(config, permission) { - if (!permission || !config.extendedPermissions) { + 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) => { @@ -18,12 +33,7 @@ module.exports = function(config, permission) { } return res .status(403) - .json( - new MissingPermission({ - permission, - message: `You require ${permission} to perform this action`, - }), - ) + .json(new NoAccessError(permission)) .end(); }; }; diff --git a/src/lib/middleware/permission-checker.test.js b/src/lib/middleware/permission-checker.test.js index 23691bc67e..c9d57f1c2e 100644 --- a/src/lib/middleware/permission-checker.test.js +++ b/src/lib/middleware/permission-checker.test.js @@ -2,11 +2,13 @@ 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(); @@ -77,3 +79,57 @@ test('should allow access with admin permissions', t => { 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); +}); diff --git a/src/lib/middleware/rbac-middleware.test.ts b/src/lib/middleware/rbac-middleware.test.ts new file mode 100644 index 0000000000..d03c2daa37 --- /dev/null +++ b/src/lib/middleware/rbac-middleware.test.ts @@ -0,0 +1,257 @@ +import test from 'ava'; + +import sinon from 'sinon'; + +import rbacMiddleware from './rbac-middleware'; +import getLogger from '../../test/fixtures/no-logger'; +import ffStore from '../../test/fixtures/fake-feature-toggle-store'; +import User from '../user'; +import perms from '../permissions'; + +let config: any; +let featureToggleStore: any; + +test.beforeEach(() => { + featureToggleStore = ffStore(); + config = { + getLogger, + stores: { + featureToggleStore, + }, + experimental: { + rbac: false, + }, + }; +}); + +test('should be disabled if rbac is disabled', 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 }); + + const cb = sinon.fake(); + + const req = sinon.fake(); + + func(req, undefined, cb); + + t.truthy(req.checkRbac); + t.is(typeof req.checkRbac, 'function'); +}); + +test('should give api-user ADMIN permission', async t => { + config.experimental.rbac = true; + + const accessService = {}; + + const func = rbacMiddleware(config, { accessService }); + + const cb = sinon.fake(); + const req: any = { + user: new User({ + username: 'api', + permissions: [perms.ADMIN], + isAPI: true, + }), + }; + + func(req, undefined, cb); + + const hasAccess = await req.checkRbac(perms.ADMIN); + + t.true(hasAccess); +}); + +test('should not give api-user ADMIN permission', async t => { + config.experimental.rbac = true; + + const accessService = {}; + + const func = rbacMiddleware(config, { accessService }); + + const cb = sinon.fake(); + const req: any = { + user: new User({ + username: 'api', + permissions: [perms.CLIENT], + isAPI: true, + }), + }; + + func(req, undefined, cb); + + const hasAccess = await req.checkRbac(perms.ADMIN); + + t.false(hasAccess); +}); + +test('should not allow user to miss userId', async t => { + config.experimental.rbac = true; + + const accessService = {}; + + const func = rbacMiddleware(config, { accessService }); + + const cb = sinon.fake(); + const req: any = { + user: new User({ + username: 'user', + permissions: [perms.ADMIN], + }), + }; + + func(req, undefined, cb); + + const hasAccess = await req.checkRbac(perms.ADMIN); + + t.false(hasAccess); +}); + +test('should return false for missing user', async t => { + config.experimental.rbac = true; + + const accessService = {}; + + const func = rbacMiddleware(config, { accessService }); + + const cb = sinon.fake(); + const req: any = {}; + + func(req, undefined, cb); + + const hasAccess = await req.checkRbac(perms.ADMIN); + + t.false(hasAccess); +}); + +test('should verify permission for root resource', async t => { + config.experimental.rbac = true; + + const accessService = { + hasPermission: sinon.fake(), + }; + + const func = rbacMiddleware(config, { accessService }); + + const cb = sinon.fake(); + const req: any = { + user: new User({ + username: 'user', + id: 1, + }), + params: {}, + }; + + func(req, undefined, cb); + + await req.checkRbac(perms.ADMIN); + + t.true(accessService.hasPermission.calledOnce); + t.is(accessService.hasPermission.firstArg, req.user); + t.is(accessService.hasPermission.args[0][1], perms.ADMIN); + t.is(accessService.hasPermission.args[0][2], undefined); +}); + +test('should lookup projectId from params', async t => { + config.experimental.rbac = true; + + const accessService = { + hasPermission: sinon.fake(), + }; + + const func = rbacMiddleware(config, { accessService }); + + const cb = sinon.fake(); + const req: any = { + user: new User({ + username: 'user', + id: 1, + }), + params: { + projectId: 'some-proj', + }, + }; + + func(req, undefined, cb); + + await req.checkRbac(perms.UPDATE_PROJECT); + + t.is(accessService.hasPermission.args[0][2], req.params.projectId); +}); + +test('should lookup projectId from feature toggle', async t => { + const projectId = 'some-project-33'; + const featureName = 'some-feature-toggle'; + config.experimental.rbac = true; + + const accessService = { + hasPermission: sinon.fake(), + }; + + featureToggleStore.getProjectId = sinon.fake.returns(projectId); + + const func = rbacMiddleware(config, { accessService }); + + const cb = sinon.fake(); + const req: any = { + user: new User({ + username: 'user', + id: 1, + }), + params: { + featureName, + }, + }; + + func(req, undefined, cb); + + await req.checkRbac(perms.UPDATE_FEATURE); + + t.is(accessService.hasPermission.args[0][2], projectId); + t.is(featureToggleStore.getProjectId.firstArg, featureName); +}); + +test('should lookup projectId from data', async t => { + const projectId = 'some-project-33'; + const featureName = 'some-feature-toggle'; + config.experimental.rbac = true; + + const accessService = { + hasPermission: sinon.fake(), + }; + + const func = rbacMiddleware(config, { accessService }); + + const cb = sinon.fake(); + const req: any = { + user: new User({ + username: 'user', + id: 1, + }), + params: {}, + body: { + featureName, + project: projectId, + }, + }; + + func(req, undefined, cb); + + await req.checkRbac(perms.CREATE_FEATURE); + + t.is(accessService.hasPermission.args[0][2], projectId); +}); diff --git a/src/lib/middleware/rbac-middleware.ts b/src/lib/middleware/rbac-middleware.ts new file mode 100644 index 0000000000..355efc33cd --- /dev/null +++ b/src/lib/middleware/rbac-middleware.ts @@ -0,0 +1,58 @@ +/* eslint-disable @typescript-eslint/explicit-module-boundary-types */ +import { + CREATE_FEATURE, + UPDATE_FEATURE, + DELETE_FEATURE, + 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'); + + const { featureToggleStore } = config.stores; + + return (req, res, next) => { + req.checkRbac = async (permission: string) => { + const { user, params } = req; + + if (!user) { + logger.error('RBAC requires a user to exist on the request.'); + return false; + } + + // Support ADMIN API tokens for enterpriseAuthentication. + if (user.isAPI) { + return user.permissions.includes(ADMIN); + } + + if (!user.id) { + logger.error('RBAC requires the user to have a unique id.'); + return false; + } + + // For /api/admin/projects/:projectId we will find it as part of params + let { projectId } = params; + + // Temporary workaround to figure our projectId for feature toggle updates. + if ([UPDATE_FEATURE, DELETE_FEATURE].includes(permission)) { + const { featureName } = params; + projectId = await featureToggleStore.getProjectId(featureName); + } else if (permission === CREATE_FEATURE) { + projectId = req.body.project; + } + + return accessService.hasPermission(user, permission, projectId); + }; + return next(); + }; +}; + +module.exports = rbacMiddleware; +export default rbacMiddleware; diff --git a/src/lib/middleware/simple-authentication.js b/src/lib/middleware/simple-authentication.js index 4ce4c9d8d6..a6ae11b580 100644 --- a/src/lib/middleware/simple-authentication.js +++ b/src/lib/middleware/simple-authentication.js @@ -2,7 +2,7 @@ const auth = require('basic-auth'); const User = require('../user'); const AuthenticationRequired = require('../authentication-required'); -function unsecureAuthentication(basePath = '', app) { +function insecureAuthentication(basePath = '', app) { app.post(`${basePath}/api/admin/login`, (req, res) => { const user = req.body; req.session.user = new User({ email: user.email }); @@ -41,4 +41,4 @@ function unsecureAuthentication(basePath = '', app) { }); } -module.exports = unsecureAuthentication; +module.exports = insecureAuthentication; diff --git a/src/lib/missing-permission.js b/src/lib/missing-permission.js deleted file mode 100644 index c66dca34e6..0000000000 --- a/src/lib/missing-permission.js +++ /dev/null @@ -1,8 +0,0 @@ -'use strict'; - -module.exports = class MissingPermission { - constructor({ permission, message }) { - this.permission = permission; - this.message = message; - } -}; diff --git a/src/lib/options.js b/src/lib/options.js index f396aa4177..258c12fcca 100644 --- a/src/lib/options.js +++ b/src/lib/options.js @@ -95,6 +95,7 @@ function defaultOptions() { enabled: process.env.CLIENT_FEATURE_MEMOIZE || false, maxAge: process.env.CLIENT_FEATURE_MAXAGE || 1000, }, + rbac: false, }, }; } diff --git a/src/lib/permissions.js b/src/lib/permissions.js index b1ae66ea53..75e174db2c 100644 --- a/src/lib/permissions.js +++ b/src/lib/permissions.js @@ -18,6 +18,8 @@ const DELETE_PROJECT = 'DELETE_PROJECT'; const CREATE_ADDON = 'CREATE_ADDON'; const UPDATE_ADDON = 'UPDATE_ADDON'; const DELETE_ADDON = 'DELETE_ADDON'; +const READ_ROLE = 'READ_ROLE'; +const UPDATE_ROLE = 'UPDATE_ROLE'; module.exports = { ADMIN, @@ -38,4 +40,6 @@ module.exports = { CREATE_ADDON, DELETE_ADDON, UPDATE_ADDON, + READ_ROLE, + UPDATE_ROLE, }; diff --git a/src/lib/routes/admin-api/feature.js b/src/lib/routes/admin-api/feature.js index 534a433af5..3dfe690432 100644 --- a/src/lib/routes/admin-api/feature.js +++ b/src/lib/routes/admin-api/feature.js @@ -39,7 +39,7 @@ class FeatureController extends Controller { this.post('/:featureName/toggle/off', this.toggleOff, UPDATE_FEATURE); this.post('/:featureName/stale/on', this.staleOn, UPDATE_FEATURE); this.post('/:featureName/stale/off', this.staleOff, UPDATE_FEATURE); - this.get('/:featureName/tags', this.listTags, UPDATE_FEATURE); + this.get('/:featureName/tags', this.listTags); this.post('/:featureName/tags', this.addTag, UPDATE_FEATURE); this.delete( '/:featureName/tags/:type/:value', diff --git a/src/lib/routes/admin-api/util.js b/src/lib/routes/admin-api/util.js index 5e8608d301..7bc2d1fa2e 100644 --- a/src/lib/routes/admin-api/util.js +++ b/src/lib/routes/admin-api/util.js @@ -29,6 +29,11 @@ const handleErrors = (res, logger, error) => { // eslint-disable-next-line no-param-reassign error.isJoi = true; switch (error.name) { + case 'NoAccessError': + return res + .status(403) + .json(error) + .end(); case 'NotFoundError': return res.status(404).end(); case 'InvalidOperationError': diff --git a/src/lib/routes/index.ts b/src/lib/routes/index.ts index e29e5d7bf0..87beeadf52 100644 --- a/src/lib/routes/index.ts +++ b/src/lib/routes/index.ts @@ -1,4 +1,4 @@ -'use strict'; +import { BackstageController } from './backstage'; const AdminApi = require('./admin-api'); const ClientApi = require('./client-api'); @@ -6,13 +6,12 @@ const FeatureController = require('./client-api/feature.js'); const Controller = require('./controller'); const HealthCheckController = require('./health-check'); -import { BackstageController } from './backstage'; const LogoutController = require('./logout'); const api = require('./api-def'); class IndexRouter extends Controller { constructor(config, services) { - super(); + super(config); this.use('/health', new HealthCheckController(config).router); this.use('/internal-backstage', new BackstageController(config).router); this.use('/logout', new LogoutController(config).router); diff --git a/src/lib/services/access-service.ts b/src/lib/services/access-service.ts new file mode 100644 index 0000000000..da4674cf22 --- /dev/null +++ b/src/lib/services/access-service.ts @@ -0,0 +1,223 @@ +import { AccessStore, IRole, IUserPermission } from '../db/access-store'; +import p from '../permissions'; +import User from '../user'; + +export const ALL_PROJECTS = '*'; + +const PROJECT_DESCRIPTION = { + ADMIN: 'Users with the project admin role have full control over the project, and can add and manage other users within the project context, manage feature toggles within the project, and control advanced project features like archiving and deleting the project.', + REGULAR: 'Users with the regular role within a project are allowed to view, create and update feature toggles, but have limited permissions in regards to managing the projects user access and can not archive or delete the project.', +}; + +const { ADMIN } = p; + +const PROJECT_ADMIN = [ + p.UPDATE_PROJECT, + p.DELETE_PROJECT, + p.CREATE_FEATURE, + p.UPDATE_FEATURE, + p.DELETE_FEATURE, +]; + +const PROJECT_REGULAR = [p.CREATE_FEATURE, p.UPDATE_FEATURE, p.DELETE_FEATURE]; + +const isProjectPermission = permission => PROJECT_ADMIN.includes(permission); + +interface IStores { + accessStore: AccessStore; + userStore: any; +} + +export interface IUserWithRole { + id: number; + roleId: number; + name?: string + username?: string; + email?: string; + imageUrl?: string; +} + +interface IRoleData { + role: IRole; + users: User[]; + permissions: IUserPermission[]; +} + +interface IPermission { + name: string; + type: PermissionType; +} + +enum PermissionType { + root='root', + project='project', +} + +export enum RoleName { + ADMIN = 'Admin', + REGULAR = 'Regular', + READ = 'Read', +} + +export enum RoleType { + ROOT = 'root', + PROJECT = 'project', +} + +export class AccessService { + public RoleName = RoleName; + private store: AccessStore; + + private userStore: any; + + private logger: any; + + private permissions: IPermission[]; + + constructor({ accessStore, userStore }: IStores, { getLogger } : { getLogger: Function}) { + this.store = accessStore; + this.userStore = userStore; + this.logger = getLogger('/services/access-service.ts'); + this.permissions = Object.values(p).map(p => ({ + name: p, + type: isProjectPermission(p) ? PermissionType.project : PermissionType.root + })) + } + + /** + * Used to check if a user has access to the requested resource + * + * @param user + * @param permission + * @param projectId + */ + async hasPermission(user: User, permission: string, projectId?: string): Promise { + this.logger.info(`Checking permission=${permission}, userId=${user.id} projectId=${projectId}`) + + const permissions = await this.store.getPermissionsForUser(user.id); + + return permissions + .filter(p => !p.project || p.project === projectId || p.project === ALL_PROJECTS) + .some(p => p.permission === permission || p.permission === ADMIN); + } + + getPermissions(): IPermission[] { + return this.permissions; + } + + async addUserToRole(userId: number, roleId: number) { + return this.store.addUserToRole(userId, roleId); + } + + async setUserRootRole(userId: number, roleName: RoleName ) { + const userRoles = await this.store.getRolesForUserId(userId); + const currentRootRoles = userRoles.filter(r => r.type === RoleType.ROOT); + + const roles = await this.getRoles(); + const role = roles.find(r => r.type === RoleType.ROOT && r.name === roleName); + if(role) { + try { + await Promise.all(currentRootRoles.map(r => this.store.removeUserFromRole(userId, r.id))); + await this.store.addUserToRole(userId, role.id); + } catch (error) { + this.logger.warn('Could not add role=${roleName} to userId=${userId}'); + } + } + } + + async removeUserFromRole(userId: number, roleId: number) { + return this.store.removeUserFromRole(userId, roleId); + } + + async addPermissionToRole(roleId: number, permission: string, projectId?: string) { + if(isProjectPermission(permission) && !projectId) { + throw new Error(`ProjectId cannot be empty for permission=${permission}`) + } + return this.store.addPermissionsToRole(roleId, [permission], projectId); + } + + async removePermissionFromRole(roleId: number, permission: string, projectId?: string) { + if(isProjectPermission(permission) && !projectId) { + throw new Error(`ProjectId cannot be empty for permission=${permission}`) + } + return this.store.removePermissionFromRole(roleId, permission, projectId); + } + + async getRoles(): Promise { + return this.store.getRoles(); + } + + async getRole(roleId: number): Promise { + const [role, permissions, users] = await Promise.all([ + this.store.getRoleWithId(roleId), + this.store.getPermissionsForRole(roleId), + this.getUsersForRole(roleId), + ]); + return { role, permissions, users }; + } + + async getRolesForProject(projectId: string): Promise { + return this.store.getRolesForProject(projectId); + } + + async getRolesForUser(userId: number): Promise { + return this.store.getRolesForUserId(userId); + } + + async getUsersForRole(roleId) : Promise { + const userIdList = await this.store.getUserIdsForRole(roleId); + return this.userStore.getAllWithId(userIdList); + } + + // Move to project-service? + async getProjectRoleUsers(projectId: string): Promise<[IRole[], IUserWithRole[]]> { + const roles = await this.store.getRolesForProject(projectId); + + const users = await Promise.all(roles.map(async role => { + const users = await this.getUsersForRole(role.id); + return users.map(u => ({ ...u, roleId: role.id })) + })); + return [roles, users.flat()]; + } + + async createDefaultProjectRoles(owner: User, projectId: string) { + if(!projectId) { + throw new Error("ProjectId cannot be empty"); + } + + const adminRole = await this.store.createRole( + RoleName.ADMIN, + RoleType.PROJECT, + projectId, + PROJECT_DESCRIPTION.ADMIN, + ); + await this.store.addPermissionsToRole( + adminRole.id, + PROJECT_ADMIN, + projectId, + ); + + // TODO: remove this when all users is guaranteed to have a unique id. + if (owner.id) { + this.logger.info(`Making ${owner.id} admin of ${projectId} via roleId=${adminRole.id}`); + await this.store.addUserToRole(owner.id, adminRole.id); + }; + + const regularRole = await this.store.createRole( + RoleName.REGULAR, + RoleType.PROJECT, + projectId, + PROJECT_DESCRIPTION.REGULAR, + ); + await this.store.addPermissionsToRole( + regularRole.id, + PROJECT_REGULAR, + projectId + ); + } + + async removeDefaultProjectRoles(owner: User, projectId: string) { + this.logger.info(`Removing project roles for ${projectId}`); + return this.store.removeRolesForProject(projectId); + } +} diff --git a/src/lib/services/index.js b/src/lib/services/index.js index ef6ff73b56..fad6f67dd8 100644 --- a/src/lib/services/index.js +++ b/src/lib/services/index.js @@ -8,10 +8,16 @@ const StrategyService = require('./strategy-service'); const AddonService = require('./addon-service'); const ContextService = require('./context-service'); const VersionService = require('./version-service'); +const { AccessService } = require('./access-service'); module.exports.createServices = (stores, config) => { - const featureToggleService = new FeatureToggleService(stores, config); - const projectService = new ProjectService(stores, config); + const accessService = new AccessService(stores, config); + const featureToggleService = new FeatureToggleService( + stores, + config, + accessService, + ); + const projectService = new ProjectService(stores, config, accessService); const stateService = new StateService(stores, config); const strategyService = new StrategyService(stores, config); const tagTypeService = new TagTypeService(stores, config); @@ -22,6 +28,7 @@ module.exports.createServices = (stores, config) => { const versionService = new VersionService(stores, config); return { + accessService, addonService, featureToggleService, projectService, diff --git a/src/lib/services/project-service.js b/src/lib/services/project-service.js deleted file mode 100644 index d6d52350e4..0000000000 --- a/src/lib/services/project-service.js +++ /dev/null @@ -1,93 +0,0 @@ -const NameExistsError = require('../error/name-exists-error'); -const InvalidOperationError = require('../error/invalid-operation-error'); -const eventType = require('../event-type'); -const { nameType } = require('../routes/admin-api/util'); -const schema = require('./project-schema'); - -class ProjectService { - constructor( - { projectStore, eventStore, featureToggleStore }, - { getLogger }, - ) { - this.projectStore = projectStore; - this.eventStore = eventStore; - this.featureToggleStore = featureToggleStore; - this.logger = getLogger('services/project-service.js'); - } - - async getProjects() { - return this.projectStore.getAll(); - } - - async getProject(id) { - return this.projectStore.get(id); - } - - async createProject(newProject, username) { - const data = await schema.validateAsync(newProject); - await this.validateUniqueId(data.id); - await this.eventStore.store({ - type: eventType.PROJECT_CREATED, - createdBy: username, - data, - }); - await this.projectStore.create(data); - } - - async updateProject(updatedProject, username) { - await this.projectStore.get(updatedProject.id); - const project = await schema.validateAsync(updatedProject); - await this.eventStore.store({ - type: eventType.PROJECT_UPDATED, - createdBy: username, - data: project, - }); - await this.projectStore.update(project); - } - - async deleteProject(id, username) { - if (id === 'default') { - throw new InvalidOperationError( - 'You can not delete the default project!', - ); - } - - const toggles = await this.featureToggleStore.getFeaturesBy({ - project: id, - archived: 0, - }); - - if (toggles.length > 0) { - throw new InvalidOperationError( - 'You can not delete as project with active feature toggles', - ); - } - - await this.eventStore.store({ - type: eventType.PROJECT_DELETED, - createdBy: username, - data: { id }, - }); - await this.projectStore.delete(id); - } - - async validateId(id) { - await nameType.validateAsync(id); - await this.validateUniqueId(id); - return true; - } - - async validateUniqueId(id) { - try { - await this.projectStore.hasProject(id); - } catch (error) { - // No conflict, everything ok! - return; - } - - // Interntional throw here! - throw new NameExistsError('A project with this id already exists.'); - } -} - -module.exports = ProjectService; diff --git a/src/lib/services/project-service.ts b/src/lib/services/project-service.ts new file mode 100644 index 0000000000..25fa54817f --- /dev/null +++ b/src/lib/services/project-service.ts @@ -0,0 +1,197 @@ +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'); +const eventType = require('../event-type'); +const { nameType } = require('../routes/admin-api/util'); +const schema = require('./project-schema'); +const NotFoundError = require('../error/notfound-error'); + +interface IProject { + id: string; + name: string; + description?: string; +} + +const getCreatedBy = (user: User) => user.email || user.username; + +const DEFAULT_PROJECT = 'default'; + +class ProjectService { + private projectStore: any; + + private accessService: AccessService; + + private eventStore: any; + + private featureToggleStore: any; + + private logger: any; + + private rbacEnabled: boolean; + + constructor( + { projectStore, eventStore, featureToggleStore }, + config: any, + accessService: AccessService, + ) { + this.projectStore = projectStore; + this.accessService = accessService; + this.eventStore = eventStore; + this.featureToggleStore = featureToggleStore; + this.logger = config.getLogger('services/project-service.js'); + this.rbacEnabled = isRbacEnabled(config); + } + + async getProjects() { + return this.projectStore.getAll(); + } + + async getProject(id) { + return this.projectStore.get(id); + } + + async createProject(newProject: IProject, user: User): Promise { + const data = await schema.validateAsync(newProject); + await this.validateUniqueId(data.id); + + await this.projectStore.create(data); + + if (this.rbacEnabled) { + await this.accessService.createDefaultProjectRoles(user, data.id); + } + + await this.eventStore.store({ + type: eventType.PROJECT_CREATED, + createdBy: getCreatedBy(user), + data, + }); + + return data; + } + + async updateProject(updatedProject: IProject, user: User): Promise { + await this.projectStore.get(updatedProject.id); + const project = await schema.validateAsync(updatedProject); + + await this.projectStore.update(project); + + await this.eventStore.store({ + type: eventType.PROJECT_UPDATED, + createdBy: getCreatedBy(user), + data: project, + }); + } + + async deleteProject(id: string, user: User): Promise { + if (id === DEFAULT_PROJECT) { + throw new InvalidOperationError( + 'You can not delete the default project!', + ); + } + + const toggles = await this.featureToggleStore.getFeaturesBy({ + project: id, + archived: 0, + }); + + if (toggles.length > 0) { + throw new InvalidOperationError( + 'You can not delete as project with active feature toggles', + ); + } + + await this.projectStore.delete(id); + + await this.eventStore.store({ + type: eventType.PROJECT_DELETED, + createdBy: getCreatedBy(user), + data: { id }, + }); + + if (this.rbacEnabled) { + this.accessService.removeDefaultProjectRoles(user, id); + } + } + + async validateId(id: string): Promise { + await nameType.validateAsync(id); + await this.validateUniqueId(id); + return true; + } + + async validateUniqueId(id: string): Promise { + try { + await this.projectStore.hasProject(id); + } catch (error) { + // No conflict, everything ok! + return; + } + + // Intentional throw here! + throw new NameExistsError('A project with this id already exists.'); + } + + // RBAC methods + async getUsersWithAccess(projectId: string) { + const [roles, users] = await this.accessService.getProjectRoleUsers( + projectId, + ); + + return { + roles, + users, + }; + } + + async addUser( + projectId: string, + roleId: number, + userId: number, + ): Promise { + const [roles, users] = await this.accessService.getProjectRoleUsers( + projectId, + ); + + const role = roles.find(r => r.id === roleId); + if (!role) { + throw new NotFoundError( + `Could not find roleId=${roleId} on project=${projectId}`, + ); + } + + const alreadyHasAccess = users.some(u => u.id === userId); + if (alreadyHasAccess) { + throw new Error(`User already have access to project=${projectId}`); + } + + await this.accessService.addUserToRole(userId, role.id); + } + + async removeUser( + projectId: string, + roleId: number, + userId: number, + ): Promise { + const roles = await this.accessService.getRolesForProject(projectId); + const role = roles.find(r => r.id === roleId); + if (!role) { + throw new NotFoundError( + `Couldn't find roleId=${roleId} on project=${projectId}`, + ); + } + + if (role.name === RoleName.ADMIN) { + const users = await this.accessService.getUsersForRole(role.id); + if (users.length < 2) { + throw new Error('A project must have at least one admin'); + } + } + + await this.accessService.removeUserFromRole(userId, role.id); + } +} + +module.exports = ProjectService; diff --git a/src/lib/user.js b/src/lib/user.js deleted file mode 100644 index b87c088663..0000000000 --- a/src/lib/user.js +++ /dev/null @@ -1,42 +0,0 @@ -'use strict'; - -const gravatarUrl = require('gravatar-url'); -const Joi = require('joi'); - -module.exports = class User { - constructor({ - id, - name, - email, - username, - imageUrl, - permissions, - seenAt, - loginAttempts, - createdAt, - } = {}) { - if (!username && !email) { - throw new TypeError('Username or Email us reuqired'); - } - Joi.assert(email, Joi.string().email(), 'Email'); - Joi.assert(username, Joi.string(), 'Username'); - Joi.assert(name, Joi.string(), 'Name'); - - this.id = id; - this.name = name; - this.username = username; - this.email = email; - this.permissions = permissions; - this.imageUrl = imageUrl || this.generateImageUrl(); - this.seenAt = seenAt; - this.loginAttempts = loginAttempts; - this.createdAt = createdAt; - } - - generateImageUrl() { - return gravatarUrl(this.email || this.username, { - size: '42', - default: 'retro', - }); - } -}; diff --git a/src/lib/user.test.js b/src/lib/user.test.js index 17766f0eed..104cc3434a 100644 --- a/src/lib/user.test.js +++ b/src/lib/user.test.js @@ -38,7 +38,7 @@ test('should require email or username', t => { { instanceOf: Error }, ); - t.is(error.message, 'Username or Email us reuqired'); + t.is(error.message, 'Username or Email is required'); }); test('Should create user with only email defined', t => { diff --git a/src/lib/user.ts b/src/lib/user.ts new file mode 100644 index 0000000000..c44ebbc3b2 --- /dev/null +++ b/src/lib/user.ts @@ -0,0 +1,79 @@ +import gravatarUrl from 'gravatar-url'; +import Joi from 'joi'; + +export interface UserData { + id?: number; + isAPI?: boolean; + name?: string; + username?: string; + email?: string; + permissions?: string[]; + imageUrl?: string; + seenAt?: Date; + loginAttempts?: number; + createdAt?: Date; +} + +export default class User { + id: number; + + isAPI: boolean; + + name: string; + + username: string; + + email: string; + + permissions: string[]; + + imageUrl: string; + + seenAt: Date; + + loginAttempts: number; + + createdAt: Date; + + constructor( + { + id, + isAPI, + name, + email, + username, + imageUrl, + permissions, + seenAt, + loginAttempts, + createdAt, + }: UserData = { isAPI: false }, + ) { + if (!username && !email) { + throw new TypeError('Username or Email is required'); + } + Joi.assert(email, Joi.string().email(), 'Email'); + Joi.assert(username, Joi.string(), 'Username'); + Joi.assert(name, Joi.string(), 'Name'); + + this.id = id; + this.isAPI = isAPI; + this.name = name; + this.username = username; + this.email = email; + this.permissions = permissions; + this.imageUrl = imageUrl || this.generateImageUrl(); + this.seenAt = seenAt; + this.loginAttempts = loginAttempts; + this.createdAt = createdAt; + } + + generateImageUrl(): string { + return gravatarUrl(this.email || this.username, { + size: 42, + default: 'retro', + }); + } +} + +module.exports = User; diff --git a/src/lib/util/feature-enabled.ts b/src/lib/util/feature-enabled.ts new file mode 100644 index 0000000000..5eb699c307 --- /dev/null +++ b/src/lib/util/feature-enabled.ts @@ -0,0 +1,13 @@ +interface IExperimentalFlags { + [key: string]: boolean; +} + +interface IConfig { + experimental: IExperimentalFlags; +} + +export const isRbacEnabled = (config: IConfig): boolean => { + return config && config.experimental && config.experimental.rbac; +}; + +module.exports = { isRbacEnabled }; diff --git a/src/migrations/.eslintrc b/src/migrations/.eslintrc new file mode 100644 index 0000000000..9e300b35fc --- /dev/null +++ b/src/migrations/.eslintrc @@ -0,0 +1,16 @@ +{ + "env": {}, + "extends": [], + "parser": "", + "plugins": [], + "rules": {}, + "overrides": [ + { + "files": "*.js", + "rules": { + "@typescript-eslint/indent": "off" + } + } + ], + "settings": {} +} \ No newline at end of file diff --git a/src/migrations/20210217195834-rbac-tables.js b/src/migrations/20210217195834-rbac-tables.js new file mode 100644 index 0000000000..99329dee62 --- /dev/null +++ b/src/migrations/20210217195834-rbac-tables.js @@ -0,0 +1,81 @@ +exports.up = function(db, cb) { + db.runSql( + `CREATE TABLE IF NOT EXISTS roles + ( + id SERIAL PRIMARY KEY, + name text not null, + description text, + type text not null default 'custom', + project text, + created_at TIMESTAMP WITH TIME ZONE DEFAULT now() + ); + CREATE TABLE IF NOT EXISTS role_user + ( + role_id integer not null references roles (id) ON DELETE CASCADE, + user_id integer not null references users (id) ON DELETE CASCADE, + created_at TIMESTAMP WITH TIME ZONE DEFAULT now(), + PRIMARY KEY (role_id, user_id) + ); + CREATE TABLE IF NOT EXISTS role_permission + ( + role_id integer not null references roles (id) ON DELETE CASCADE, + project text, + permission text not null, + created_at TIMESTAMP WITH TIME ZONE DEFAULT now() + ); + + WITH admin AS ( + INSERT INTO roles(name, description, type) + VALUES ('Admin', 'Users with the global admin role have superuser access to Unleash and can perform any operation within the unleash platform.', 'root') + RETURNING id role_id + ) + + INSERT INTO role_permission(role_id, permission) + SELECT role_id, 'ADMIN' from admin; + + WITH regular AS ( + INSERT INTO roles(name, description, type) + VALUES ('Regular', 'Users with the global regular role have access most features in Unleash, but can not manage users and roles in the global scope. If a user with a global regular role creates a project, they will become a project admin and receive superuser rights within the context of that project.', 'root') + RETURNING id role_id + ) + INSERT INTO role_permission(role_id, project, permission) + VALUES + ((SELECT role_id from regular), '', 'CREATE_STRATEGY'), + ((SELECT role_id from regular), '', 'UPDATE_STRATEGY'), + ((SELECT role_id from regular), '', 'DELETE_STRATEGY'), + + ((SELECT role_id from regular), '', 'UPDATE_APPLICATION'), + + ((SELECT role_id from regular), '', 'CREATE_CONTEXT_FIELD'), + ((SELECT role_id from regular), '', 'UPDATE_CONTEXT_FIELD'), + ((SELECT role_id from regular), '', 'DELETE_CONTEXT_FIELD'), + + ((SELECT role_id from regular), '', 'CREATE_PROJECT'), + + ((SELECT role_id from regular), '', 'CREATE_ADDON'), + ((SELECT role_id from regular), '', 'UPDATE_ADDON'), + ((SELECT role_id from regular), '', 'DELETE_ADDON'), + + ((SELECT role_id from regular), 'default', 'UPDATE_PROJECT'), + ((SELECT role_id from regular), 'default', 'DELETE_PROJECT'), + ((SELECT role_id from regular), 'default', 'CREATE_FEATURE'), + ((SELECT role_id from regular), 'default', 'UPDATE_FEATURE'), + ((SELECT role_id from regular), 'default', 'DELETE_FEATURE'); + + INSERT INTO roles(name, description, type) + VALUES ('Read', 'Users with this role can only read root resources in Unleash. They may be added as collaborator to specific projects.', 'root'); + `, + cb, + ); +}; + +exports.down = function(db, cb) { + db.runSql( + ` + DROP TABLE role_user; + DROP TABLE role_permission; + DROP TABLE roles; + `, + cb, + ); +}; diff --git a/src/test/e2e/helpers/database-init.js b/src/test/e2e/helpers/database-init.js index 432673674d..5950062b3c 100644 --- a/src/test/e2e/helpers/database-init.js +++ b/src/test/e2e/helpers/database-init.js @@ -28,6 +28,7 @@ async function resetDatabase(stores) { stores.db('tags').del(), stores.db('tag_types').del(), stores.db('addons').del(), + stores.db('users').del(), ]); } diff --git a/src/test/e2e/services/access-service.e2e.test.js b/src/test/e2e/services/access-service.e2e.test.js new file mode 100644 index 0000000000..eb69b7dcf9 --- /dev/null +++ b/src/test/e2e/services/access-service.e2e.test.js @@ -0,0 +1,420 @@ +const test = require('ava'); +const dbInit = require('../helpers/database-init'); +const getLogger = require('../../fixtures/no-logger'); + +// eslint-disable-next-line import/no-unresolved +const { + AccessService, + RoleName, + ALL_PROJECTS, +} = require('../../../lib/services/access-service'); +const permissions = require('../../../lib/permissions'); +const User = require('../../../lib/user'); + +let stores; +let accessService; + +let regularUser; +let superUser; + +const createUserWithRegularAccess = async (name, email) => { + const { userStore } = stores; + const user = await userStore.insert(new User({ name, email })); + const roles = await accessService.getRoles(); + const regularRole = roles.find(r => r.name === 'Regular'); + await accessService.addUserToRole(user.id, regularRole.id); + return user; +}; + +const createSuperUser = async () => { + const { userStore } = stores; + const user = await userStore.insert( + new User({ name: 'Alice Admin', email: 'admin@getunleash.io' }), + ); + const roles = await accessService.getRoles(); + const superRole = roles.find(r => r.name === 'Admin'); + await accessService.addUserToRole(user.id, superRole.id); + return user; +}; + +test.before(async () => { + const db = await dbInit('access_service_serial', getLogger); + stores = db.stores; + // projectStore = stores.projectStore; + accessService = new AccessService(stores, { getLogger }); + regularUser = await createUserWithRegularAccess( + 'Bob Test', + 'bob@getunleash.io', + ); + superUser = await createSuperUser(); +}); + +test.after(async () => { + await stores.db.destroy(); +}); + +test.serial('should have access to admin addons', async t => { + const { CREATE_ADDON, UPDATE_ADDON, DELETE_ADDON } = permissions; + const user = regularUser; + t.true(await accessService.hasPermission(user, CREATE_ADDON)); + t.true(await accessService.hasPermission(user, UPDATE_ADDON)); + t.true(await accessService.hasPermission(user, DELETE_ADDON)); +}); + +test.serial('should have access to admin strategies', async t => { + const { CREATE_STRATEGY, UPDATE_STRATEGY, DELETE_STRATEGY } = permissions; + const user = regularUser; + t.true(await accessService.hasPermission(user, CREATE_STRATEGY)); + t.true(await accessService.hasPermission(user, UPDATE_STRATEGY)); + t.true(await accessService.hasPermission(user, DELETE_STRATEGY)); +}); + +test.serial('should have access to admin contexts', async t => { + const { + CREATE_CONTEXT_FIELD, + UPDATE_CONTEXT_FIELD, + DELETE_CONTEXT_FIELD, + } = permissions; + const user = regularUser; + t.true(await accessService.hasPermission(user, CREATE_CONTEXT_FIELD)); + t.true(await accessService.hasPermission(user, UPDATE_CONTEXT_FIELD)); + t.true(await accessService.hasPermission(user, DELETE_CONTEXT_FIELD)); +}); + +test.serial('should have access to create projects', async t => { + const { CREATE_PROJECT } = permissions; + const user = regularUser; + t.true(await accessService.hasPermission(user, CREATE_PROJECT)); +}); + +test.serial('should have access to update applications', async t => { + const { UPDATE_APPLICATION } = permissions; + const user = regularUser; + t.true(await accessService.hasPermission(user, UPDATE_APPLICATION)); +}); + +test.serial('should not have admin permission', async t => { + const { ADMIN } = permissions; + const user = regularUser; + t.false(await accessService.hasPermission(user, ADMIN)); +}); + +test.serial('should have project admin to default project', async t => { + const { + DELETE_PROJECT, + UPDATE_PROJECT, + CREATE_FEATURE, + UPDATE_FEATURE, + DELETE_FEATURE, + } = permissions; + const user = regularUser; + t.true(await accessService.hasPermission(user, DELETE_PROJECT, 'default')); + t.true(await accessService.hasPermission(user, UPDATE_PROJECT, 'default')); + t.true(await accessService.hasPermission(user, CREATE_FEATURE, 'default')); + t.true(await accessService.hasPermission(user, UPDATE_FEATURE, 'default')); + t.true(await accessService.hasPermission(user, DELETE_FEATURE, 'default')); +}); + +test.serial('should grant regular CREATE_FEATURE on all projects', async t => { + const { CREATE_FEATURE } = permissions; + const user = regularUser; + + const roles = await accessService.getRoles(); + const regularRole = roles.find( + r => r.name === 'Regular' && r.type === 'root', + ); + + await accessService.addPermissionToRole( + regularRole.id, + permissions.CREATE_FEATURE, + ALL_PROJECTS, + ); + + t.true( + await accessService.hasPermission(user, CREATE_FEATURE, 'some-project'), + ); +}); + +test.serial('cannot add CREATE_FEATURE without defining project', async t => { + const roles = await accessService.getRoles(); + const regularRole = roles.find( + r => r.name === 'Regular' && r.type === 'root', + ); + + await t.throwsAsync( + async () => { + await accessService.addPermissionToRole( + regularRole.id, + permissions.CREATE_FEATURE, + ); + }, + { + instanceOf: Error, + message: 'ProjectId cannot be empty for permission=CREATE_FEATURE', + }, + ); +}); + +test.serial( + 'cannot remove CREATE_FEATURE without defining project', + async t => { + const roles = await accessService.getRoles(); + const regularRole = roles.find( + r => r.name === 'Regular' && r.type === 'root', + ); + + await t.throwsAsync( + async () => { + await accessService.removePermissionFromRole( + regularRole.id, + permissions.CREATE_FEATURE, + ); + }, + { + instanceOf: Error, + message: + 'ProjectId cannot be empty for permission=CREATE_FEATURE', + }, + ); + }, +); + +test.serial('should remove CREATE_FEATURE on all projects', async t => { + const { CREATE_FEATURE } = permissions; + const user = regularUser; + + const roles = await accessService.getRoles(); + const regularRole = roles.find( + r => r.name === 'Regular' && r.type === 'root', + ); + + await accessService.addPermissionToRole( + regularRole.id, + permissions.CREATE_FEATURE, + ALL_PROJECTS, + ); + + await accessService.removePermissionFromRole( + regularRole.id, + permissions.CREATE_FEATURE, + ALL_PROJECTS, + ); + + t.false( + await accessService.hasPermission(user, CREATE_FEATURE, 'some-project'), + ); +}); + +test.serial('admin should be admin', async t => { + const { + DELETE_PROJECT, + UPDATE_PROJECT, + CREATE_FEATURE, + UPDATE_FEATURE, + DELETE_FEATURE, + ADMIN, + } = permissions; + const user = superUser; + t.true(await accessService.hasPermission(user, DELETE_PROJECT, 'default')); + t.true(await accessService.hasPermission(user, UPDATE_PROJECT, 'default')); + t.true(await accessService.hasPermission(user, CREATE_FEATURE, 'default')); + t.true(await accessService.hasPermission(user, UPDATE_FEATURE, 'default')); + t.true(await accessService.hasPermission(user, DELETE_FEATURE, 'default')); + t.true(await accessService.hasPermission(user, ADMIN)); +}); + +test.serial('should create default roles to project', async t => { + const { + DELETE_PROJECT, + UPDATE_PROJECT, + CREATE_FEATURE, + UPDATE_FEATURE, + DELETE_FEATURE, + } = permissions; + const project = 'some-project'; + const user = regularUser; + await accessService.createDefaultProjectRoles(user, project); + t.true(await accessService.hasPermission(user, UPDATE_PROJECT, project)); + t.true(await accessService.hasPermission(user, DELETE_PROJECT, project)); + t.true(await accessService.hasPermission(user, CREATE_FEATURE, project)); + t.true(await accessService.hasPermission(user, UPDATE_FEATURE, project)); + t.true(await accessService.hasPermission(user, DELETE_FEATURE, project)); +}); + +test.serial( + 'should require name when create default roles to project', + async t => { + await t.throwsAsync( + async () => { + await accessService.createDefaultProjectRoles(regularUser); + }, + { instanceOf: Error, message: 'ProjectId cannot be empty' }, + ); + }, +); + +test.serial('should grant user access to project', async t => { + const { + DELETE_PROJECT, + UPDATE_PROJECT, + CREATE_FEATURE, + UPDATE_FEATURE, + DELETE_FEATURE, + } = permissions; + const project = 'another-project'; + const user = regularUser; + const sUser = await createUserWithRegularAccess( + 'Some Random', + 'random@getunleash.io', + ); + await accessService.createDefaultProjectRoles(user, project); + + const roles = await accessService.getRolesForProject(project); + + const regularRole = roles.find( + r => r.name === 'Regular' && r.project === project, + ); + await accessService.addUserToRole(sUser.id, regularRole.id); + + // Should be able to update feature toggles inside the project + t.true(await accessService.hasPermission(sUser, CREATE_FEATURE, project)); + t.true(await accessService.hasPermission(sUser, UPDATE_FEATURE, project)); + t.true(await accessService.hasPermission(sUser, DELETE_FEATURE, project)); + + // Should not be able to admin the project itself. + t.false(await accessService.hasPermission(sUser, UPDATE_PROJECT, project)); + t.false(await accessService.hasPermission(sUser, DELETE_PROJECT, project)); +}); + +test.serial('should not get access if not specifying project', async t => { + const { CREATE_FEATURE, UPDATE_FEATURE, DELETE_FEATURE } = permissions; + const project = 'another-project-2'; + const user = regularUser; + const sUser = await createUserWithRegularAccess( + 'Some Random', + 'random22@getunleash.io', + ); + await accessService.createDefaultProjectRoles(user, project); + + const roles = await accessService.getRolesForProject(project); + + const regularRole = roles.find( + r => r.name === 'Regular' && r.project === project, + ); + await accessService.addUserToRole(sUser.id, regularRole.id); + + // Should not be able to update feature toggles outside project + t.false(await accessService.hasPermission(sUser, CREATE_FEATURE)); + t.false(await accessService.hasPermission(sUser, UPDATE_FEATURE)); + t.false(await accessService.hasPermission(sUser, DELETE_FEATURE)); +}); + +test.serial('should remove user from role', async t => { + const { userStore } = stores; + const user = await userStore.insert( + new User({ name: 'Some User', email: 'random123@getunleash.io' }), + ); + + const roles = await accessService.getRoles(); + const regularRole = roles.find(r => r.name === 'Regular'); + await accessService.addUserToRole(user.id, regularRole.id); + + // check user has one role + const userRoles = await accessService.getRolesForUser(user.id); + t.is(userRoles.length, 1); + t.is(userRoles[0].name, 'Regular'); + + await accessService.removeUserFromRole(user.id, regularRole.id); + const userRolesAfterRemove = await accessService.getRolesForUser(user.id); + t.is(userRolesAfterRemove.length, 0); +}); + +test.serial('should return role with users', async t => { + const { userStore } = stores; + const user = await userStore.insert( + new User({ name: 'Some User', email: 'random2223@getunleash.io' }), + ); + + const roles = await accessService.getRoles(); + const regularRole = roles.find(r => r.name === 'Regular'); + await accessService.addUserToRole(user.id, regularRole.id); + + const roleWithUsers = await accessService.getRole(regularRole.id); + + t.is(roleWithUsers.role.name, 'Regular'); + t.true(roleWithUsers.users.length > 2); + t.truthy(roleWithUsers.users.find(u => u.id === user.id)); + t.truthy(roleWithUsers.users.find(u => u.email === user.email)); +}); + +test.serial('should return role with permissions and users', async t => { + const { userStore } = stores; + const user = await userStore.insert( + new User({ name: 'Some User', email: 'random2244@getunleash.io' }), + ); + + const roles = await accessService.getRoles(); + const regularRole = roles.find(r => r.name === 'Regular'); + await accessService.addUserToRole(user.id, regularRole.id); + + const roleWithPermission = await accessService.getRole(regularRole.id); + + t.is(roleWithPermission.role.name, 'Regular'); + t.true(roleWithPermission.permissions.length > 2); + t.truthy( + roleWithPermission.permissions.find( + p => p.permission === permissions.CREATE_PROJECT, + ), + ); + t.true(roleWithPermission.users.length > 2); +}); + +test.serial('should return list of permissions', async t => { + const p = await accessService.getPermissions(); + + const findPerm = perm => p.find(_ => _.name === perm); + + const { + DELETE_FEATURE, + UPDATE_FEATURE, + CREATE_FEATURE, + UPDATE_PROJECT, + CREATE_PROJECT, + } = permissions; + + t.true(p.length > 2); + t.is(findPerm(CREATE_PROJECT).type, 'root'); + t.is(findPerm(UPDATE_PROJECT).type, 'project'); + t.is(findPerm(CREATE_FEATURE).type, 'project'); + t.is(findPerm(UPDATE_FEATURE).type, 'project'); + t.is(findPerm(DELETE_FEATURE).type, 'project'); +}); + +test.serial('should set root role for user', async t => { + const { userStore } = stores; + const user = await userStore.insert( + new User({ name: 'Some User', email: 'random2255@getunleash.io' }), + ); + + await accessService.setUserRootRole(user.id, RoleName.REGULAR); + + const roles = await accessService.getRolesForUser(user.id); + + t.is(roles.length, 1); + t.is(roles[0].name, RoleName.REGULAR); +}); + +test.serial('should switch root role for user', async t => { + const { userStore } = stores; + const user = await userStore.insert( + new User({ name: 'Some User', email: 'random22Read@getunleash.io' }), + ); + + await accessService.setUserRootRole(user.id, RoleName.REGULAR); + await accessService.setUserRootRole(user.id, RoleName.READ); + + const roles = await accessService.getRolesForUser(user.id); + + t.is(roles.length, 1); + t.is(roles[0].name, RoleName.READ); +}); diff --git a/src/test/e2e/services/project-service.e2e.test.js b/src/test/e2e/services/project-service.e2e.test.js index dd1c7418a7..77b55ee9ac 100644 --- a/src/test/e2e/services/project-service.e2e.test.js +++ b/src/test/e2e/services/project-service.e2e.test.js @@ -2,16 +2,29 @@ const test = require('ava'); const dbInit = require('../helpers/database-init'); const getLogger = require('../../fixtures/no-logger'); const ProjectService = require('../../../lib/services/project-service'); +const { + AccessService, + RoleName, +} = require('../../../lib/services/access-service'); +const User = require('../../../lib/user'); +const { UPDATE_PROJECT } = require('../../../lib/permissions'); +const NotFoundError = require('../../../lib/error/notfound-error'); let stores; // let projectStore; let projectService; +let accessService; +let user; test.before(async () => { const db = await dbInit('project_service_serial', getLogger); stores = db.stores; - // projectStore = stores.projectStore; - projectService = new ProjectService(stores, { getLogger }); + user = await stores.userStore.insert( + new User({ name: 'Some Name', email: 'test@getunleash.io' }), + ); + const config = { getLogger, experimental: { rbac: true } }; + accessService = new AccessService(stores, config); + projectService = new ProjectService(stores, config, accessService); }); test.after(async () => { @@ -30,7 +43,8 @@ test.serial('should list all projects', async t => { name: 'New project', description: 'Blah', }; - await projectService.createProject(project, 'someUser'); + + await projectService.createProject(project, user); const projects = await projectService.getProjects(); t.is(projects.length, 2); }); @@ -41,7 +55,8 @@ test.serial('should create new project', async t => { name: 'New project', description: 'Blah', }; - await projectService.createProject(project, 'someUser'); + + await projectService.createProject(project, user); const ret = await projectService.getProject('test'); t.deepEqual(project.id, ret.id); t.deepEqual(project.name, ret.name); @@ -55,8 +70,9 @@ test.serial('should delete project', async t => { name: 'New project', description: 'Blah', }; - await projectService.createProject(project, 'some-user'); - await projectService.deleteProject(project.id, 'some-user'); + + await projectService.createProject(project, user); + await projectService.deleteProject(project.id, user); try { await projectService.getProject(project.id); @@ -71,7 +87,7 @@ test.serial('should not be able to delete project with toggles', async t => { name: 'New project', description: 'Blah', }; - await projectService.createProject(project, 'some-user'); + await projectService.createProject(project, user); await stores.featureToggleStore.createFeature({ name: 'test-project-delete', project: project.id, @@ -79,7 +95,7 @@ test.serial('should not be able to delete project with toggles', async t => { }); try { - await projectService.deleteProject(project.id, 'some-user'); + await projectService.deleteProject(project.id, user); } catch (err) { t.is( err.message, @@ -90,7 +106,7 @@ test.serial('should not be able to delete project with toggles', async t => { test.serial('should not delete "default" project', async t => { try { - await projectService.deleteProject('default', 'some-user'); + await projectService.deleteProject('default', user); } catch (err) { t.is(err.message, 'You can not delete the default project!'); } @@ -108,8 +124,8 @@ test.serial('should not be able to create exiting project', async t => { description: 'Blah', }; try { - await projectService.createProject(project, 'some-user'); - await projectService.createProject(project, 'some-user'); + await projectService.createProject(project, user); + await projectService.createProject(project, user); } catch (err) { t.is(err.message, 'A project with this id already exists.'); } @@ -144,8 +160,8 @@ test.serial('should update project', async t => { description: 'Blah longer desc', }; - await projectService.createProject(project, 'some-user'); - await projectService.updateProject(updatedProject, 'some-user'); + await projectService.createProject(project, user); + await projectService.updateProject(updatedProject, user); const readProject = await projectService.getProject(project.id); @@ -160,3 +176,197 @@ test.serial('should give error when getting unknown project', async t => { t.is(err.message, 'No project found'); } }); + +test.serial( + '(TODO: v4): should create roles for new project if userId is missing', + async t => { + const project = { + id: 'test-roles-no-id', + name: 'New project', + description: 'Blah', + }; + await projectService.createProject(project, { + username: 'random-user', + }); + const roles = await stores.accessStore.getRolesForProject(project.id); + + t.is(roles.length, 2); + t.false( + await accessService.hasPermission(user, UPDATE_PROJECT, project.id), + ); + }, +); + +test.serial('should create roles when project is created', async t => { + const project = { + id: 'test-roles', + name: 'New project', + description: 'Blah', + }; + await projectService.createProject(project, user); + const roles = await stores.accessStore.getRolesForProject(project.id); + t.is(roles.length, 2); + t.true(await accessService.hasPermission(user, UPDATE_PROJECT, project.id)); +}); + +test.serial('should get list of users with access to project', async t => { + const project = { + id: 'test-roles-access', + name: 'New project', + description: 'Blah', + }; + await projectService.createProject(project, user); + const { roles, users } = await projectService.getUsersWithAccess( + project.id, + user, + ); + + const admin = roles.find(role => role.name === RoleName.ADMIN); + const regular = roles.find(role => role.name === RoleName.REGULAR); + + t.is(users.length, 1); + t.is(users[0].id, user.id); + t.is(users[0].name, user.name); + t.is(users[0].roleId, admin.id); + t.truthy(regular); +}); + +test.serial('should add a regular user to the project', async t => { + const project = { + id: 'add-users', + name: 'New project', + description: 'Blah', + }; + await projectService.createProject(project, user); + + const projectMember1 = await stores.userStore.insert( + new User({ name: 'Some Member', email: 'member1@getunleash.io' }), + ); + const projectMember2 = await stores.userStore.insert( + new User({ name: 'Some Member 2', email: 'member2@getunleash.io' }), + ); + + const roles = await stores.accessStore.getRolesForProject(project.id); + const regularRole = roles.find(r => r.name === RoleName.REGULAR); + + await projectService.addUser(project.id, regularRole.id, projectMember1.id); + await projectService.addUser(project.id, regularRole.id, projectMember2.id); + + const { users } = await projectService.getUsersWithAccess(project.id, user); + const regularUsers = users.filter(u => u.roleId === regularRole.id); + + t.is(regularUsers.length, 2); + t.is(regularUsers[0].id, projectMember1.id); + t.is(regularUsers[0].name, projectMember1.name); + t.is(regularUsers[1].id, projectMember2.id); + t.is(regularUsers[1].name, projectMember2.name); +}); + +test.serial('should add admin users to the project', async t => { + const project = { + id: 'add-admin-users', + name: 'New project', + description: 'Blah', + }; + await projectService.createProject(project, user); + + const projectAdmin1 = await stores.userStore.insert( + new User({ name: 'Some Member', email: 'admin1@getunleash.io' }), + ); + const projectAdmin2 = await stores.userStore.insert( + new User({ name: 'Some Member 2', email: 'admin2@getunleash.io' }), + ); + + const projectRoles = await stores.accessStore.getRolesForProject( + project.id, + ); + const adminRole = projectRoles.find(r => r.name === RoleName.ADMIN); + + await projectService.addUser(project.id, adminRole.id, projectAdmin1.id); + await projectService.addUser(project.id, adminRole.id, projectAdmin2.id); + + const { users } = await projectService.getUsersWithAccess(project.id, user); + + const adminUsers = users.filter(u => u.roleId === adminRole.id); + + t.is(adminUsers.length, 3); + t.is(adminUsers[1].id, projectAdmin1.id); + t.is(adminUsers[1].name, projectAdmin1.name); + t.is(adminUsers[2].id, projectAdmin2.id); + t.is(adminUsers[2].name, projectAdmin2.name); +}); + +test.serial('add user only accept to add users to project roles', async t => { + const roles = await accessService.getRoles(); + const regularRole = roles.find(r => r.name === RoleName.REGULAR); + + await t.throwsAsync( + async () => { + await projectService.addUser('some-id', regularRole.id, user.id); + }, + { + instanceOf: NotFoundError, + message: 'Could not find roleId=2 on project=some-id', + }, + ); +}); + +test.serial('add user should fail if user already have access', async t => { + const project = { + id: 'add-users-twice', + name: 'New project', + description: 'Blah', + }; + await projectService.createProject(project, user); + + const projectMember1 = await stores.userStore.insert( + new User({ name: 'Some Member', email: 'member42@getunleash.io' }), + ); + + const roles = await stores.accessStore.getRolesForProject(project.id); + const regularRole = roles.find(r => r.name === RoleName.REGULAR); + + await projectService.addUser(project.id, regularRole.id, projectMember1.id); + + await t.throwsAsync( + async () => { + await projectService.addUser( + project.id, + regularRole.id, + projectMember1.id, + ); + }, + { + instanceOf: Error, + message: 'User already have access to project=add-users-twice', + }, + ); +}); + +test.serial('should remove user from the project', async t => { + const project = { + id: 'remove-users', + name: 'New project', + description: 'Blah', + }; + await projectService.createProject(project, user); + + const projectMember1 = await stores.userStore.insert( + new User({ name: 'Some Member', email: 'member99@getunleash.io' }), + ); + + const roles = await stores.accessStore.getRolesForProject(project.id); + const regularRole = roles.find(r => r.name === RoleName.REGULAR); + + await projectService.addUser(project.id, regularRole.id, projectMember1.id); + await projectService.removeUser( + project.id, + regularRole.id, + projectMember1.id, + ); + + const { users } = await projectService.getUsersWithAccess(project.id, user); + const regularUsers = users.filter(u => u.roleId === regularRole.id); + + t.is(regularUsers.length, 0); +});