From 241393eaa3ccbf06719c839a5e44f0869c9c2977 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Pierre=20No=C3=ABl?= Date: Tue, 4 Oct 2022 17:10:18 +0200 Subject: [PATCH] implement new findAvailable route --- .../server/controllers/relations.js | 69 ++++++++----------- .../content-manager/server/routes/admin.js | 14 +--- .../find-available-relations.test.e2e.js | 31 +++------ 3 files changed, 42 insertions(+), 72 deletions(-) diff --git a/packages/core/content-manager/server/controllers/relations.js b/packages/core/content-manager/server/controllers/relations.js index 6ff50b5948..aabdf84b93 100644 --- a/packages/core/content-manager/server/controllers/relations.js +++ b/packages/core/content-manager/server/controllers/relations.js @@ -26,57 +26,52 @@ module.exports = { // idsToOmit: used to exclude relations that the front already added but that were not saved yet // idsToInclude: used to include relations that the front removed but not saved yes - const { component, entityId, idsToOmit, idsToInclude, _q, ...query } = ctx.request.query; + const { entityId, idsToOmit, idsToInclude, _q, ...query } = ctx.request.query; - const sourceModelUid = component || model; - - const sourceModel = strapi.getModel(sourceModelUid); - if (!sourceModel) { + const modelSchema = strapi.getModel(model); + if (!modelSchema) { return ctx.badRequest("The model doesn't exist"); } - const permissionChecker = getService('permission-checker').create({ - userAbility, - model, - }); - - if (permissionChecker.cannot.read()) { - return ctx.forbidden(); - } - - const attribute = sourceModel.attributes[targetField]; + const attribute = modelSchema.attributes[targetField]; if (!attribute || attribute.type !== 'relation') { return ctx.badRequest("This relational field doesn't exist"); } - // TODO: find a way to check field permission for component - if (!component && permissionChecker.cannot.read(null, targetField)) { - return ctx.forbidden(); - } + const isComponent = modelSchema.modelType === 'component'; - if (entityId) { - const entityManager = getService('entity-manager'); + // RBAC checks when it's a content-type + // TODO: do RBAC check for components too + if (!isComponent) { + const permissionChecker = getService('permission-checker').create({ + userAbility, + model, + }); - const entity = await entityManager.findOneWithCreatorRoles(entityId, model); - - if (!entity) { - return ctx.notFound(); - } - - if (!component && permissionChecker.cannot.read(entity, targetField)) { + if (permissionChecker.cannot.read(null, targetField)) { return ctx.forbidden(); } - // TODO: find a way to check field permission for component - if (component && permissionChecker.cannot.read(entity)) { - return ctx.forbidden(); + + if (entityId) { + const entityManager = getService('entity-manager'); + + const entity = await entityManager.findOneWithCreatorRoles(entityId, model); + + if (!entity) { + return ctx.notFound(); + } + + if (permissionChecker.cannot.read(entity, targetField)) { + return ctx.forbidden(); + } } } const targetedModel = strapi.getModel(attribute.target); - const modelConfig = component - ? await getService('components').findConfiguration(sourceModel) - : await getService('content-types').findConfiguration(sourceModel); + const modelConfig = isComponent + ? await getService('components').findConfiguration(modelSchema) + : await getService('content-types').findConfiguration(modelSchema); const mainField = prop(`metadatas.${targetField}.edit.mainField`, modelConfig) || 'id'; const fieldsToSelect = ['id', mainField]; @@ -101,7 +96,7 @@ module.exports = { } if (entityId) { - const subQuery = strapi.db.queryBuilder(sourceModel.uid); + const subQuery = strapi.db.queryBuilder(modelSchema.uid); const alias = subQuery.getAlias(); @@ -153,10 +148,6 @@ module.exports = { model, }); - if (permissionChecker.cannot.read()) { - return ctx.forbidden(); - } - if (permissionChecker.cannot.read(null, targetField)) { return ctx.forbidden(); } diff --git a/packages/core/content-manager/server/routes/admin.js b/packages/core/content-manager/server/routes/admin.js index 7f84d509fb..70d47abee9 100644 --- a/packages/core/content-manager/server/routes/admin.js +++ b/packages/core/content-manager/server/routes/admin.js @@ -84,19 +84,7 @@ module.exports = { path: '/relations/:model/:targetField', handler: 'relations.findAvailable', config: { - policies: [ - 'admin::isAuthenticatedAdmin', - { - name: 'plugin::content-manager.hasPermissions', - config: { - actions: [ - 'plugin::content-manager.explorer.create', - 'plugin::content-manager.explorer.update', - ], - hasAtLeastOne: true, - }, - }, - ], + policies: ['admin::isAuthenticatedAdmin'], }, }, { diff --git a/packages/core/content-manager/server/tests/find-available-relations.test.e2e.js b/packages/core/content-manager/server/tests/find-available-relations.test.e2e.js index b5607f98c0..adb2136e69 100644 --- a/packages/core/content-manager/server/tests/find-available-relations.test.e2e.js +++ b/packages/core/content-manager/server/tests/find-available-relations.test.e2e.js @@ -180,25 +180,25 @@ describe.each([[false], [true]])('Relations, with d&p: %p', (withDraftAndPublish ['products_mw', false], ['compo_products_ow', true], ['compo_products_mw', true], - ])('Relation not in a component (%s)', (fieldName, isComponent) => { + ])('Relation (%s) (is in component: %s)', (fieldName, isComponent) => { let entityId; let entityIdEmptyShop; + let modelUID; beforeAll(() => { entityId = isComponent ? data.shops[0].myCompo.id : data.shops[0].id; entityIdEmptyShop = isComponent ? data.shops[1].myCompo.id : data.shops[1].id; + modelUID = isComponent ? 'default.compo' : 'api::shop.shop'; }); test('Can retrieve available relation(s) for an entity that have some relations', async () => { let res = await rq({ method: 'GET', - url: `/content-manager/relations/api::shop.shop/${fieldName}`, + url: `/content-manager/relations/${modelUID}/${fieldName}`, qs: { entityId, - ...(isComponent ? { component: 'default.compo' } : {}), }, }); - expect(res.status).toBe(200); expect(res.body.results).toMatchObject([ @@ -212,11 +212,10 @@ describe.each([[false], [true]])('Relations, with d&p: %p', (withDraftAndPublish // can omitIds res = await rq({ method: 'GET', - url: `/content-manager/relations/api::shop.shop/${fieldName}`, + url: `/content-manager/relations/${modelUID}/${fieldName}`, qs: { entityId, idsToOmit: [data.products[1].id], - ...(isComponent ? { component: 'default.compo' } : {}), }, }); @@ -226,10 +225,9 @@ describe.each([[false], [true]])('Relations, with d&p: %p', (withDraftAndPublish test("Can retrieve available relation(s) for an entity that don't have relations yet", async () => { let res = await rq({ method: 'GET', - url: `/content-manager/relations/api::shop.shop/${fieldName}`, + url: `/content-manager/relations/${modelUID}/${fieldName}`, qs: { entityId: entityIdEmptyShop, - ...(isComponent ? { component: 'default.compo' } : {}), }, }); @@ -251,11 +249,10 @@ describe.each([[false], [true]])('Relations, with d&p: %p', (withDraftAndPublish // can omitIds res = await rq({ method: 'GET', - url: `/content-manager/relations/api::shop.shop/${fieldName}`, + url: `/content-manager/relations/${modelUID}/${fieldName}`, qs: { entityId, idsToOmit: [data.products[1].id], - ...(isComponent ? { component: 'default.compo' } : {}), }, }); @@ -265,10 +262,7 @@ describe.each([[false], [true]])('Relations, with d&p: %p', (withDraftAndPublish test('Can retrieve available relation(s) without entity', async () => { let res = await rq({ method: 'GET', - url: `/content-manager/relations/api::shop.shop/${fieldName}`, - qs: { - ...(isComponent ? { component: 'default.compo' } : {}), - }, + url: `/content-manager/relations/${modelUID}/${fieldName}`, }); expect(res.status).toBe(200); @@ -289,10 +283,9 @@ describe.each([[false], [true]])('Relations, with d&p: %p', (withDraftAndPublish // can omitIds res = await rq({ method: 'GET', - url: `/content-manager/relations/api::shop.shop/${fieldName}`, + url: `/content-manager/relations/${modelUID}/${fieldName}`, qs: { idsToOmit: [data.products[0].id], - ...(isComponent ? { component: 'default.compo' } : {}), }, }); @@ -311,11 +304,10 @@ describe.each([[false], [true]])('Relations, with d&p: %p', (withDraftAndPublish test("search ''", async () => { const res = await rq({ method: 'GET', - url: `/content-manager/relations/api::shop.shop/${fieldName}`, + url: `/content-manager/relations/${modelUID}/${fieldName}`, qs: { _q: '', ...(withEntity ? { entityId } : {}), - ...(isComponent ? { component: 'default.compo' } : {}), }, }); @@ -331,11 +323,10 @@ describe.each([[false], [true]])('Relations, with d&p: %p', (withDraftAndPublish test("search 'Can'", async () => { const res = await rq({ method: 'GET', - url: `/content-manager/relations/api::shop.shop/${fieldName}`, + url: `/content-manager/relations/${modelUID}/${fieldName}`, qs: { _q: 'Can', ...(withEntity ? { entityId } : {}), - ...(isComponent ? { component: 'default.compo' } : {}), }, });