From 765d7aaa5270a90ea42fb5e4068b92aa2919d932 Mon Sep 17 00:00:00 2001 From: Convly Date: Wed, 10 Jun 2020 18:04:47 +0200 Subject: [PATCH] Add check many permissions route/controller / Add userAbility to the context's state / Add isAuthenticatedAdmin.js Signed-off-by: Convly --- .../config/policies/isAuthenticatedAdmin.js | 9 ++ packages/strapi-admin/config/routes.json | 9 +- .../controllers/__tests__/permission.test.js | 125 ++++++++++++++++++ .../strapi-admin/controllers/permission.js | 23 ++++ .../strapi-admin/middlewares/auth/index.js | 3 + .../strapi-admin/validation/permission.js | 20 +++ packages/strapi-utils/lib/policy.js | 19 ++- 7 files changed, 203 insertions(+), 5 deletions(-) create mode 100644 packages/strapi-admin/config/policies/isAuthenticatedAdmin.js create mode 100644 packages/strapi-admin/controllers/__tests__/permission.test.js diff --git a/packages/strapi-admin/config/policies/isAuthenticatedAdmin.js b/packages/strapi-admin/config/policies/isAuthenticatedAdmin.js new file mode 100644 index 0000000000..0803cf2de7 --- /dev/null +++ b/packages/strapi-admin/config/policies/isAuthenticatedAdmin.js @@ -0,0 +1,9 @@ +'use strict'; + +module.exports = (ctx, next) => { + if (!ctx.state.isAuthenticatedAdmin) { + throw strapi.errors.forbidden(); + } + + return next(); +}; diff --git a/packages/strapi-admin/config/routes.json b/packages/strapi-admin/config/routes.json index 84d6ed952c..d0dc0f3886 100644 --- a/packages/strapi-admin/config/routes.json +++ b/packages/strapi-admin/config/routes.json @@ -198,9 +198,14 @@ { "method": "GET", "path": "/permissions", - "handler": "permission.getAll", + "handler": "permission.getAll" + }, + { + "method": "POST", + "path": "/permissions/check", + "handler": "permission.check", "config": { - "policies": [] + "policies": ["admin::isAuthenticatedAdmin"] } } ] diff --git a/packages/strapi-admin/controllers/__tests__/permission.test.js b/packages/strapi-admin/controllers/__tests__/permission.test.js new file mode 100644 index 0000000000..ec565c96d9 --- /dev/null +++ b/packages/strapi-admin/controllers/__tests__/permission.test.js @@ -0,0 +1,125 @@ +'use strict'; + +const permissionController = require('../permission'); + +const createContext = ({ params = {}, query = {}, body = {} }, overrides = {}) => ({ + params, + query, + request: { + body, + }, + ...overrides, +}); + +describe('Permission Controller', () => { + const localTestData = { + permissions: { + valid: [ + { action: 'read', subject: 'article', field: 'title' }, + { action: 'read', subject: 'article' }, + { action: 'read' }, + ], + invalid: [ + { action: {}, subject: '', field: '' }, + { subject: 'article', field: 'title' }, + { action: 'read', subject: {}, field: 'title' }, + { action: 'read', subject: 'article', field: {} }, + { action: 'read', subject: 'article', field: 'title', foo: 'bar' }, + ], + }, + ability: { + can: jest.fn(() => true), + }, + badRequest: jest.fn(), + }; + + global.strapi = { + admin: { + services: { + permission: { + engine: { + checkMany: jest.fn(ability => permissions => { + return permissions.map(({ action, subject, field }) => + ability.can(action, subject, field) + ); + }), + }, + }, + }, + }, + }; + + afterEach(async () => { + jest.clearAllMocks(); + }); + + describe('Check Many Permissions', () => { + test('Invalid Permission Shape (bad type for action)', async () => { + const ctx = createContext( + { body: { permissions: [localTestData.permissions.invalid[0]] } }, + { state: { userAbility: localTestData.ability }, badRequest: localTestData.badRequest } + ); + + await permissionController.check(ctx); + + expect(localTestData.badRequest).toHaveBeenCalled(); + }); + + test('Invalid Permission Shape (missing required action)', async () => { + const ctx = createContext( + { body: { permissions: [localTestData.permissions.invalid[1]] } }, + { state: { userAbility: localTestData.ability }, badRequest: localTestData.badRequest } + ); + + await permissionController.check(ctx); + + expect(localTestData.badRequest).toHaveBeenCalled(); + }); + + test('Invalid Permission Shape (bad type for subject)', async () => { + const ctx = createContext( + { body: { permissions: [localTestData.permissions.invalid[2]] } }, + { state: { userAbility: localTestData.ability }, badRequest: localTestData.badRequest } + ); + + await permissionController.check(ctx); + + expect(localTestData.badRequest).toHaveBeenCalled(); + }); + + test('Invalid Permission Shape (bad type for field)', async () => { + const ctx = createContext( + { body: { permissions: [localTestData.permissions.invalid[3]] } }, + { state: { userAbility: localTestData.ability }, badRequest: localTestData.badRequest } + ); + + await permissionController.check(ctx); + + expect(localTestData.badRequest).toHaveBeenCalled(); + }); + + test('Invalid Permission Shape (unrecognized foo param)', async () => { + const ctx = createContext( + { body: { permissions: [localTestData.permissions.invalid[4]] } }, + { state: { userAbility: localTestData.ability }, badRequest: localTestData.badRequest } + ); + + await permissionController.check(ctx); + + expect(localTestData.badRequest).toHaveBeenCalled(); + }); + + test('Check Many Permissions', async () => { + const ctx = createContext( + { body: { permissions: localTestData.permissions.valid } }, + { state: { userAbility: localTestData.ability } } + ); + + await permissionController.check(ctx); + + expect(localTestData.ability.can).toHaveBeenCalled(); + expect(strapi.admin.services.permission.engine.checkMany).toHaveBeenCalled(); + expect(ctx.body.data).toHaveLength(localTestData.permissions.valid.length); + }); + }); +}); diff --git a/packages/strapi-admin/controllers/permission.js b/packages/strapi-admin/controllers/permission.js index c8f099053a..f6c459e0b3 100644 --- a/packages/strapi-admin/controllers/permission.js +++ b/packages/strapi-admin/controllers/permission.js @@ -1,8 +1,31 @@ 'use strict'; const { formatActionsBySections } = require('./formatters'); +const { validateCheckPermissionsInput } = require('../validation/permission'); module.exports = { + /** + * Check each permissions from `request.body.permissions` and returns an array of booleans + * @param {KoaContext} ctx - koa context + */ + async check(ctx) { + const { body: input } = ctx.request; + + try { + await validateCheckPermissionsInput(input); + } catch (err) { + return ctx.badRequest('ValidationError', err); + } + + const checkPermissions = strapi.admin.services.permission.engine.checkMany( + ctx.state.userAbility + ); + + ctx.body = { + data: checkPermissions(input.permissions), + }; + }, + /** * Returns every permissions, in nested format * @param {KoaContext} ctx - koa context diff --git a/packages/strapi-admin/middlewares/auth/index.js b/packages/strapi-admin/middlewares/auth/index.js index 27e175b01a..ad349fa59b 100644 --- a/packages/strapi-admin/middlewares/auth/index.js +++ b/packages/strapi-admin/middlewares/auth/index.js @@ -44,6 +44,9 @@ module.exports = strapi => ({ ctx.state.admin = admin; ctx.state.user = admin; + ctx.state.userAbility = await strapi.admin.services.permission.engine.generateUserAbility( + admin + ); ctx.state.isAuthenticatedAdmin = true; return next(); } diff --git a/packages/strapi-admin/validation/permission.js b/packages/strapi-admin/validation/permission.js index f511d6ec0d..91369f1cc7 100644 --- a/packages/strapi-admin/validation/permission.js +++ b/packages/strapi-admin/validation/permission.js @@ -69,6 +69,25 @@ const updatePermissionsSchema = yup .required() .noUnknown(); +const checkPermissionsSchema = yup.object().shape({ + permissions: yup.array().of( + yup + .object() + .shape({ + action: yup.string().required(), + subject: yup.string(), + field: yup.string(), + }) + .noUnknown() + ), +}); + +const validateCheckPermissionsInput = data => { + return checkPermissionsSchema + .validate(data, { strict: true, abortEarly: false }) + .catch(handleReject); +}; + const validatedUpdatePermissionsInput = data => { return updatePermissionsSchema .validate(data, { strict: true, abortEarly: true }) @@ -110,4 +129,5 @@ const validatePermissionsExist = data => { module.exports = { validatedUpdatePermissionsInput, validatePermissionsExist, + validateCheckPermissionsInput, }; diff --git a/packages/strapi-utils/lib/policy.js b/packages/strapi-utils/lib/policy.js index 4c1e129584..7d5d74d1e8 100644 --- a/packages/strapi-utils/lib/policy.js +++ b/packages/strapi-utils/lib/policy.js @@ -14,6 +14,10 @@ const get = (policy, plugin, apiName) => { return parsePolicy(getPluginPolicy(policy)); } + if (adminPolicyExists(policy)) { + return parsePolicy(getAdminPolicy(policy)); + } + if (APIPolicyExists(policy)) { return parsePolicy(getAPIPolicy(policy)); } @@ -63,6 +67,7 @@ const parsePolicy = policy => { const GLOBAL_PREFIX = 'global::'; const PLUGIN_PREFIX = 'plugins::'; +const ADMIN_PREFIX = 'admin::'; const APPLICATION_PREFIX = 'application::'; const getPolicyIn = (container, policy) => { @@ -90,12 +95,20 @@ const getPluginPolicy = policy => { return getPolicyIn(_.get(strapi, ['plugins', plugin]), policyName); }; -const pluginPolicyExists = policy => { - return isPluginPolicy(policy) && !_.isUndefined(getPluginPolicy(policy)); -}; +const pluginPolicyExists = policy => + isPluginPolicy(policy) && !_.isUndefined(getPluginPolicy(policy)); const isPluginPolicy = policy => _.startsWith(policy, PLUGIN_PREFIX); +const getAdminPolicy = policy => { + const strippedPolicy = policy.replace(ADMIN_PREFIX, ''); + return getPolicyIn(_.get(strapi, 'admin'), strippedPolicy); +}; + +const isAdminPolicy = policy => _.startsWith(policy, ADMIN_PREFIX); + +const adminPolicyExists = policy => isAdminPolicy(policy) && !_.isUndefined(getAdminPolicy(policy)); + const getAPIPolicy = policy => { const [, policyWithoutPrefix] = policy.split('::'); const [api = '', policyName = ''] = policyWithoutPrefix.split('.');