Refactor policy util and error handling

Signed-off-by: Alexandre Bodin <bodin.alex@gmail.com>

packages/strapi-plugin-graphql/services/Aggregator.js

@@ -526,13 +526,19 @@ const formatModelConnectionsGQL = function(
             }),
           ];
 
+          try {
+            policiesFn.push(
               policyUtils.get(
                 'plugins.users-permissions.permissions',
                 plugin,
-            policiesFn,
-            `GraphQL connection "${name}" `,
                 name
+              )
             );
+          } catch (error) {
+            strapi.stopWithError(
+              `Error building graphql connection "${queryName}": ${error.message}`
+            );
+          }
 
           // Execute policies stack.
           const policy = await compose(policiesFn)(ctx);

packages/strapi-plugin-graphql/services/Mutation.js

@@ -155,15 +155,15 @@ module.exports = {
     }
 
     // Populate policies.
-    policies.forEach(policy =>
+    policies.forEach(policyName => {
-      policyUtils.get(
+      try {
-        policy,
+        policiesFn.push(policyUtils.get(policyName, plugin, name));
-        plugin,
+      } catch (error) {
-        policiesFn,
+        strapi.stopWithError(
-        `GraphQL query "${queryName}"`,
+          `Error building graphql mutation "${queryName}": ${error.message}`
-        name
-      )
         );
+      }
+    });
 
     return async (obj, options, graphqlCtx) => {
       const { context } = graphqlCtx;

packages/strapi-plugin-graphql/services/Query.js

@@ -227,15 +227,15 @@ module.exports = {
     }
 
     // Populate policies.
-    policies.forEach(policy =>
+    policies.forEach(policyName => {
-      policyUtils.get(
+      try {
-        policy,
+        policiesFn.push(policyUtils.get(policyName, plugin, name));
-        plugin,
+      } catch (error) {
-        policiesFn,
+        strapi.stopWithError(
-        `GraphQL query "${queryName}"`,
+          `Error building graphql query "${queryName}": ${error.message}`
-        name
-      )
         );
+      }
+    });
 
     return async (obj, options = {}, graphqlContext) => {
       const { context } = graphqlContext;

packages/strapi-utils/lib/__tests__/policy.test.js

@@ -0,0 +1,83 @@
+'use strict';
+
+const policyUtils = require('../policy');
+
+describe('Policy util', () => {
+  describe('Get policy', () => {
+    test('Throws on policy not found', () => {
+      expect(() => policyUtils.get('undefined')).toThrow();
+    });
+
+    test('Retrieves global policy', () => {
+      const policyFn = () => {};
+
+      // init global strapi
+      global.strapi = {
+        config: {
+          policies: {
+            'test-policy': policyFn,
+          },
+        },
+      };
+
+      expect(policyUtils.get('global.test-policy')).toBe(policyFn);
+    });
+
+    test('Retrieves a global plugin policy', () => {
+      const policyFn = () => {};
+
+      global.strapi = {
+        plugins: {
+          'test-plugin': {
+            config: {
+              policies: {
+                'test-policy': policyFn,
+              },
+            },
+          },
+        },
+      };
+
+      expect(() => policyUtils.get('test-plugin.test-policy')).toThrow();
+      expect(policyUtils.get('plugins.test-plugin.test-policy')).toBe(policyFn);
+    });
+
+    test('Retrieves a plugin policy locally', () => {
+      const policyFn = () => {};
+
+      global.strapi = {
+        plugins: {
+          'test-plugin': {
+            config: {
+              policies: {
+                'test-policy': policyFn,
+              },
+            },
+          },
+        },
+      };
+
+      expect(policyUtils.get('test-policy', 'test-plugin')).toBe(policyFn);
+    });
+
+    test('Retrieves an api policy locally', () => {
+      const policyFn = () => {};
+
+      global.strapi = {
+        api: {
+          'test-api': {
+            config: {
+              policies: {
+                'test-policy': policyFn,
+              },
+            },
+          },
+        },
+      };
+
+      expect(policyUtils.get('test-policy', undefined, 'test-api')).toBe(
+        policyFn
+      );
+    });
+  });
+});

packages/strapi-utils/lib/policy.js

@@ -1,110 +1,34 @@
-// Public dependencies.
+/**
+ * Policies util
+ */
+'use strict';
+
 const _ = require('lodash');
-/* eslint-disable prefer-template */
-module.exports = {
-  get(policy, plugin, policies = [], endpoint, currentApiName) {
-    // Define global policy prefix.
-    const globalPolicyPrefix = 'global.';
-    const pluginPolicyPrefix = 'plugins.';
-    const policySplited = policy.split('.');
 
-    // Looking for global policy or namespaced.
+const get = (policy, plugin, apiName) => {
-    if (
+  if (globalPolicyExists(policy)) {
-      _.startsWith(policy, globalPolicyPrefix, 0) &&
+    return parsePolicy(getGlobalPolicy(policy));
-      !_.isUndefined(
-        _.get(
-          strapi.config.policies,
-          policy.replace(globalPolicyPrefix, '').toLowerCase()
-        )
-      )
-    ) {
-      // Global policy.
-      return policies.push(
-        this.parsePolicy(
-          _.get(
-            strapi.config.policies,
-            policy.replace(globalPolicyPrefix, '').toLowerCase()
-          )
-        )
-      );
-    } else if (
-      _.startsWith(policy, pluginPolicyPrefix, 0) &&
-      strapi.plugins[policySplited[1]] &&
-      !_.isUndefined(
-        _.get(
-          strapi.plugins,
-          policySplited[1] +
-            '.config.policies.' +
-            policySplited[2].toLowerCase()
-        )
-      )
-    ) {
-      // Plugin's policies can be used from app APIs with a specific syntax (`plugins.pluginName.policyName`).
-      return policies.push(
-        this.parsePolicy(
-          _.get(
-            strapi.plugins,
-            policySplited[1] +
-              '.config.policies.' +
-              policySplited[2].toLowerCase()
-          )
-        )
-      );
-    } else if (
-      !_.startsWith(policy, globalPolicyPrefix, 0) &&
-      plugin &&
-      !_.isUndefined(
-        _.get(
-          strapi.plugins,
-          plugin + '.config.policies.' + policy.toLowerCase()
-        )
-      )
-    ) {
-      // Plugin policy used in the plugin itself.
-      return policies.push(
-        this.parsePolicy(
-          _.get(
-            strapi.plugins,
-            plugin + '.config.policies.' + policy.toLowerCase()
-          )
-        )
-      );
-    } else if (
-      !_.startsWith(policy, globalPolicyPrefix, 0) &&
-      !_.isUndefined(
-        _.get(
-          strapi.api,
-          currentApiName + '.config.policies.' + policy.toLowerCase()
-        )
-      )
-    ) {
-      // API policy used in the API itself.
-      return policies.push(
-        this.parsePolicy(
-          _.get(
-            strapi.api,
-            currentApiName + '.config.policies.' + policy.toLowerCase()
-          )
-        )
-      );
   }
 
-    strapi.log.error(
+  if (pluginPolicyExists(policy)) {
-      `Ignored attempt to bind to ${endpoint} with unknown policy "${policy}"`
+    return parsePolicy(getPluginPolicy(policy));
-    );
-  },
-
-  parsePolicy(policy) {
-    if (_.isFunction(policy)) {
-      return policy;
   }
 
-    return policy.handler;
+  const pluginPolicy = `${PLUGIN_PREFIX}${plugin}.${policy}`;
-  },
 
-  // Middleware used for every routes.
+  if (!isGlobal(policy) && plugin && pluginPolicyExists(pluginPolicy)) {
-  // Expose the endpoint in `this`.
+    return parsePolicy(getPluginPolicy(pluginPolicy));
-  globalPolicy({ method, endpoint, controller, action, plugin }) {
+  }
+
+  const api = _.get(strapi.api, apiName);
+  if (!isGlobal(policy) && api && policyExistsIn(api, policy)) {
+    return parsePolicy(getPolicyIn(api, policy));
+  }
+
+  throw new Error(`Could not find policy "${policy}"`);
+};
+
+const globalPolicy = ({ method, endpoint, controller, action, plugin }) => {
   return async (ctx, next) => {
     ctx.request.route = {
       endpoint: `${method} ${endpoint}`,
@@ -117,5 +41,50 @@ module.exports = {
 
     await next();
   };
-  },
+};
+
+const parsePolicy = policy => {
+  if (_.isFunction(policy)) {
+    return policy;
+  }
+
+  return policy.handler;
+};
+
+const GLOBAL_PREFIX = 'global.';
+const PLUGIN_PREFIX = 'plugins.';
+
+const getPolicyIn = (container, policy) => {
+  return _.get(container, ['config', 'policies', policy.toLowerCase()]);
+};
+
+const policyExistsIn = (container, policy) => {
+  return !_.isUndefined(getPolicyIn(container, policy));
+};
+
+const isGlobal = policy => _.startsWith(policy, GLOBAL_PREFIX);
+
+const getGlobalPolicy = policy => {
+  const strippedPolicy = policy.replace(GLOBAL_PREFIX, '');
+  return getPolicyIn(strapi, strippedPolicy);
+};
+
+const globalPolicyExists = policy => {
+  return isGlobal(policy) && !_.isUndefined(getGlobalPolicy(policy));
+};
+
+const getPluginPolicy = policy => {
+  const [, plugin = '', policyName = ''] = policy.split('.');
+  return getPolicyIn(_.get(strapi, ['plugins', plugin]), policyName);
+};
+
+const pluginPolicyExists = policy => {
+  return isPluginPolicy(policy) && !_.isUndefined(getPluginPolicy(policy));
+};
+
+const isPluginPolicy = policy => _.startsWith(policy, PLUGIN_PREFIX);
+
+module.exports = {
+  get,
+  globalPolicy,
 };

packages/strapi/lib/core-api/index.js

@@ -25,8 +25,7 @@ function createCoreApi({ api, model }) {
 
   const controller = Object.assign(
     createController({ service, model }),
-    userController,
+    userController
-    { identity: userController.identity || _.upperFirst(modelName) }
   );
 
   return {

packages/strapi/lib/middlewares/router/utils/routerChecker.js

@@ -41,40 +41,34 @@ module.exports = strapi =>
       controller
     );
 
-    // Init policies array.
-    const policies = [];
-
     // Add the `globalPolicy`.
-    policies.push(
+    const globalPolicy = policyUtils.globalPolicy({
-      policyUtils.globalPolicy({
       controller: controllerKey,
       action: actionName,
       method,
       endpoint,
       plugin,
-      })
+    });
-    );
+
+    // Init policies array.
+    const policies = [globalPolicy];
+
+    let policyOption = _.get(value, 'config.policies');
 
     // Allow string instead of array of policies.
-    if (
+    if (_.isString(policyOption) && !_.isEmpty(policyOption)) {
-      !_.isArray(_.get(value, 'config.policies')) &&
+      policyOption = [policyOption];
-      !_.isEmpty(_.get(value, 'config.policies'))
-    ) {
-      value.config.policies = [value.config.policies];
     }
 
-    if (
+    if (_.isArray(policyOption)) {
-      _.isArray(_.get(value, 'config.policies')) &&
+      policyOption.forEach(policyName => {
-      !_.isEmpty(_.get(value, 'config.policies'))
+        try {
-    ) {
+          policies.push(policyUtils.get(policyName, plugin, currentApiName));
-      _.forEach(value.config.policies, policy => {
+        } catch (error) {
-        policyUtils.get(
+          strapi.stopWithError(
-          policy,
+            `Error creating endpoint ${method} ${endpoint}: ${error.message}`
-          plugin,
-          policies,
-          `${method} ${endpoint}`,
-          currentApiName
           );
+        }
       });
     }