diff --git a/packages/core/strapi/lib/Strapi.js b/packages/core/strapi/lib/Strapi.js index 30c77ef821..f7c9c37357 100644 --- a/packages/core/strapi/lib/Strapi.js +++ b/packages/core/strapi/lib/Strapi.js @@ -454,7 +454,7 @@ class Strapi { await this.runLifecyclesFunctions(LIFECYCLES.BOOTSTRAP); // TODO: is this the best place for this? - await this.contentAPI.permissions.syncActions(); + await this.contentAPI.permissions.registerActions(); this.cron.start(); diff --git a/packages/core/strapi/lib/services/__tests__/content-api-permissions.test.js b/packages/core/strapi/lib/services/__tests__/content-api-permissions.test.js new file mode 100644 index 0000000000..06b327d636 --- /dev/null +++ b/packages/core/strapi/lib/services/__tests__/content-api-permissions.test.js @@ -0,0 +1,291 @@ +'use strict'; + +const createContentAPI = require('../content-api'); + +describe('Content API - Permissions', () => { + const bindToContentAPI = (action) => { + Object.assign(action, { [Symbol.for('__type__')]: ['content-api'] }); + return action; + }; + + describe('Get Actions Map', () => { + test('When no API are defined, it should return an empty object', () => { + global.strapi = {}; + + const contentAPI = createContentAPI(global.strapi); + const actionsMap = contentAPI.permissions.getActionsMap(); + + expect(actionsMap).toEqual({}); + }); + + test('When no controller are defined for an API, it should ignore the API', () => { + global.strapi = { + api: { + foo: {}, + bar: {}, + }, + }; + + const contentAPI = createContentAPI(global.strapi); + const actionsMap = contentAPI.permissions.getActionsMap(); + + expect(actionsMap).toEqual({}); + }); + + test(`Do not register controller if they're not bound to the content API`, () => { + const actionC = () => {}; + Object.assign(actionC, { [Symbol.for('__type__')]: ['admin-api'] }); + + global.strapi = { + api: { + foo: { + controllers: { + controllerA: { + actionA: bindToContentAPI(() => {}), + actionB() {}, + actionC, + }, + }, + }, + }, + }; + + const contentAPI = createContentAPI(global.strapi); + const actionsMap = contentAPI.permissions.getActionsMap(); + + expect(actionsMap).toEqual({ + 'api::foo': { controllers: { controllerA: ['actionA'] } }, + }); + }); + + test('Creates and populate a map of actions from APIs and plugins', () => { + global.strapi = { + api: { + foo: { + controllers: { + controllerA: { + actionA: bindToContentAPI(() => {}), + actionB: bindToContentAPI(() => {}), + }, + }, + }, + bar: { + controllers: { + controllerA: { + actionA: bindToContentAPI(() => {}), + actionB: bindToContentAPI(() => {}), + }, + }, + }, + foobar: { + controllers: { + controllerA: { + actionA: bindToContentAPI(() => {}), + actionB: bindToContentAPI(() => {}), + }, + controllerB: { + actionC: bindToContentAPI(() => {}), + actionD: bindToContentAPI(() => {}), + }, + }, + }, + }, + plugins: { + foo: { + controllers: { + controllerA: { + actionA: bindToContentAPI(() => {}), + actionB: bindToContentAPI(() => {}), + }, + }, + }, + bar: { + controllers: { + controllerA: { + actionA: bindToContentAPI(() => {}), + actionB: bindToContentAPI(() => {}), + }, + }, + }, + }, + }; + + const contentAPI = createContentAPI(global.strapi); + const actionsMap = contentAPI.permissions.getActionsMap(); + + expect(actionsMap).toEqual({ + 'api::foo': { controllers: { controllerA: ['actionA', 'actionB'] } }, + 'api::bar': { controllers: { controllerA: ['actionA', 'actionB'] } }, + 'api::foobar': { + controllers: { + controllerA: ['actionA', 'actionB'], + controllerB: ['actionC', 'actionD'], + }, + }, + 'plugin::foo': { controllers: { controllerA: ['actionA', 'actionB'] } }, + 'plugin::bar': { controllers: { controllerA: ['actionA', 'actionB'] } }, + }); + }); + }); + + describe('Register Actions', () => { + beforeEach(() => { + global.strapi = { + api: { + foo: { + controllers: { + controllerA: { + actionA: bindToContentAPI(() => {}), + actionB: bindToContentAPI(() => {}), + }, + controllerB: { + actionC: bindToContentAPI(() => {}), + actionD: bindToContentAPI(() => {}), + }, + }, + }, + }, + plugins: { + foo: { + controllers: { + controllerA: { + actionA: bindToContentAPI(() => {}), + }, + }, + }, + }, + }; + }); + + test('The action provider should holds every action from APIs and plugins', async () => { + const contentAPI = createContentAPI(global.strapi); + + await contentAPI.permissions.registerActions(); + + const values = contentAPI.permissions.providers.action.values(); + + expect(values).toEqual([ + { + uid: 'api::foo.controllerA.actionA', + api: 'api::foo', + controller: 'controllerA', + action: 'actionA', + }, + { + uid: 'api::foo.controllerA.actionB', + api: 'api::foo', + controller: 'controllerA', + action: 'actionB', + }, + { + uid: 'api::foo.controllerB.actionC', + api: 'api::foo', + controller: 'controllerB', + action: 'actionC', + }, + { + uid: 'api::foo.controllerB.actionD', + api: 'api::foo', + controller: 'controllerB', + action: 'actionD', + }, + { + uid: 'plugin::foo.controllerA.actionA', + api: 'plugin::foo', + controller: 'controllerA', + action: 'actionA', + }, + ]); + }); + + test('Call registerActions twice should throw a duplicate error', async () => { + const contentAPI = createContentAPI(global.strapi); + + await contentAPI.permissions.registerActions(); + + expect(() => contentAPI.permissions.registerActions()).rejects.toThrowError( + 'Duplicated item key: api::foo.controllerA.actionA' + ); + }); + }); + + describe('Providers', () => { + test('You should not be able to register action once strapi is loaded', () => { + global.strapi.isLoaded = true; + + const contentAPI = createContentAPI(global.strapi); + + // Actions + expect(() => + contentAPI.permissions.providers.action.register('foo', {}) + ).rejects.toThrowError(`You can't register new actions outside the bootstrap function.`); + + // Conditions + expect(() => + contentAPI.permissions.providers.condition.register({ name: 'myCondition' }) + ).rejects.toThrowError(`You can't register new conditions outside the bootstrap function.`); + + // Register Actions + expect(() => contentAPI.permissions.registerActions()).rejects.toThrowError( + `You can't register new actions outside the bootstrap function.` + ); + }); + }); + + describe('Engine', () => { + test('Engine warns when registering an unknown action', async () => { + global.strapi = { + log: { + debug: jest.fn(), + }, + }; + + const contentAPI = createContentAPI(); + + const ability = await contentAPI.permissions.engine.generateAbility([{ action: 'foo' }]); + + expect(ability.rules).toHaveLength(0); + expect(global.strapi.log.debug).toHaveBeenCalledWith( + `Unknown action "foo" supplied when registering a new permission` + ); + }); + + test('Engine filter out invalid action when generating an ability', async () => { + global.strapi = { + log: { + debug: jest.fn(), + }, + + api: { + foo: { + controllers: { + bar: { foobar: bindToContentAPI(() => {}) }, + }, + }, + }, + }; + + const contentAPI = createContentAPI(global.strapi); + + await contentAPI.permissions.registerActions(); + + const ability = await contentAPI.permissions.engine.generateAbility([ + { action: 'foo' }, + { action: 'api::foo.bar.foobar' }, + ]); + + expect(ability.rules).toHaveLength(1); + expect(ability.rules).toEqual([ + { + action: 'api::foo.bar.foobar', + subject: 'all', + }, + ]); + + expect(global.strapi.log.debug).toHaveBeenCalledTimes(1); + expect(global.strapi.log.debug).toHaveBeenCalledWith( + `Unknown action "foo" supplied when registering a new permission` + ); + }); + }); +}); diff --git a/packages/core/strapi/lib/services/content-api/index.js b/packages/core/strapi/lib/services/content-api/index.js index d4c3f6dbe8..4f03728cf5 100644 --- a/packages/core/strapi/lib/services/content-api/index.js +++ b/packages/core/strapi/lib/services/content-api/index.js @@ -1,65 +1,7 @@ 'use strict'; -const { uniq } = require('lodash'); +const _ = require('lodash'); const permissions = require('./permissions'); -/** - * Create a content API container that holds logic, tools and utils. (eg: permissions, ...) - */ -const createContentAPI = (/* strapi */) => { - const syncActions = async () => { - /** - * NOTE: For some reason, this doesn't seem to be necessary because all the routes exist - * createActionProvider uses a providerFactory, which seems to already include everything, and when we try - * to register our actions we get an error that the keys already exist - * Could providerFactory not be providing a new provider, and instead sharing the registry with everything that uses it? - * - * If this isn't an issue to fix and is expected, we don't need the route registration code below and it should be removed - * */ - // Start of route registration - const apiRoutesName = Object.values(strapi.api) - .map((api) => api.routes) - .reduce((acc, routesMap) => { - const routes = Object.values(routesMap) - // Only content api routes - .filter((p) => p.type === 'content-api') - // Resolve every handler name for each route - .reduce((a, p) => a.concat(p.routes.map((i) => i.handler)), []); - return acc.concat(routes); - }, []); - const pluginsRoutesname = Object.values(strapi.plugins) - .map((plugin) => plugin.routes['content-api'] || {}) - .map((p) => (p.routes || []).map((i) => i.handler)) - .flat(); - const actions = apiRoutesName.concat(pluginsRoutesname); - Promise.all( - uniq(actions).map((action) => - providers.action.register(action).catch(() => { - // console.log('Key already exists', action); - }) - ) - ); - }; - // End of route registration - - // Add providers - const providers = { - action: permissions.providers.createActionProvider(), - condition: permissions.providers.createConditionProvider(), - }; - - // create permission engine - const engine = permissions - .createPermissionEngine({ providers }) - .on('before-format::validate.permission', createValidatePermissionHandler(providers.action)); - - return { - permissions: { - engine, - providers, - syncActions, - }, - }; -}; /** * Creates an handler which check that the permission's action exists in the action registry @@ -78,4 +20,96 @@ const createValidatePermissionHandler = } }; +/** + * Create a content API container that holds logic, tools and utils. (eg: permissions, ...) + */ +const createContentAPI = (strapi) => { + // Add providers + const providers = { + action: permissions.providers.createActionProvider(), + condition: permissions.providers.createConditionProvider(), + }; + + const getActionsMap = () => { + const actionMap = {}; + + const isContentApi = (action) => { + if (!_.has(action, Symbol.for('__type__'))) { + return false; + } + + return action[Symbol.for('__type__')].includes('content-api'); + }; + + const registerAPIsActions = (apis, source) => { + _.forEach(apis, (api, apiName) => { + const controllers = _.reduce( + api.controllers, + (acc, controller, controllerName) => { + const contentApiActions = _.pickBy(controller, isContentApi); + + if (_.isEmpty(contentApiActions)) { + return acc; + } + + acc[controllerName] = Object.keys(contentApiActions); + + return acc; + }, + {} + ); + + if (!_.isEmpty(controllers)) { + actionMap[`${source}::${apiName}`] = { controllers }; + } + }); + }; + + registerAPIsActions(strapi.api, 'api'); + registerAPIsActions(strapi.plugins, 'plugin'); + + return actionMap; + }; + + const registerActions = async () => { + const actionsMap = getActionsMap(); + + // For each API + for (const [api, value] of Object.entries(actionsMap)) { + const { controllers } = value; + + // Register controllers methods as actions + for (const [controller, actions] of Object.entries(controllers)) { + // Register each action individually + await Promise.all( + actions.map((action) => { + const actionUID = `${api}.${controller}.${action}`; + + return providers.action.register(actionUID, { + api, + controller, + action, + uid: actionUID, + }); + }) + ); + } + } + }; + + // create permission engine + const engine = permissions + .createPermissionEngine({ providers }) + .on('before-format::validate.permission', createValidatePermissionHandler(providers.action)); + + return { + permissions: { + engine, + providers, + registerActions, + getActionsMap, + }, + }; +}; + module.exports = createContentAPI; diff --git a/packages/core/strapi/lib/services/content-api/permissions/providers/action.js b/packages/core/strapi/lib/services/content-api/permissions/providers/action.js index e44cce17a2..ead6855351 100644 --- a/packages/core/strapi/lib/services/content-api/permissions/providers/action.js +++ b/packages/core/strapi/lib/services/content-api/permissions/providers/action.js @@ -8,12 +8,12 @@ module.exports = (options = {}) => { return { ...provider, - async register(action) { + async register(action, payload) { if (strapi.isLoaded) { throw new Error(`You can't register new actions outside the bootstrap function.`); } - return provider.register(action, { name: action }); + return provider.register(action, payload); }, }; }; diff --git a/packages/plugins/users-permissions/server/services/users-permissions.js b/packages/plugins/users-permissions/server/services/users-permissions.js index 65eb9cafd0..1389cd582e 100644 --- a/packages/plugins/users-permissions/server/services/users-permissions.js +++ b/packages/plugins/users-permissions/server/services/users-permissions.js @@ -171,10 +171,6 @@ module.exports = ({ strapi }) => ({ const toDelete = _.difference(permissionsFoundInDB, allActions); - // Register actions into the content API action provider - // TODO: do this in the content API bootstrap phase instead - allActions.forEach((action) => strapi.contentAPI.permissions.providers.action.register(action)); - await Promise.all( toDelete.map((action) => { return strapi.query('plugin::users-permissions.permission').delete({ where: { action } });