From 54af039f1789c4391f07ac2bf1b34a3b27af2b14 Mon Sep 17 00:00:00 2001 From: Alexandre BODIN Date: Tue, 26 Jan 2021 10:18:43 +0100 Subject: [PATCH] Fix is-creator condition not applied on find (#9213) * Fix is-creator condition not applied on find * Add test --- .../__tests__/permissions-manager.test.js | 14 ++++++++++++-- .../permission/permissions-manager/index.js | 17 +++++++++++------ .../controllers/collection-types.js | 4 ++-- .../services/permission-checker.js | 8 ++++++-- 4 files changed, 31 insertions(+), 12 deletions(-) diff --git a/packages/strapi-admin/services/__tests__/permissions-manager.test.js b/packages/strapi-admin/services/__tests__/permissions-manager.test.js index 52e5460dc8..d6bf586b72 100644 --- a/packages/strapi-admin/services/__tests__/permissions-manager.test.js +++ b/packages/strapi-admin/services/__tests__/permissions-manager.test.js @@ -14,7 +14,7 @@ describe('Permissions Manager', () => { model: 'foo', }); - expect(pm.query).toStrictEqual({}); + expect(pm.getQuery()).toStrictEqual({}); }); test('It should returns a valid query from the ability', () => { @@ -27,7 +27,17 @@ describe('Permissions Manager', () => { const expected = { _or: [{ kai: 'doe' }] }; - expect(pm.query).toStrictEqual(expected); + expect(pm.getQuery()).toStrictEqual(expected); + }); + + test('It should throw if no action is defined', () => { + const ability = defineAbility(can => can('read', 'foo', ['bar'], { kai: 'doe' })); + const pm = createPermissionsManager({ + ability, + model: 'foo', + }); + + expect(() => pm.getQuery()).toThrowError(); }); }); diff --git a/packages/strapi-admin/services/permission/permissions-manager/index.js b/packages/strapi-admin/services/permission/permissions-manager/index.js index 14dd5590b0..754179b009 100644 --- a/packages/strapi-admin/services/permission/permissions-manager/index.js +++ b/packages/strapi-admin/services/permission/permissions-manager/index.js @@ -14,10 +14,6 @@ module.exports = ({ ability, action, model }) => ({ action, model, - get query() { - return buildStrapiQuery(buildCaslQuery(ability, action, model)); - }, - get isAllowed() { return this.ability.can(action, model); }, @@ -30,10 +26,19 @@ module.exports = ({ ability, action, model }) => ({ return this.sanitize(data, { ...options, isOutput: false }); }, - queryFrom(query) { + getQuery(queryAction = action) { + if (_.isUndefined(queryAction)) { + throw new Error('Action must be defined to build a permission query'); + } + + return buildStrapiQuery(buildCaslQuery(ability, queryAction, model)); + }, + + queryFrom(query = {}, action) { + const permissionQuery = this.getQuery(action); return { ...query, - _where: query._where ? _.concat(this.query, query._where) : [this.query], + _where: query._where ? _.concat(permissionQuery, query._where) : [permissionQuery], }; }, diff --git a/packages/strapi-plugin-content-manager/controllers/collection-types.js b/packages/strapi-plugin-content-manager/controllers/collection-types.js index 5ebe871f3e..6f1d75a9fb 100644 --- a/packages/strapi-plugin-content-manager/controllers/collection-types.js +++ b/packages/strapi-plugin-content-manager/controllers/collection-types.js @@ -26,7 +26,7 @@ module.exports = { const method = has('_q', query) ? 'searchWithRelationCounts' : 'findWithRelationCounts'; - const permissionQuery = permissionChecker.buildPermissionQuery(query); + const permissionQuery = permissionChecker.buildReadQuery(query); const { results, pagination } = await entityManager[method](permissionQuery, model); @@ -214,7 +214,7 @@ module.exports = { return ctx.forbidden(); } - const permissionQuery = permissionChecker.buildPermissionQuery(query); + const permissionQuery = permissionChecker.buildDeleteQuery(query); const idsWhereClause = { [`id_in`]: ids }; const params = { diff --git a/packages/strapi-plugin-content-manager/services/permission-checker.js b/packages/strapi-plugin-content-manager/services/permission-checker.js index c71981dee6..c0a84ea8be 100644 --- a/packages/strapi-plugin-content-manager/services/permission-checker.js +++ b/packages/strapi-plugin-content-manager/services/permission-checker.js @@ -43,7 +43,10 @@ const createPermissionChecker = ({ userAbility, model }) => { const sanitizeCreateInput = data => sanitizeInput(ACTIONS.create, data); const sanitizeUpdateInput = entity => data => sanitizeInput(ACTIONS.update, data, entity); - const buildPermissionQuery = query => permissionsManager.queryFrom(query); + const buildPermissionQuery = (query, action) => permissionsManager.queryFrom(query, action); + + const buildReadQuery = query => buildPermissionQuery(query, ACTIONS.read); + const buildDeleteQuery = query => buildPermissionQuery(query, ACTIONS.delete); Object.keys(ACTIONS).forEach(action => { can[action] = (...args) => can(ACTIONS[action], ...args); @@ -56,7 +59,8 @@ const createPermissionChecker = ({ userAbility, model }) => { sanitizeOutput, sanitizeCreateInput, sanitizeUpdateInput, - buildPermissionQuery, + buildReadQuery, + buildDeleteQuery, }; };