From e0dd6dc6029ae4ce50a24f0b01f32e3f39bcd0a6 Mon Sep 17 00:00:00 2001 From: Don Masakayan Date: Sun, 1 Sep 2019 02:34:16 +0800 Subject: [PATCH 1/5] Fixed issue where custom GraphQL Mutation resolvers never triggered policies --- packages/strapi-plugin-graphql/services/Mutation.js | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/packages/strapi-plugin-graphql/services/Mutation.js b/packages/strapi-plugin-graphql/services/Mutation.js index a9ca8c5990..802abeb801 100644 --- a/packages/strapi-plugin-graphql/services/Mutation.js +++ b/packages/strapi-plugin-graphql/services/Mutation.js @@ -23,7 +23,11 @@ module.exports = { // Extract custom resolver or type description. const { resolver: handler = {} } = _schema; - const queryName = `${action}${_.capitalize(name)}`; + let queryName = `${action}${_.capitalize(name)}`; + + if (_.has(handler, `Mutation.${action}`)) { + queryName = action; + } // Retrieve policies. const policies = _.get(handler, `Mutation.${queryName}.policies`, []); From 0bead70c2c5fa90b6cfccd793a001b26c21d324c Mon Sep 17 00:00:00 2001 From: Don Masakayan Date: Sun, 1 Sep 2019 02:39:34 +0800 Subject: [PATCH 2/5] Fixed issue where all policies triggered by GraphQL Mutations and Queries had ctx.state.user as undefined --- .../strapi-plugin-graphql/services/Mutation.js | 14 +++++++++++--- packages/strapi-plugin-graphql/services/Query.js | 13 ++++++++++--- 2 files changed, 21 insertions(+), 6 deletions(-) diff --git a/packages/strapi-plugin-graphql/services/Mutation.js b/packages/strapi-plugin-graphql/services/Mutation.js index 802abeb801..4c7dc44b11 100644 --- a/packages/strapi-plugin-graphql/services/Mutation.js +++ b/packages/strapi-plugin-graphql/services/Mutation.js @@ -29,9 +29,6 @@ module.exports = { queryName = action; } - // Retrieve policies. - const policies = _.get(handler, `Mutation.${queryName}.policies`, []); - // Retrieve resolverOf. const resolverOf = _.get(handler, `Mutation.${queryName}.resolverOf`, ''); @@ -158,10 +155,21 @@ module.exports = { ); } + // Retrieve policies. + let policies = []; + if (strapi.plugins['users-permissions']) { policies.push('plugins.users-permissions.permissions'); } + // Retrieve policies. + if (_.get(handler, `Mutation.${queryName}.policies`)) { + policies = _.concat( + policies, + _.get(handler, `Mutation.${queryName}.policies`) + ); + } + // Populate policies. policies.forEach(policy => policyUtils.get( diff --git a/packages/strapi-plugin-graphql/services/Query.js b/packages/strapi-plugin-graphql/services/Query.js index 2c70215086..c024a3c988 100644 --- a/packages/strapi-plugin-graphql/services/Query.js +++ b/packages/strapi-plugin-graphql/services/Query.js @@ -91,9 +91,6 @@ module.exports = { : pluralize.plural(name); } - // Retrieve policies. - const policies = _.get(handler, `Query.${queryName}.policies`, []); - // Retrieve resolverOf. const resolverOf = _.get(handler, `Query.${queryName}.resolverOf`, ''); @@ -235,10 +232,20 @@ module.exports = { ); } + // Retrieve policies. + let policies = []; + if (strapi.plugins['users-permissions']) { policies.push('plugins.users-permissions.permissions'); } + if (_.get(handler, `Query.${queryName}.policies`)) { + policies = _.concat( + policies, + _.get(handler, `Query.${queryName}.policies`) + ); + } + // Populate policies. policies.forEach(policy => policyUtils.get( From e0fea833a7d473afd38bb52de8fbc7c281af9ca6 Mon Sep 17 00:00:00 2001 From: Don Masakayan Date: Sun, 1 Sep 2019 03:27:24 +0800 Subject: [PATCH 3/5] Updated documentation so that GraphQL policies use correct 'policies' key instead of 'policy' --- docs/3.0.0-beta.x/guides/graphql.md | 15 +++++++++------ 1 file changed, 9 insertions(+), 6 deletions(-) diff --git a/docs/3.0.0-beta.x/guides/graphql.md b/docs/3.0.0-beta.x/guides/graphql.md index 4fb20835a5..7662a826cd 100644 --- a/docs/3.0.0-beta.x/guides/graphql.md +++ b/docs/3.0.0-beta.x/guides/graphql.md @@ -492,7 +492,7 @@ module.exports = { Query: { post: { description: 'Return a single post', - policy: ['plugins.users-permissions.isAuthenticated', 'isOwner'], // Apply the 'isAuthenticated' policy of the `Users & Permissions` plugin, then the 'isOwner' policy before executing the resolver. + policies: ['plugins.users-permissions.isAuthenticated', 'isOwner'], // Apply the 'isAuthenticated' policy of the `Users & Permissions` plugin, then the 'isOwner' policy before executing the resolver. }, posts: { description: 'Return a list of posts', // Add a description to the query. @@ -504,7 +504,7 @@ module.exports = { }, postsByTags: { description: 'Return the posts published by the author', - resolverOf: 'Post.findByTags', // Will apply the same policy on the custom resolver than the controller's action `findByTags`. + resolverOf: 'Post.findByTags', // Will apply the same policy on the custom resolver as the controller's action `findByTags`. resolver: (obj, options, ctx) => { // ctx is the context of the Koa request. await strapi.controllers.posts.findByTags(ctx); @@ -516,7 +516,7 @@ module.exports = { Mutation: { attachPostToAuthor: { description: 'Attach a post to an author', - policy: ['plugins.users-permissions.isAuthenticated', 'isOwner'], + policies: ['plugins.users-permissions.isAuthenticated', 'isOwner'], resolver: 'Post.attachToAuthor' } } @@ -677,7 +677,7 @@ module.exports = { Query: { posts: { description: 'Return a list of posts', - policy: [ + policies: [ 'plugins.users-permissions.isAuthenticated', 'isOwner', 'global.logging', @@ -687,7 +687,10 @@ module.exports = { Mutation: { createPost: { description: 'Create a new post', - policy: ['plugins.users-permissions.isAuthenticated', 'global.logging'], + policies: [ + 'plugins.users-permissions.isAuthenticated', + 'global.logging', + ], }, }, }, @@ -782,7 +785,7 @@ module.exports = { Query: { posts: { description: 'Return a list of posts by author', - resolverOf: 'Post.find', // Will apply the same policy on the custom resolver than the controller's action `find` located in `Post.js`. + resolverOf: 'Post.find', // Will apply the same policy on the custom resolver as the controller's action `find` located in `Post.js`. resolver: (obj, options, context) => { // You can return a raw JSON object or a promise. From 8d3d79fd18c1964f528192b473d50593d523f9e7 Mon Sep 17 00:00:00 2001 From: Don Masakayan Date: Mon, 2 Sep 2019 17:01:04 +0800 Subject: [PATCH 4/5] Simplified ctx.state.user fix --- .../strapi-plugin-graphql/services/Mutation.js | 16 ++++------------ packages/strapi-plugin-graphql/services/Query.js | 15 ++++----------- 2 files changed, 8 insertions(+), 23 deletions(-) diff --git a/packages/strapi-plugin-graphql/services/Mutation.js b/packages/strapi-plugin-graphql/services/Mutation.js index 4c7dc44b11..6068782e3f 100644 --- a/packages/strapi-plugin-graphql/services/Mutation.js +++ b/packages/strapi-plugin-graphql/services/Mutation.js @@ -29,6 +29,9 @@ module.exports = { queryName = action; } + // Retrieve policies. + const policies = _.get(handler, `Mutation.${queryName}.policies`, []); + // Retrieve resolverOf. const resolverOf = _.get(handler, `Mutation.${queryName}.resolverOf`, ''); @@ -155,19 +158,8 @@ module.exports = { ); } - // Retrieve policies. - let policies = []; - if (strapi.plugins['users-permissions']) { - policies.push('plugins.users-permissions.permissions'); - } - - // Retrieve policies. - if (_.get(handler, `Mutation.${queryName}.policies`)) { - policies = _.concat( - policies, - _.get(handler, `Mutation.${queryName}.policies`) - ); + policies.unshift('plugins.users-permissions.permissions'); } // Populate policies. diff --git a/packages/strapi-plugin-graphql/services/Query.js b/packages/strapi-plugin-graphql/services/Query.js index c024a3c988..cc3633f25b 100644 --- a/packages/strapi-plugin-graphql/services/Query.js +++ b/packages/strapi-plugin-graphql/services/Query.js @@ -91,6 +91,9 @@ module.exports = { : pluralize.plural(name); } + // Retrieve policies. + const policies = _.get(handler, `Query.${queryName}.policies`, []); + // Retrieve resolverOf. const resolverOf = _.get(handler, `Query.${queryName}.resolverOf`, ''); @@ -232,18 +235,8 @@ module.exports = { ); } - // Retrieve policies. - let policies = []; - if (strapi.plugins['users-permissions']) { - policies.push('plugins.users-permissions.permissions'); - } - - if (_.get(handler, `Query.${queryName}.policies`)) { - policies = _.concat( - policies, - _.get(handler, `Query.${queryName}.policies`) - ); + policies.unshift('plugins.users-permissions.permissions'); } // Populate policies. From 17c4244f141657f8807782451fdf95f193ded3ef Mon Sep 17 00:00:00 2001 From: Don Masakayan Date: Wed, 11 Sep 2019 21:55:26 +0800 Subject: [PATCH 5/5] Pass easier-to-understand options instead of arguments to Mutation.composeMutationResolver() and Query.composeQueryResolver() --- .../services/Mutation.js | 2 +- .../strapi-plugin-graphql/services/Query.js | 2 +- .../services/Resolvers.js | 35 ++++++++++++++++--- .../strapi-plugin-graphql/services/Schema.js | 20 +++++------ 4 files changed, 42 insertions(+), 17 deletions(-) diff --git a/packages/strapi-plugin-graphql/services/Mutation.js b/packages/strapi-plugin-graphql/services/Mutation.js index 6068782e3f..bf5d5b30bf 100644 --- a/packages/strapi-plugin-graphql/services/Mutation.js +++ b/packages/strapi-plugin-graphql/services/Mutation.js @@ -19,7 +19,7 @@ module.exports = { * @return Promise or Error. */ - composeMutationResolver: function(_schema, plugin, name, action) { + composeMutationResolver: function({ _schema, plugin, name, action }) { // Extract custom resolver or type description. const { resolver: handler = {} } = _schema; diff --git a/packages/strapi-plugin-graphql/services/Query.js b/packages/strapi-plugin-graphql/services/Query.js index cc3633f25b..26edffcd76 100644 --- a/packages/strapi-plugin-graphql/services/Query.js +++ b/packages/strapi-plugin-graphql/services/Query.js @@ -69,7 +69,7 @@ module.exports = { * @return Promise or Error. */ - composeQueryResolver: function(_schema, plugin, name, isSingular) { + composeQueryResolver: function({ _schema, plugin, name, isSingular }) { const params = { model: name, }; diff --git a/packages/strapi-plugin-graphql/services/Resolvers.js b/packages/strapi-plugin-graphql/services/Resolvers.js index 6952d0b057..53ece2e335 100644 --- a/packages/strapi-plugin-graphql/services/Resolvers.js +++ b/packages/strapi-plugin-graphql/services/Resolvers.js @@ -326,11 +326,21 @@ const buildShadowCRUD = (models, plugin) => { const queries = { singular: _.get(resolver, `Query.${singularName}`) !== false - ? Query.composeQueryResolver(_schema, plugin, name, true) + ? Query.composeQueryResolver({ + _schema, + plugin, + name, + isSingular: true, + }) : null, plural: _.get(resolver, `Query.${pluralName}`) !== false - ? Query.composeQueryResolver(_schema, plugin, name, false) + ? Query.composeQueryResolver({ + _schema, + plugin, + name, + isSingular: false, + }) : null, }; @@ -376,15 +386,30 @@ const buildShadowCRUD = (models, plugin) => { const mutations = { create: _.get(resolver, `Mutation.create${capitalizedName}`) !== false - ? Mutation.composeMutationResolver(_schema, plugin, name, 'create') + ? Mutation.composeMutationResolver({ + _schema, + plugin, + name, + action: 'create', + }) : null, update: _.get(resolver, `Mutation.update${capitalizedName}`) !== false - ? Mutation.composeMutationResolver(_schema, plugin, name, 'update') + ? Mutation.composeMutationResolver({ + _schema, + plugin, + name, + action: 'update', + }) : null, delete: _.get(resolver, `Mutation.delete${capitalizedName}`) !== false - ? Mutation.composeMutationResolver(_schema, plugin, name, 'delete') + ? Mutation.composeMutationResolver({ + _schema, + plugin, + name, + action: 'delete', + }) : null, }; diff --git a/packages/strapi-plugin-graphql/services/Schema.js b/packages/strapi-plugin-graphql/services/Schema.js index c1217ca39d..b577bc4150 100644 --- a/packages/strapi-plugin-graphql/services/Schema.js +++ b/packages/strapi-plugin-graphql/services/Schema.js @@ -228,22 +228,22 @@ const schemaBuilder = { const [name, action] = acc[type][resolver].split('.'); const normalizedName = _.toLower(name); - acc[type][resolver] = Mutation.composeMutationResolver( - strapi.plugins.graphql.config._schema.graphql, + acc[type][resolver] = Mutation.composeMutationResolver({ + _schema: strapi.plugins.graphql.config._schema.graphql, plugin, - normalizedName, - action - ); + name: normalizedName, + action, + }); break; } case 'Query': default: - acc[type][resolver] = Query.composeQueryResolver( - strapi.plugins.graphql.config._schema.graphql, + acc[type][resolver] = Query.composeQueryResolver({ + _schema: strapi.plugins.graphql.config._schema.graphql, plugin, - resolver, - 'force' // Avoid singular/pluralize and force query name. - ); + name: resolver, + isSingular: 'force', // Avoid singular/pluralize and force query name. + }); break; } }