Add category to conditions

Signed-off-by: Convly <jean-sebastien.herbaux@epitech.eu>

packages/strapi-admin/config/functions/bootstrap.js

@@ -29,9 +29,14 @@ const cleanPermissionInDatabase = async () => {
 const registerAdminConditions = () => {
   const { conditionProvider } = strapi.admin.services.permission;
 
-  conditionProvider.registerMany({
+  conditionProvider.registerMany([
-    'strapi-admin::isOwner': user => ({ 'strapi_created_by.id': user.id }),
+    {
-  });
+      name: 'isOwner',
+      plugin: 'admin',
+      category: 'default',
+      handler: user => ({ 'strapi_created_by.id': user.id }),
+    },
+  ]);
 };
 
 module.exports = async () => {

packages/strapi-admin/controllers/permission.js

@@ -1,5 +1,6 @@
 'use strict';
 
+const _ = require('lodash');
 const { formatActionsBySections } = require('./formatters');
 const { validateCheckPermissionsInput } = require('../validation/permission');
 
@@ -36,7 +37,9 @@ module.exports = {
 
     ctx.body = {
       data: {
-        conditions,
+        conditions: conditions.map(condition =>
+          _.pick(condition, 'id', 'name', 'plugin', 'category')
+        ),
         sections: formatActionsBySections(allActions),
       },
     };

packages/strapi-admin/domain/condition.js

@@ -0,0 +1,24 @@
+'use strict';
+
+const getConditionId = ({ name, plugin }) => {
+  let id;
+
+  if (plugin === 'admin') {
+    id = `admin::${name}`;
+  } else if (plugin) {
+    id = `plugins::${plugin}.${name}`;
+  } else {
+    id = `application::${name}`;
+  }
+  return id;
+};
+
+const createCondition = condition => ({
+  ...condition,
+  id: getConditionId(condition),
+});
+
+module.exports = {
+  getConditionId,
+  createCondition,
+};

packages/strapi-admin/services/__tests__/permissions.conditions-provider.test.js

@@ -1,11 +1,31 @@
 'use strict';
 
+const _ = require('lodash');
 const createConditionProvider = require('../permission/condition-provider');
+const { createCondition, getConditionId } = require('../../domain/condition');
 
 describe('Condition Provider', () => {
   let provider;
+  const localTestData = {
+    conditions: [
+      {
+        name: 'foo',
+        plugin: 'test',
+        category: 'default',
+        handler: jest.fn(() => true),
+      },
+      {
+        name: 'john',
+        plugin: 'test',
+        category: 'default',
+        handler: jest.fn(() => false),
+      },
+    ],
+  };
 
   beforeEach(() => {
+    global.strapi = { isLoaded: false };
+
     provider = createConditionProvider();
 
     jest.spyOn(provider, 'register');
@@ -17,22 +37,33 @@ describe('Condition Provider', () => {
   });
 
   describe('Register', () => {
+    test('Cannot register if strapi is already loaded', () => {
+      global.strapi.isLoaded = true;
+
+      const condition = localTestData.conditions[0];
+
+      const registerFn = () => provider.register(condition);
+
+      expect(registerFn).toThrowError();
+    });
+
     test('Successfully register a new condition', () => {
-      const condition = { key: 'conditionName', value: jest.fn(() => true) };
+      const condition = localTestData.conditions[0];
 
-      provider.register(condition.key, condition.value);
+      provider.register(condition);
 
-      const res = provider.get(condition.key);
+      const res = provider.get(condition.name, condition.plugin);
 
-      expect(provider.has).toHaveBeenCalledWith(condition.key);
+      expect(provider.has).toHaveBeenCalledWith(condition.name, condition.plugin);
-      expect(res).toBe(condition.value);
+      expect(res).toMatchObject(condition);
-      expect(res()).toBeTruthy();
+      expect(res.handler()).toBe(true);
-      expect(condition.value).toHaveBeenCalled();
+      expect(condition.handler).toHaveBeenCalled();
     });
 
     test('The condition already exists', () => {
-      const key = 'conditionName';
+      const condition = localTestData.conditions[0];
-      const registerFn = () => provider.register(key, {});
+
+      const registerFn = () => provider.register(condition);
 
       registerFn();
 
@@ -43,34 +74,28 @@ describe('Condition Provider', () => {
 
   describe('Registers Many', () => {
     test('Registers many conditions successfully', () => {
-      const conditions = {
+      const conditions = localTestData.conditions;
-        foo: jest.fn(() => 'bar'),
-        john: jest.fn(() => 'doe'),
-      };
 
       provider.registerMany(conditions);
 
-      const resFoo = provider.get('foo');
+      const resFoo = provider.get('foo', 'test');
-      const resJohn = provider.get('john');
+      const resJohn = provider.get('john', 'test');
 
       expect(provider.register).toHaveBeenCalledTimes(2);
       expect(provider.has).toHaveBeenCalledTimes(2);
 
-      expect(resFoo).toBe(conditions.foo);
+      expect(resFoo).toMatchObject(createCondition(conditions[0]));
-      expect(resJohn).toBe(conditions.john);
+      expect(resJohn).toMatchObject(createCondition(conditions[1]));
 
-      expect(resFoo()).toBe('bar');
+      expect(resFoo.handler()).toBe(true);
-      expect(resJohn()).toBe('doe');
+      expect(resJohn.handler()).toBe(false);
 
-      expect(conditions.foo).toHaveBeenCalled();
+      expect(conditions[0].handler).toHaveBeenCalled();
-      expect(conditions.john).toHaveBeenCalled();
+      expect(conditions[1].handler).toHaveBeenCalled();
     });
 
     test('Fails to register already existing conditions', () => {
-      const conditions = {
+      const conditions = localTestData.conditions;
-        foo: {},
-        john: {},
-      };
 
       const registerFn = () => provider.registerMany(conditions);
 
@@ -83,54 +108,46 @@ describe('Condition Provider', () => {
 
   describe('Conditions', () => {
     test('Returns an array of all the conditions key', () => {
-      const conditions = {
+      const conditions = localTestData.conditions;
-        foo: {},
+
-        bar: {},
+      const expected = ['plugins::test.foo', 'plugins::test.john'];
-      };
-      const expected = ['bar', 'foo'];
 
       provider.registerMany(conditions);
 
-      expect(provider.getAll().sort()).toMatchObject(expected);
+      expect(
+        provider
+          .getAll()
+          .map(_.property('id'))
+          .sort()
+      ).toMatchObject(expected);
     });
   });
 
   describe('Has', () => {
     test('The key exists', () => {
-      const key = 'foo';
+      const condition = localTestData.conditions[0];
-      provider.register(key, {});
 
-      expect(provider.has(key)).toBeTruthy();
+      provider.register(condition);
+
+      expect(provider.has(condition.name, condition.plugin)).toBeTruthy();
     });
 
     test(`The key doesn't exists`, () => {
-      const key = 'foo';
+      const { name, plugin } = localTestData.conditions[1];
 
-      expect(provider.has(key)).toBeFalsy();
+      expect(provider.has(name, plugin)).toBeFalsy();
     });
   });
 
-  describe('Delete', () => {
+  describe('GetById', () => {
-    test('Delete existing condition', () => {
+    test('Successfully get a condition by its ID', () => {
-      const key = 'foo';
+      const condition = localTestData.conditions[0];
 
-      provider.register(key);
+      provider.register(condition);
 
-      expect(provider.getAll()).toHaveLength(1);
+      const res = provider.getById(getConditionId(condition));
 
-      provider.delete(key);
+      expect(res).toMatchObject(createCondition(condition));
-
-      expect(provider.has).toHaveBeenCalledWith(key);
-      expect(provider.getAll()).toHaveLength(0);
-    });
-
-    test('Do nothing when the key does not exists', () => {
-      const key = 'foo';
-
-      provider.delete(key);
-
-      expect(provider.has).toHaveBeenCalledWith(key);
-      expect(provider.getAll()).toHaveLength(0);
     });
   });
 });

packages/strapi-admin/services/__tests__/permissions.engine.test.js

@@ -30,40 +30,74 @@ describe('Permissions Engine', () => {
     roles: {
       1: {
         permissions: [
-          { action: 'read', subject: 'article', fields: ['**'], conditions: ['isBob'] },
+          {
-          { action: 'read', subject: 'user', fields: ['title'], conditions: ['isAdmin'] },
+            action: 'read',
+            subject: 'article',
+            fields: ['**'],
+            conditions: ['plugins::test.isBob'],
+          },
+          {
+            action: 'read',
+            subject: 'user',
+            fields: ['title'],
+            conditions: ['plugins::test.isAdmin'],
+          },
         ],
       },
       2: {
-        permissions: [{ action: 'post', subject: 'article', fields: ['*'], conditions: ['isBob'] }],
+        permissions: [
+          {
+            action: 'post',
+            subject: 'article',
+            fields: ['*'],
+            conditions: ['plugins::test.isBob'],
+          },
+        ],
       },
       3: {
         permissions: [
-          { action: 'read', subject: 'user', fields: ['title'], conditions: ['isContainedIn'] },
+          {
+            action: 'read',
+            subject: 'user',
+            fields: ['title'],
+            conditions: ['plugins::test.isContainedIn'],
+          },
         ],
       },
     },
-    conditions: {
+    conditions: [
-      isBob: async user => new Promise(resolve => resolve(user.firstname === 'Bob')),
+      {
-      isAdmin: user => user.title === 'admin',
+        plugin: 'test',
-      isCreatedBy: user => ({ created_by: user.firstname }),
+        name: 'isBob',
-      isContainedIn: { firstname: { $in: ['Alice', 'Foo'] } },
+        category: 'default',
-    },
+        handler: async user => new Promise(resolve => resolve(user.firstname === 'Bob')),
+      },
+      {
+        plugin: 'test',
+        name: 'isAdmin',
+        category: 'default',
+        handler: user => user.title === 'admin',
+      },
+      {
+        plugin: 'test',
+        name: 'isCreatedBy',
+        category: 'default',
+        handler: user => ({ created_by: user.firstname }),
+      },
+      {
+        plugin: 'test',
+        name: 'isContainedIn',
+        category: 'default',
+        handler: { firstname: { $in: ['Alice', 'Foo'] } },
+      },
+    ],
   };
 
   const getUser = name => localTestData.users[name];
 
   beforeEach(() => {
-    conditionProvider = createConditionProvider();
-    conditionProvider.registerMany(localTestData.conditions);
-
-    engine = createPermissionsEngine(conditionProvider);
-
-    jest.spyOn(engine, 'evaluatePermission');
-    jest.spyOn(engine, 'createRegisterFunction');
-    jest.spyOn(engine, 'generateAbilityCreatorFor');
-
     global.strapi = {
+      isLoaded: false,
       admin: {
         services: {
           permission: {
@@ -82,6 +116,15 @@ describe('Permissions Engine', () => {
         },
       },
     };
+
+    conditionProvider = createConditionProvider();
+    conditionProvider.registerMany(localTestData.conditions);
+
+    engine = createPermissionsEngine(conditionProvider);
+
+    jest.spyOn(engine, 'evaluatePermission');
+    jest.spyOn(engine, 'createRegisterFunction');
+    jest.spyOn(engine, 'generateAbilityCreatorFor');
   });
 
   afterEach(() => {
@@ -211,7 +254,7 @@ describe('Permissions Engine', () => {
         action: 'read',
         subject: 'article',
         fields: ['title'],
-        conditions: ['isAdmin'],
+        conditions: ['plugins::test.isAdmin'],
       };
       const user = getUser('alice');
       const registerFn = jest.fn();
@@ -231,7 +274,7 @@ describe('Permissions Engine', () => {
         action: 'read',
         subject: 'article',
         fields: ['title'],
-        conditions: ['isBob'],
+        conditions: ['plugins::test.isBob'],
       };
       const user = getUser('alice');
       const registerFn = jest.fn();
@@ -246,7 +289,7 @@ describe('Permissions Engine', () => {
         action: 'read',
         subject: 'article',
         fields: ['title'],
-        conditions: ['isCreatedBy'],
+        conditions: ['plugins::test.isCreatedBy'],
       };
       const user = getUser('alice');
       const registerFn = jest.fn();

packages/strapi-admin/services/permission/condition-provider.js

@@ -1,70 +1,75 @@
 'use strict';
 
 const _ = require('lodash');
+const { getConditionId, createCondition } = require('../../domain/condition');
 
 module.exports = () => {
   const _registry = new Map();
 
   return {
     /**
-     * Register a new condition with its associated unique key.
+     * Register a new condition
-     * @throws Error if the key already exists
+     * @throws Error if the conditionId already exists
-     * @param name
      * @param condition
      */
-    register(name, condition) {
+    register(condition) {
-      if (this.has(name)) {
+      const conditionId = getConditionId(condition);
-        throw new Error(
+
-          `Error while trying to add condition "${name}" to the registry. "${name}" already exists.`
+      if (strapi.isLoaded) {
-        );
+        throw new Error(`You can't register new conditions outside of the bootstrap function.`);
       }
 
-      _registry.set(name, condition);
+      if (this.has(condition.name, condition.plugin)) {
+        throw new Error(`Duplicated condition id: ${getConditionId(condition)}.`);
+      }
+
+      _registry.set(conditionId, createCondition(condition));
     },
 
     /**
      * Shorthand for batch-register operations.
-     * Internally calls `register` for each key/value couple.
+     * Internally calls `register` for each condition.
-     * @param conditionsMap
+     * @param conditions
      */
-    registerMany(conditionsMap) {
+    registerMany(conditions) {
-      _.each(conditionsMap, (value, key) => this.register(key, value));
+      _.each(conditions, this.register.bind(this));
-    },
-
-    /**
-     * Deletes a condition by its key
-     * @param key
-     */
-    delete(key) {
-      if (this.has(key)) {
-        _registry.delete(key);
-      }
-    },
-
-    /**
-     * Returns the keys of the conditions registry.
-     * @returns {string[]}
-     */
-    conditions() {
-      return Array.from(_registry.keys());
-    },
-
-    /**
-     * Get a condition by its key
-     * @param name
-     * @returns {any}
-     */
-    get(name) {
-      return _registry.get(name);
     },
 
     /**
      * Check if a key is already present in the registry
      * @param name
-     * @returns {boolean} true if the key is present in the registry, false otherwise.
+     * @param plugin
+     * @returns {boolean} true if the condition is present in the registry, false otherwise.
      */
-    has(name) {
+    has(name, plugin) {
-      return _registry.has(name);
+      return _registry.has(getConditionId({ name, plugin }));
+    },
+
+    /**
+     * Get a condition by its name and plugin
+     * @param {string} name
+     * @param {string} plugin
+     * @returns {any}
+     */
+    get(name, plugin) {
+      return _registry.get(getConditionId({ name, plugin }));
+    },
+
+    /**
+     * Get a condition by its id
+     * @param {string} id
+     * @returns {any}
+     */
+    getById(id) {
+      return _registry.get(id);
+    },
+
+    /**
+     * Returns all the registered conditions.
+     * @returns {any[]}
+     */
+    getAll() {
+      return Array.from(_registry.values());
     },
   };
 };

packages/strapi-admin/services/permission/engine.js

@@ -51,7 +51,10 @@ module.exports = conditionProvider => ({
     }
 
     // Replace each condition name by its associated value
-    const resolveConditions = map(conditionProvider.get);
+    const resolveConditions = map(conditionProvider.getById);
+
+    // Only keep the handler of each condition
+    const pickHandlers = map(_.property('handler'));
 
     // Filter conditions, only keeps objects and functions
     const filterValidConditions = filter(_.isObject);
@@ -76,6 +79,7 @@ module.exports = conditionProvider => ({
 
     await Promise.resolve(conditions)
       .then(resolveConditions)
+      .then(pickHandlers)
       .then(filterValidConditions)
       .then(evaluateConditions)
       .then(filterValidResults)