From f2e769cd695f764d5a13a938ae09aae4d2b94ce8 Mon Sep 17 00:00:00 2001 From: Mark Kaylor Date: Thu, 4 May 2023 10:38:09 +0200 Subject: [PATCH] Refactor to check permissions on entities before passing to updateMany --- .../server/controllers/collection-types.js | 46 +++++----- .../services/__tests__/entity-manager.test.js | 26 +++--- .../server/services/entity-manager.js | 84 ++++++++++--------- 3 files changed, 87 insertions(+), 69 deletions(-) diff --git a/packages/core/content-manager/server/controllers/collection-types.js b/packages/core/content-manager/server/controllers/collection-types.js index 79b2f38a8d..3cba7592c4 100644 --- a/packages/core/content-manager/server/controllers/collection-types.js +++ b/packages/core/content-manager/server/controllers/collection-types.js @@ -184,7 +184,7 @@ module.exports = { async bulkPublish(ctx) { const { userAbility } = ctx.state; const { model } = ctx.params; - const { query, body } = ctx.request; + const { body } = ctx.request; const { ids } = body; await validateBulkActionInput(body); @@ -196,24 +196,27 @@ module.exports = { return ctx.forbidden(); } - const permissionQuery = await permissionChecker.sanitizedQuery.publish(query); + const entityPromises = ids.map((id) => entityManager.findOneWithCreatorRoles(id, model)); + const entities = await Promise.all(entityPromises); - const idsWhereClause = { id: { $in: ids } }; - const params = { - ...permissionQuery, - filters: { - $and: [idsWhereClause].concat(permissionQuery.filters || []), - }, - }; + for (const entity of entities) { + if (!entity) { + return ctx.notFound(); + } - const { count } = await entityManager.publishMany(params, model); + if (permissionChecker.cannot.publish(entity)) { + return ctx.forbidden(); + } + } + + const { count } = await entityManager.publishMany(entities, model); ctx.body = { count }; }, async bulkUnpublish(ctx) { const { userAbility } = ctx.state; const { model } = ctx.params; - const { query, body } = ctx.request; + const { body } = ctx.request; const { ids } = body; await validateBulkActionInput(body); @@ -225,17 +228,20 @@ module.exports = { return ctx.forbidden(); } - const permissionQuery = await permissionChecker.sanitizedQuery.unpublish(query); + const entityPromises = ids.map((id) => entityManager.findOneWithCreatorRoles(id, model)); + const entities = await Promise.all(entityPromises); - const idsWhereClause = { id: { $in: ids } }; - const params = { - ...permissionQuery, - filters: { - $and: [idsWhereClause].concat(permissionQuery.filters || []), - }, - }; + for (const entity of entities) { + if (!entity) { + return ctx.notFound(); + } - const { count } = await entityManager.unpublishMany(params, model); + if (permissionChecker.cannot.publish(entity)) { + return ctx.forbidden(); + } + } + + const { count } = await entityManager.unpublishMany(entities, model); ctx.body = { count }; }, diff --git a/packages/core/content-manager/server/services/__tests__/entity-manager.test.js b/packages/core/content-manager/server/services/__tests__/entity-manager.test.js index 15aae7bd73..5d00169f35 100644 --- a/packages/core/content-manager/server/services/__tests__/entity-manager.test.js +++ b/packages/core/content-manager/server/services/__tests__/entity-manager.test.js @@ -17,11 +17,11 @@ describe('Content-Manager', () => { beforeEach(() => { global.strapi = { entityService: { + findMany: jest.fn().mockResolvedValue([{ id: 1 }, { id: 2 }]), update: jest.fn().mockReturnValue({ id: 1, publishedAt: new Date() }), }, db: { query: jest.fn(() => ({ - findMany: jest.fn().mockResolvedValue([{ id: 1 }, { id: 2 }]), updateMany: queryUpdateMock, })), }, @@ -55,13 +55,16 @@ describe('Content-Manager', () => { test('Publish many content-types', async () => { const uid = 'api::test.test'; - const params = { filters: { $and: [1, 2] } }; + const entities = [ + { id: 1, publishedAt: null }, + { id: 2, publishedAt: null }, + ]; - await entityManager.publishMany(params, uid); + await entityManager.publishMany(entities, uid); - expect(strapi.db.query().updateMany).toBeCalledWith({ + expect(strapi.db.query().updateMany).toHaveBeenCalledWith({ where: { - $and: [1, 2], + id: { $in: [1, 2] }, }, data: { publishedAt: expect.any(Date) }, }); @@ -74,11 +77,11 @@ describe('Content-Manager', () => { global.strapi = { db: { query: jest.fn(() => ({ - findMany: jest.fn().mockResolvedValue([{ id: 1 }, { id: 2 }]), updateMany: queryUpdateMock, })), }, entityService: { + findMany: jest.fn().mockResolvedValue([{ id: 1 }, { id: 2 }]), update: jest.fn().mockReturnValue({ id: 1, publishedAt: null }), }, eventHub: { emit: jest.fn(), sanitizeEntity: (entity) => entity }, @@ -107,13 +110,16 @@ describe('Content-Manager', () => { test('Unpublish many content-types', async () => { const uid = 'api::test.test'; - const params = { filters: { $and: [1, 2] } }; + const entities = [ + { id: 1, publishedAt: new Date() }, + { id: 2, publishedAt: new Date() }, + ]; - await entityManager.unpublishMany(params, uid); + await entityManager.unpublishMany(entities, uid); - expect(strapi.db.query().updateMany).toBeCalledWith({ + expect(strapi.db.query().updateMany).toHaveBeenCalledWith({ where: { - $and: [1, 2], + id: { $in: [1, 2] }, }, data: { publishedAt: null }, }); diff --git a/packages/core/content-manager/server/services/entity-manager.js b/packages/core/content-manager/server/services/entity-manager.js index f059bd4ed2..00b76aa862 100644 --- a/packages/core/content-manager/server/services/entity-manager.js +++ b/packages/core/content-manager/server/services/entity-manager.js @@ -3,7 +3,6 @@ const { assoc, has, prop, omit } = require('lodash/fp'); const strapiUtils = require('@strapi/utils'); const { mapAsync } = require('@strapi/utils'); -const { transformParamsToQuery } = require('@strapi/utils').convertQueryParams; const { ApplicationError } = require('@strapi/utils').errors; const { getDeepPopulate, getDeepPopulateDraftCount } = require('./utils/populate'); const { getDeepRelationsCount } = require('./utils/count'); @@ -267,41 +266,44 @@ module.exports = ({ strapi }) => ({ return mappedEntity; }, - async publishMany(opts, uid) { - const params = { - ...opts, - data: { - [PUBLISHED_AT_ATTRIBUTE]: new Date(), - }, - }; - - const query = transformParamsToQuery(uid, params); - const entitiesToUpdate = await strapi.db.query(uid).findMany(query); - // No entities to update, return early - if (!entitiesToUpdate.length) { + async publishMany(entities, uid) { + if (!entities.length) { return null; } // Validate entities before publishing, throw if invalid await Promise.all( - entitiesToUpdate.map((entityToUpdate) => - strapi.entityValidator.validateEntityCreation( + entities.map((entityToUpdate) => { + if (entityToUpdate[PUBLISHED_AT_ATTRIBUTE]) { + throw new ApplicationError('already.published'); + } + + return strapi.entityValidator.validateEntityCreation( strapi.getModel(uid), entityToUpdate, { isDraft: true, }, entityToUpdate - ) - ) + ); + }) ); - // Everything is valid, publish - const publishedEntitiesCount = await strapi.db - .query(uid) - .updateMany({ ...query, data: params.data }); + const where = { id: { $in: entities.map((entity) => entity.id) } }; + const data = { + [PUBLISHED_AT_ATTRIBUTE]: new Date(), + }; + const populate = isRelationsPopulateEnabled(uid) + ? getDeepPopulate(uid, {}) + : getDeepPopulate(uid, { countMany: true, countOne: true }); + + // Everything is valid, publish + const publishedEntitiesCount = await strapi.db.query(uid).updateMany({ + where, + data, + }); // Get the updated entities since updateMany only returns the count - const publishedEntities = await strapi.db.query(uid).findMany(query); + const publishedEntities = await strapi.entityService.findMany(uid, { where, populate }); // Emit the publish event for all updated entities await Promise.all(publishedEntities.map((entity) => emitEvent(ENTRY_PUBLISH, entity, uid))); @@ -309,28 +311,32 @@ module.exports = ({ strapi }) => ({ return publishedEntitiesCount; }, - async unpublishMany(opts, uid) { - const params = { - ...opts, - data: { - [PUBLISHED_AT_ATTRIBUTE]: null, - }, - }; - - const query = transformParamsToQuery(uid, params); - const entitiesToUpdate = await strapi.db.query(uid).findMany(query); - // No entities to update, return early - if (!entitiesToUpdate.length) { + async unpublishMany(entities, uid) { + if (!entities.length) { return null; } - // No need to validate, unpublish - const unpublishedEntitiesCount = await strapi.db - .query(uid) - .updateMany({ ...query, data: params.data }); + entities.forEach((entity) => { + if (!entity[PUBLISHED_AT_ATTRIBUTE]) { + throw new ApplicationError('already.draft'); + } + }); + const where = { id: { $in: entities.map((entity) => entity.id) } }; + const data = { + [PUBLISHED_AT_ATTRIBUTE]: null, + }; + const populate = isRelationsPopulateEnabled(uid) + ? getDeepPopulate(uid, {}) + : getDeepPopulate(uid, { countMany: true, countOne: true }); + + // No need to validate, unpublish + const unpublishedEntitiesCount = await strapi.db.query(uid).updateMany({ + where, + data, + }); // Get the updated entities since updateMany only returns the count - const unpublishedEntities = await strapi.db.query(uid).findMany(query); + const unpublishedEntities = await strapi.entityService.findMany(uid, { where, populate }); // Emit the unpublish event for all updated entities await Promise.all(unpublishedEntities.map((entity) => emitEvent(ENTRY_UNPUBLISH, entity, uid)));