From 5ba0d2d65773eb08d01b9ed82386783a160d4740 Mon Sep 17 00:00:00 2001 From: harimkims Date: Tue, 21 Dec 2021 10:43:36 +0900 Subject: [PATCH 01/11] Fix unable to populate user Signed-off-by: harimkims --- .../users-permissions/server/controllers/user.js | 13 ++++++++++--- 1 file changed, 10 insertions(+), 3 deletions(-) diff --git a/packages/plugins/users-permissions/server/controllers/user.js b/packages/plugins/users-permissions/server/controllers/user.js index 2aa763c09d..a0d747fde1 100644 --- a/packages/plugins/users-permissions/server/controllers/user.js +++ b/packages/plugins/users-permissions/server/controllers/user.js @@ -8,6 +8,7 @@ const _ = require('lodash'); const utils = require('@strapi/utils'); +const { convertPopulateQueryParams } = require('@strapi/utils/lib/convert-query-params'); const { getService } = require('../utils'); const { validateCreateUserBody, validateUpdateUserBody } = require('./validation/user'); @@ -133,8 +134,11 @@ module.exports = { * Retrieve user records. * @return {Object|Array} */ - async find(ctx, next, { populate } = {}) { - const users = await getService('user').fetchAll(ctx.query.filters, populate); + async find(ctx) { + const users = await getService('user').fetchAll( + ctx.query.filters, + convertPopulateQueryParams(ctx.query.populate) + ); ctx.body = await Promise.all(users.map(user => sanitizeOutput(user, ctx))); }, @@ -145,7 +149,10 @@ module.exports = { */ async findOne(ctx) { const { id } = ctx.params; - let data = await getService('user').fetch({ id }); + let data = await getService('user').fetch( + { id }, + convertPopulateQueryParams(ctx.query.populate) + ); if (data) { data = await sanitizeOutput(data, ctx); From 54d10ec83cc1caab5ef026878c0d55dd8057115a Mon Sep 17 00:00:00 2001 From: harimkims Date: Tue, 21 Dec 2021 11:24:39 +0900 Subject: [PATCH 02/11] Fix e2e test --- packages/plugins/users-permissions/server/controllers/user.js | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/packages/plugins/users-permissions/server/controllers/user.js b/packages/plugins/users-permissions/server/controllers/user.js index a0d747fde1..4725b6e980 100644 --- a/packages/plugins/users-permissions/server/controllers/user.js +++ b/packages/plugins/users-permissions/server/controllers/user.js @@ -137,7 +137,7 @@ module.exports = { async find(ctx) { const users = await getService('user').fetchAll( ctx.query.filters, - convertPopulateQueryParams(ctx.query.populate) + convertPopulateQueryParams(ctx.query.populate || {}) ); ctx.body = await Promise.all(users.map(user => sanitizeOutput(user, ctx))); @@ -151,7 +151,7 @@ module.exports = { const { id } = ctx.params; let data = await getService('user').fetch( { id }, - convertPopulateQueryParams(ctx.query.populate) + convertPopulateQueryParams(ctx.query.populate || {}) ); if (data) { From 4a71a9c93c6d5ffc2db18293b087ca7d29541811 Mon Sep 17 00:00:00 2001 From: harimkims Date: Thu, 3 Mar 2022 22:45:55 +0900 Subject: [PATCH 03/11] Add new sanitizer to remove user relation from role entities --- packages/core/utils/lib/sanitize/index.js | 5 ++++- packages/core/utils/lib/sanitize/sanitizers.js | 7 ++++++- packages/core/utils/lib/sanitize/visitors/index.js | 1 + .../remove-user-relation-from-role-entities.js | 11 +++++++++++ 4 files changed, 22 insertions(+), 2 deletions(-) create mode 100644 packages/core/utils/lib/sanitize/visitors/remove-user-relation-from-role-entities.js diff --git a/packages/core/utils/lib/sanitize/index.js b/packages/core/utils/lib/sanitize/index.js index 8ae833c328..a5f1e3253b 100644 --- a/packages/core/utils/lib/sanitize/index.js +++ b/packages/core/utils/lib/sanitize/index.js @@ -36,7 +36,10 @@ module.exports = { return Promise.all(data.map(entry => this.output(entry, schema, { auth }))); } - const transforms = [sanitizers.defaultSanitizeOutput(schema)]; + const transforms = [ + sanitizers.defaultSanitizeOutput(schema), + sanitizers.sanitizeUserRelationFromRoleEntities(schema), + ]; if (auth) { transforms.push(traverseEntity(visitors.removeRestrictedRelations(auth), { schema })); diff --git a/packages/core/utils/lib/sanitize/sanitizers.js b/packages/core/utils/lib/sanitize/sanitizers.js index 87c42812c3..2281243c40 100644 --- a/packages/core/utils/lib/sanitize/sanitizers.js +++ b/packages/core/utils/lib/sanitize/sanitizers.js @@ -5,7 +5,7 @@ const { curry } = require('lodash/fp'); const pipeAsync = require('../pipe-async'); const traverseEntity = require('../traverse-entity'); -const { removePassword, removePrivate } = require('./visitors'); +const { removePassword, removePrivate, removeUserRelationFromRoleEntities } = require('./visitors'); const sanitizePasswords = curry((schema, entity) => { return traverseEntity(removePassword, { schema }, entity); @@ -15,6 +15,10 @@ const sanitizePrivates = curry((schema, entity) => { return traverseEntity(removePrivate, { schema }, entity); }); +const sanitizeUserRelationFromRoleEntities = curry((schema, entity) => { + return traverseEntity(removeUserRelationFromRoleEntities, { schema }, entity); +}); + const defaultSanitizeOutput = curry((schema, entity) => { return pipeAsync(sanitizePrivates(schema), sanitizePasswords(schema))(entity); }); @@ -22,5 +26,6 @@ const defaultSanitizeOutput = curry((schema, entity) => { module.exports = { sanitizePasswords, sanitizePrivates, + sanitizeUserRelationFromRoleEntities, defaultSanitizeOutput, }; diff --git a/packages/core/utils/lib/sanitize/visitors/index.js b/packages/core/utils/lib/sanitize/visitors/index.js index b0597d63f5..c34067c2b6 100644 --- a/packages/core/utils/lib/sanitize/visitors/index.js +++ b/packages/core/utils/lib/sanitize/visitors/index.js @@ -4,6 +4,7 @@ module.exports = { removePassword: require('./remove-password'), removePrivate: require('./remove-private'), removeRestrictedRelations: require('./remove-restricted-relations'), + removeUserRelationFromRoleEntities: require('./remove-user-relation-from-role-entities'), allowedFields: require('./allowed-fields'), restrictedFields: require('./restricted-fields'), }; diff --git a/packages/core/utils/lib/sanitize/visitors/remove-user-relation-from-role-entities.js b/packages/core/utils/lib/sanitize/visitors/remove-user-relation-from-role-entities.js new file mode 100644 index 0000000000..e20f85a827 --- /dev/null +++ b/packages/core/utils/lib/sanitize/visitors/remove-user-relation-from-role-entities.js @@ -0,0 +1,11 @@ +'use strict'; + +module.exports = ({ schema, key, attribute }, { remove }) => { + if ( + attribute.type === 'relation' && + attribute.target === 'plugin::users-permissions.user' && + schema.uid === 'plugin::users-permissions.role' + ) { + remove(key); + } +}; From 567816591a521630f16facb01caa86447c1039fd Mon Sep 17 00:00:00 2001 From: harimkims Date: Thu, 3 Mar 2022 22:55:29 +0900 Subject: [PATCH 04/11] Rename getRoles, getRole to find, findOne --- .../plugins/users-permissions/server/controllers/role.js | 8 ++++---- .../users-permissions/server/controllers/settings.js | 2 +- .../plugins/users-permissions/server/routes/admin/role.js | 4 ++-- .../users-permissions/server/routes/content-api/role.js | 4 ++-- .../plugins/users-permissions/server/services/role.js | 4 ++-- 5 files changed, 11 insertions(+), 11 deletions(-) diff --git a/packages/plugins/users-permissions/server/controllers/role.js b/packages/plugins/users-permissions/server/controllers/role.js index ef6a86b0d8..a31c86fdb8 100644 --- a/packages/plugins/users-permissions/server/controllers/role.js +++ b/packages/plugins/users-permissions/server/controllers/role.js @@ -21,10 +21,10 @@ module.exports = { ctx.send({ ok: true }); }, - async getRole(ctx) { + async findOne(ctx) { const { id } = ctx.params; - const role = await getService('role').getRole(id); + const role = await getService('role').findOne(id); if (!role) { return ctx.notFound(); @@ -33,8 +33,8 @@ module.exports = { ctx.send({ role }); }, - async getRoles(ctx) { - const roles = await getService('role').getRoles(); + async find(ctx) { + const roles = await getService('role').find(); ctx.send({ roles }); }, diff --git a/packages/plugins/users-permissions/server/controllers/settings.js b/packages/plugins/users-permissions/server/controllers/settings.js index 9d1eddea94..f4475bc19c 100644 --- a/packages/plugins/users-permissions/server/controllers/settings.js +++ b/packages/plugins/users-permissions/server/controllers/settings.js @@ -37,7 +37,7 @@ module.exports = { .store({ type: 'plugin', name: 'users-permissions', key: 'advanced' }) .get(); - const roles = await getService('role').getRoles(); + const roles = await getService('role').find(); ctx.send({ settings, roles }); }, diff --git a/packages/plugins/users-permissions/server/routes/admin/role.js b/packages/plugins/users-permissions/server/routes/admin/role.js index ac21ad860a..4bdbbdaec2 100644 --- a/packages/plugins/users-permissions/server/routes/admin/role.js +++ b/packages/plugins/users-permissions/server/routes/admin/role.js @@ -4,7 +4,7 @@ module.exports = [ { method: 'GET', path: '/roles/:id', - handler: 'role.getRole', + handler: 'role.findOne', config: { policies: [ { @@ -19,7 +19,7 @@ module.exports = [ { method: 'GET', path: '/roles', - handler: 'role.getRoles', + handler: 'role.find', config: { policies: [ { diff --git a/packages/plugins/users-permissions/server/routes/content-api/role.js b/packages/plugins/users-permissions/server/routes/content-api/role.js index 1434dae4f6..ca7236526a 100644 --- a/packages/plugins/users-permissions/server/routes/content-api/role.js +++ b/packages/plugins/users-permissions/server/routes/content-api/role.js @@ -4,12 +4,12 @@ module.exports = [ { method: 'GET', path: '/roles/:id', - handler: 'role.getRole', + handler: 'role.findOne', }, { method: 'GET', path: '/roles', - handler: 'role.getRoles', + handler: 'role.find', }, { method: 'POST', diff --git a/packages/plugins/users-permissions/server/services/role.js b/packages/plugins/users-permissions/server/services/role.js index 34d133f717..66cacecb77 100644 --- a/packages/plugins/users-permissions/server/services/role.js +++ b/packages/plugins/users-permissions/server/services/role.js @@ -41,7 +41,7 @@ module.exports = ({ strapi }) => ({ await Promise.all(createPromises); }, - async getRole(roleID) { + async findOne(roleID) { const role = await strapi .query('plugin::users-permissions.role') .findOne({ where: { id: roleID }, populate: ['permissions'] }); @@ -68,7 +68,7 @@ module.exports = ({ strapi }) => ({ }; }, - async getRoles() { + async find() { const roles = await strapi.query('plugin::users-permissions.role').findMany({ sort: ['name'] }); for (const role of roles) { From 183bad03d3640286c1ecb2ba34794cefe30d1395 Mon Sep 17 00:00:00 2001 From: harimkims Date: Thu, 3 Mar 2022 22:56:58 +0900 Subject: [PATCH 05/11] Replace fetch, fetchAll query with entityService --- .../users-permissions/server/controllers/user.js | 13 ++++--------- .../users-permissions/server/services/user.js | 8 ++++---- 2 files changed, 8 insertions(+), 13 deletions(-) diff --git a/packages/plugins/users-permissions/server/controllers/user.js b/packages/plugins/users-permissions/server/controllers/user.js index 2bbd1a8c35..29261837b1 100644 --- a/packages/plugins/users-permissions/server/controllers/user.js +++ b/packages/plugins/users-permissions/server/controllers/user.js @@ -8,7 +8,6 @@ const _ = require('lodash'); const utils = require('@strapi/utils'); -const { convertPopulateQueryParams } = require('@strapi/utils/lib/convert-query-params'); const { getService } = require('../utils'); const { validateCreateUserBody, validateUpdateUserBody } = require('./validation/user'); @@ -135,10 +134,7 @@ module.exports = { * @return {Object|Array} */ async find(ctx) { - const users = await getService('user').fetchAll( - ctx.query.filters, - convertPopulateQueryParams(ctx.query.populate || {}) - ); + const users = await getService('user').fetchAll(ctx.query); ctx.body = await Promise.all(users.map(user => sanitizeOutput(user, ctx))); }, @@ -149,10 +145,9 @@ module.exports = { */ async findOne(ctx) { const { id } = ctx.params; - let data = await getService('user').fetch( - { id }, - convertPopulateQueryParams(ctx.query.populate || {}) - ); + const { query } = ctx; + + let data = await getService('user').fetch(id, query); if (data) { data = await sanitizeOutput(data, ctx); diff --git a/packages/plugins/users-permissions/server/services/user.js b/packages/plugins/users-permissions/server/services/user.js index 777d4777e5..817371618f 100644 --- a/packages/plugins/users-permissions/server/services/user.js +++ b/packages/plugins/users-permissions/server/services/user.js @@ -58,8 +58,8 @@ module.exports = ({ strapi }) => ({ * Promise to fetch a/an user. * @return {Promise} */ - fetch(params, populate) { - return strapi.query('plugin::users-permissions.user').findOne({ where: params, populate }); + fetch(id, params) { + return strapi.entityService.findOne('plugin::users-permissions.user', id, params); }, /** @@ -76,8 +76,8 @@ module.exports = ({ strapi }) => ({ * Promise to fetch all users. * @return {Promise} */ - fetchAll(params, populate) { - return strapi.query('plugin::users-permissions.user').findMany({ where: params, populate }); + fetchAll(params) { + return strapi.entityService.findMany('plugin::users-permissions.user', params); }, /** From f8d920a332b79562234bee8df81576fb9fa6ac39 Mon Sep 17 00:00:00 2001 From: harimkims Date: Thu, 3 Mar 2022 22:57:37 +0900 Subject: [PATCH 06/11] Add test to verify population in users-api --- .../tests/content-api/users-api.test.e2e.js | 135 ++++++++++++++++++ 1 file changed, 135 insertions(+) diff --git a/packages/plugins/users-permissions/tests/content-api/users-api.test.e2e.js b/packages/plugins/users-permissions/tests/content-api/users-api.test.e2e.js index e527b369d2..1dc8b67404 100644 --- a/packages/plugins/users-permissions/tests/content-api/users-api.test.e2e.js +++ b/packages/plugins/users-permissions/tests/content-api/users-api.test.e2e.js @@ -8,6 +8,12 @@ const { createContentAPIRequest } = require('../../../../../test/helpers/request let strapi; let rq; +const internals = { + role: { + name: 'Test Role', + description: 'Some random test role', + }, +}; const data = {}; describe('Users API', () => { @@ -20,11 +26,42 @@ describe('Users API', () => { await strapi.destroy(); }); + test('Create and get Role', async () => { + const createRes = await rq({ + method: 'POST', + url: '/users-permissions/roles', + body: { + ...internals.role, + permissions: [], + }, + }); + + expect(createRes.statusCode).toBe(200); + expect(createRes.body).toMatchObject({ ok: true }); + + const findRes = await rq({ + method: 'GET', + url: '/users-permissions/roles', + }); + + expect(findRes.statusCode).toBe(200); + expect(findRes.body.roles).toEqual( + expect.arrayContaining([expect.objectContaining(internals.role)]) + ); + + // eslint-disable-next-line no-unused-vars + const { nb_users, ...role } = findRes.body.roles.find(r => r.name === internals.role.name); + expect(role).toMatchObject(internals.role); + + data.role = role; + }); + test('Create User', async () => { const user = { username: 'User 1', email: 'user1@strapi.io', password: 'test1234', + role: data.role.id, }; const res = await rq({ @@ -37,6 +74,7 @@ describe('Users API', () => { expect(res.body).toMatchObject({ username: user.username, email: user.email, + role: data.role, }); data.user = res.body; @@ -87,6 +125,103 @@ describe('Users API', () => { }, ]); }); + + test('should populate role', async () => { + const res = await rq({ + method: 'GET', + url: '/users?populate=role', + }); + + const { statusCode, body } = res; + + expect(statusCode).toBe(200); + expect(Array.isArray(body)).toBe(true); + expect(body).toHaveLength(1); + expect(body).toMatchObject([ + { + id: expect.anything(), + username: data.user.username, + email: data.user.email, + role: data.role, + }, + ]); + }); + + test('should not populate users in role', async () => { + const res = await rq({ + method: 'GET', + url: '/users?populate[role][populate][0]=users', + }); + + const { statusCode, body } = res; + + expect(statusCode).toBe(200); + expect(Array.isArray(body)).toBe(true); + expect(body).toHaveLength(1); + expect(body).toMatchObject([ + { + id: expect.anything(), + username: data.user.username, + email: data.user.email, + role: data.role, + }, + ]); + expect(body[0].role).not.toHaveProperty('users'); + }); + }); + + describe('Read an user', () => { + test('should populate role', async () => { + const res = await rq({ + method: 'GET', + url: `/users/${data.user.id}?populate=role`, + }); + + const { statusCode, body } = res; + + expect(statusCode).toBe(200); + expect(body).toMatchObject({ + id: data.user.id, + username: data.user.username, + email: data.user.email, + role: data.role, + }); + }); + + test('should not populate role', async () => { + const res = await rq({ + method: 'GET', + url: `/users/${data.user.id}`, + }); + + const { statusCode, body } = res; + + expect(statusCode).toBe(200); + expect(body).toMatchObject({ + id: data.user.id, + username: data.user.username, + email: data.user.email, + }); + expect(body).not.toHaveProperty('role'); + }); + + test('should not populate users in role', async () => { + const res = await rq({ + method: 'GET', + url: `/users/${data.user.id}?populate[role][populate][0]=users`, + }); + + const { statusCode, body } = res; + + expect(statusCode).toBe(200); + expect(body).toMatchObject({ + id: data.user.id, + username: data.user.username, + email: data.user.email, + role: data.role, + }); + expect(body.role).not.toHaveProperty('users'); + }); }); test('Delete user', async () => { From 683f0484f18149f9fd9a467bfbbc574ff6c0f7ee Mon Sep 17 00:00:00 2001 From: harimkims Date: Thu, 3 Mar 2022 23:59:32 +0900 Subject: [PATCH 07/11] Fix wrong input parameter in user update controller --- packages/plugins/users-permissions/server/controllers/user.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/plugins/users-permissions/server/controllers/user.js b/packages/plugins/users-permissions/server/controllers/user.js index 29261837b1..e2fc69b559 100644 --- a/packages/plugins/users-permissions/server/controllers/user.js +++ b/packages/plugins/users-permissions/server/controllers/user.js @@ -90,7 +90,7 @@ module.exports = { const { id } = ctx.params; const { email, username, password } = ctx.request.body; - const user = await getService('user').fetch({ id }); + const user = await getService('user').fetch(id); await validateUpdateUserBody(ctx.request.body); From 0b60be2aca77b400bb39ae2824590abeea97a77d Mon Sep 17 00:00:00 2001 From: chisus Date: Thu, 21 Apr 2022 00:29:26 +0900 Subject: [PATCH 08/11] Add sanitizers registry to container - Remove users-permissions sanitization step in core - Move sanitization functions to users-permissions plugin utils - Add sanitizers registry to container for manage sanitizer functions in core so that we can get/add sanitizers anywhere we want --- packages/core/strapi/lib/Strapi.js | 6 +++++ .../strapi/lib/core/registries/sanitizers.js | 26 +++++++++++++++++++ packages/core/utils/lib/sanitize/index.js | 17 +++++++++--- .../core/utils/lib/sanitize/sanitizers.js | 7 +---- .../core/utils/lib/sanitize/visitors/index.js | 1 - .../users-permissions/server/register.js | 2 ++ .../users-permissions/server/utils/index.js | 3 +++ .../server/utils/sanitize/index.js | 9 +++++++ .../server/utils/sanitize/sanitizers.js | 19 ++++++++++++++ .../server/utils/sanitize/visitors/index.js | 5 ++++ ...remove-user-relation-from-role-entities.js | 0 11 files changed, 84 insertions(+), 11 deletions(-) create mode 100644 packages/core/strapi/lib/core/registries/sanitizers.js create mode 100644 packages/plugins/users-permissions/server/utils/sanitize/index.js create mode 100644 packages/plugins/users-permissions/server/utils/sanitize/sanitizers.js create mode 100644 packages/plugins/users-permissions/server/utils/sanitize/visitors/index.js rename packages/{core/utils/lib => plugins/users-permissions/server/utils}/sanitize/visitors/remove-user-relation-from-role-entities.js (100%) diff --git a/packages/core/strapi/lib/Strapi.js b/packages/core/strapi/lib/Strapi.js index 96b645bdb8..0d55183425 100644 --- a/packages/core/strapi/lib/Strapi.js +++ b/packages/core/strapi/lib/Strapi.js @@ -37,6 +37,7 @@ const apisRegistry = require('./core/registries/apis'); const bootstrap = require('./core/bootstrap'); const loaders = require('./core/loaders'); const { destroyOnSignal } = require('./utils/signals'); +const sanitizersRegistry = require('./core/registries/sanitizers'); // TODO: move somewhere else const draftAndPublishSync = require('./migrations/draft-publish'); @@ -64,6 +65,7 @@ class Strapi { this.container.register('plugins', pluginsRegistry(this)); this.container.register('apis', apisRegistry(this)); this.container.register('auth', createAuth(this)); + this.container.register('sanitizers', sanitizersRegistry(this)); this.dirs = utils.getDirs(rootDir, { strapi: this }); @@ -157,6 +159,10 @@ class Strapi { return this.container.get('auth'); } + get sanitizers() { + return this.container.get('sanitizers'); + } + async start() { try { if (!this.isLoaded) { diff --git a/packages/core/strapi/lib/core/registries/sanitizers.js b/packages/core/strapi/lib/core/registries/sanitizers.js new file mode 100644 index 0000000000..0050020cbc --- /dev/null +++ b/packages/core/strapi/lib/core/registries/sanitizers.js @@ -0,0 +1,26 @@ +'use strict'; + +const _ = require('lodash'); + +const sanitizersRegistry = () => { + const sanitizers = { + 'content-api': { + input: [], + output: [], + }, + }; + + return { + get(path) { + return _.get(sanitizers, path); + }, + add(path, sanitizer) { + this.get(path).push(sanitizer); + }, + has(path) { + return _.has(sanitizers, path); + }, + }; +}; + +module.exports = sanitizersRegistry; diff --git a/packages/core/utils/lib/sanitize/index.js b/packages/core/utils/lib/sanitize/index.js index a5f1e3253b..1a23b95582 100644 --- a/packages/core/utils/lib/sanitize/index.js +++ b/packages/core/utils/lib/sanitize/index.js @@ -28,6 +28,12 @@ module.exports = { transforms.push(traverseEntity(visitors.removeRestrictedRelations(auth), { schema })); } + // Apply sanitizers from registry if exists + const sanitizersRegistry = strapi.container.get('sanitizers').get('content-api.input'); + if (Array.isArray(sanitizersRegistry)) { + sanitizersRegistry.forEach(sanitizer => transforms.push(sanitizer(schema))); + } + return pipeAsync(...transforms)(data); }, @@ -36,15 +42,18 @@ module.exports = { return Promise.all(data.map(entry => this.output(entry, schema, { auth }))); } - const transforms = [ - sanitizers.defaultSanitizeOutput(schema), - sanitizers.sanitizeUserRelationFromRoleEntities(schema), - ]; + const transforms = [sanitizers.defaultSanitizeOutput(schema)]; if (auth) { transforms.push(traverseEntity(visitors.removeRestrictedRelations(auth), { schema })); } + // Apply sanitizers from registry if exists + const sanitizersRegistry = strapi.container.get('sanitizers').get('content-api.output'); + if (Array.isArray(sanitizersRegistry)) { + sanitizersRegistry.forEach(sanitizer => transforms.push(sanitizer(schema))); + } + return pipeAsync(...transforms)(data); }, }, diff --git a/packages/core/utils/lib/sanitize/sanitizers.js b/packages/core/utils/lib/sanitize/sanitizers.js index 2281243c40..87c42812c3 100644 --- a/packages/core/utils/lib/sanitize/sanitizers.js +++ b/packages/core/utils/lib/sanitize/sanitizers.js @@ -5,7 +5,7 @@ const { curry } = require('lodash/fp'); const pipeAsync = require('../pipe-async'); const traverseEntity = require('../traverse-entity'); -const { removePassword, removePrivate, removeUserRelationFromRoleEntities } = require('./visitors'); +const { removePassword, removePrivate } = require('./visitors'); const sanitizePasswords = curry((schema, entity) => { return traverseEntity(removePassword, { schema }, entity); @@ -15,10 +15,6 @@ const sanitizePrivates = curry((schema, entity) => { return traverseEntity(removePrivate, { schema }, entity); }); -const sanitizeUserRelationFromRoleEntities = curry((schema, entity) => { - return traverseEntity(removeUserRelationFromRoleEntities, { schema }, entity); -}); - const defaultSanitizeOutput = curry((schema, entity) => { return pipeAsync(sanitizePrivates(schema), sanitizePasswords(schema))(entity); }); @@ -26,6 +22,5 @@ const defaultSanitizeOutput = curry((schema, entity) => { module.exports = { sanitizePasswords, sanitizePrivates, - sanitizeUserRelationFromRoleEntities, defaultSanitizeOutput, }; diff --git a/packages/core/utils/lib/sanitize/visitors/index.js b/packages/core/utils/lib/sanitize/visitors/index.js index c34067c2b6..b0597d63f5 100644 --- a/packages/core/utils/lib/sanitize/visitors/index.js +++ b/packages/core/utils/lib/sanitize/visitors/index.js @@ -4,7 +4,6 @@ module.exports = { removePassword: require('./remove-password'), removePrivate: require('./remove-private'), removeRestrictedRelations: require('./remove-restricted-relations'), - removeUserRelationFromRoleEntities: require('./remove-user-relation-from-role-entities'), allowedFields: require('./allowed-fields'), restrictedFields: require('./restricted-fields'), }; diff --git a/packages/plugins/users-permissions/server/register.js b/packages/plugins/users-permissions/server/register.js index 17f9e06f83..e10329288d 100644 --- a/packages/plugins/users-permissions/server/register.js +++ b/packages/plugins/users-permissions/server/register.js @@ -1,9 +1,11 @@ 'use strict'; const authStrategy = require('./strategies/users-permissions'); +const sanitizers = require('./utils/sanitize/sanitizers'); module.exports = ({ strapi }) => { strapi.container.get('auth').register('content-api', authStrategy); + strapi.container.get('sanitizers').add('content-api.output', sanitizers.defaultSanitizeOutput); if (strapi.plugin('graphql')) { require('./graphql')({ strapi }); diff --git a/packages/plugins/users-permissions/server/utils/index.js b/packages/plugins/users-permissions/server/utils/index.js index 992640e11d..1287a4540e 100644 --- a/packages/plugins/users-permissions/server/utils/index.js +++ b/packages/plugins/users-permissions/server/utils/index.js @@ -1,9 +1,12 @@ 'use strict'; +const sanitize = require('./sanitize'); + const getService = name => { return strapi.plugin('users-permissions').service(name); }; module.exports = { getService, + sanitize, }; diff --git a/packages/plugins/users-permissions/server/utils/sanitize/index.js b/packages/plugins/users-permissions/server/utils/sanitize/index.js new file mode 100644 index 0000000000..db0884f748 --- /dev/null +++ b/packages/plugins/users-permissions/server/utils/sanitize/index.js @@ -0,0 +1,9 @@ +'use strict'; + +const visitors = require('./visitors'); +const sanitizers = require('./sanitizers'); + +module.exports = { + sanitizers, + visitors, +}; diff --git a/packages/plugins/users-permissions/server/utils/sanitize/sanitizers.js b/packages/plugins/users-permissions/server/utils/sanitize/sanitizers.js new file mode 100644 index 0000000000..940dbd0866 --- /dev/null +++ b/packages/plugins/users-permissions/server/utils/sanitize/sanitizers.js @@ -0,0 +1,19 @@ +'use strict'; + +const { curry } = require('lodash/fp'); +const { traverseEntity, pipeAsync } = require('@strapi/utils'); + +const { removeUserRelationFromRoleEntities } = require('./visitors'); + +const sanitizeUserRelationFromRoleEntities = curry((schema, entity) => { + return traverseEntity(removeUserRelationFromRoleEntities, { schema }, entity); +}); + +const defaultSanitizeOutput = curry((schema, entity) => { + return pipeAsync(sanitizeUserRelationFromRoleEntities(schema))(entity); +}); + +module.exports = { + sanitizeUserRelationFromRoleEntities, + defaultSanitizeOutput, +}; diff --git a/packages/plugins/users-permissions/server/utils/sanitize/visitors/index.js b/packages/plugins/users-permissions/server/utils/sanitize/visitors/index.js new file mode 100644 index 0000000000..bc20d0f5d9 --- /dev/null +++ b/packages/plugins/users-permissions/server/utils/sanitize/visitors/index.js @@ -0,0 +1,5 @@ +'use strict'; + +module.exports = { + removeUserRelationFromRoleEntities: require('./remove-user-relation-from-role-entities'), +}; diff --git a/packages/core/utils/lib/sanitize/visitors/remove-user-relation-from-role-entities.js b/packages/plugins/users-permissions/server/utils/sanitize/visitors/remove-user-relation-from-role-entities.js similarity index 100% rename from packages/core/utils/lib/sanitize/visitors/remove-user-relation-from-role-entities.js rename to packages/plugins/users-permissions/server/utils/sanitize/visitors/remove-user-relation-from-role-entities.js From f6c01bd0133c316279d229445d6c4522b2cd5be2 Mon Sep 17 00:00:00 2001 From: chisus Date: Thu, 21 Apr 2022 01:04:18 +0900 Subject: [PATCH 09/11] Add fake container to pass i18n unit test --- .../server/controllers/__tests__/locales.test.js | 16 ++++++++++++++++ 1 file changed, 16 insertions(+) diff --git a/packages/plugins/i18n/server/controllers/__tests__/locales.test.js b/packages/plugins/i18n/server/controllers/__tests__/locales.test.js index ffa9977b92..d5e23a55ed 100644 --- a/packages/plugins/i18n/server/controllers/__tests__/locales.test.js +++ b/packages/plugins/i18n/server/controllers/__tests__/locales.test.js @@ -4,6 +4,14 @@ const { ApplicationError } = require('@strapi/utils').errors; const { listLocales, createLocale, updateLocale, deleteLocale } = require('../locales'); const localeModel = require('../../content-types/locale'); +const container = { + get() { + return { + get() {}, + }; + }, +}; + describe('Locales', () => { describe('listLocales', () => { test('can get locales', async () => { @@ -24,6 +32,7 @@ describe('Locales', () => { }, }, }, + container, }; const ctx = {}; @@ -61,6 +70,7 @@ describe('Locales', () => { }, }, }, + container, }; const ctx = { request: { body: { ...locale, isDefault: true } }, state: { user: { id: 1 } } }; @@ -96,6 +106,7 @@ describe('Locales', () => { }, }, }, + container, }; const ctx = { @@ -133,6 +144,7 @@ describe('Locales', () => { }, }, }, + container, }; const ctx = { @@ -180,6 +192,7 @@ describe('Locales', () => { }, }, }, + container, }; const ctx = { @@ -221,6 +234,7 @@ describe('Locales', () => { }, }, }, + container, }; const ctx = { @@ -269,6 +283,7 @@ describe('Locales', () => { }, }, }, + container, }; const ctx = { params: { id: 1 } }; @@ -302,6 +317,7 @@ describe('Locales', () => { }, }, }, + container, }; const ctx = { params: { id: 1 } }; From 59597883522e242c58f1b1c8771520d23e6b80dd Mon Sep 17 00:00:00 2001 From: harimkims Date: Sat, 7 May 2022 17:04:12 +0900 Subject: [PATCH 10/11] Refactor sanitizers module to be maintained easily --- packages/core/strapi/lib/Strapi.js | 5 +++++ packages/core/strapi/lib/core/loaders/index.js | 1 + .../core/strapi/lib/core/loaders/sanitizers.js | 5 +++++ .../core/strapi/lib/core/registries/sanitizers.js | 14 +++++++------- packages/core/utils/lib/sanitize/index.js | 14 ++++++-------- .../plugins/users-permissions/server/register.js | 2 +- 6 files changed, 25 insertions(+), 16 deletions(-) create mode 100644 packages/core/strapi/lib/core/loaders/sanitizers.js diff --git a/packages/core/strapi/lib/Strapi.js b/packages/core/strapi/lib/Strapi.js index 0d55183425..8f61901a07 100644 --- a/packages/core/strapi/lib/Strapi.js +++ b/packages/core/strapi/lib/Strapi.js @@ -310,6 +310,10 @@ class Strapi { this.app = await loaders.loadSrcIndex(this); } + async loadSanitizers() { + await loaders.loadSanitizers(this); + } + registerInternalHooks() { this.container.get('hooks').set('strapi::content-types.beforeSync', createAsyncParallelHook()); this.container.get('hooks').set('strapi::content-types.afterSync', createAsyncParallelHook()); @@ -321,6 +325,7 @@ class Strapi { async register() { await Promise.all([ this.loadApp(), + this.loadSanitizers(), this.loadPlugins(), this.loadAdmin(), this.loadAPIs(), diff --git a/packages/core/strapi/lib/core/loaders/index.js b/packages/core/strapi/lib/core/loaders/index.js index 42bdf6c702..9738dc5a59 100644 --- a/packages/core/strapi/lib/core/loaders/index.js +++ b/packages/core/strapi/lib/core/loaders/index.js @@ -8,4 +8,5 @@ module.exports = { loadPolicies: require('./policies'), loadPlugins: require('./plugins'), loadAdmin: require('./admin'), + loadSanitizers: require('./sanitizers'), }; diff --git a/packages/core/strapi/lib/core/loaders/sanitizers.js b/packages/core/strapi/lib/core/loaders/sanitizers.js new file mode 100644 index 0000000000..0f1409bcec --- /dev/null +++ b/packages/core/strapi/lib/core/loaders/sanitizers.js @@ -0,0 +1,5 @@ +'use strict'; + +module.exports = strapi => { + strapi.container.get('sanitizers').set('content-api', { input: [], output: [] }); +}; diff --git a/packages/core/strapi/lib/core/registries/sanitizers.js b/packages/core/strapi/lib/core/registries/sanitizers.js index 0050020cbc..0dfc4e19c8 100644 --- a/packages/core/strapi/lib/core/registries/sanitizers.js +++ b/packages/core/strapi/lib/core/registries/sanitizers.js @@ -3,19 +3,19 @@ const _ = require('lodash'); const sanitizersRegistry = () => { - const sanitizers = { - 'content-api': { - input: [], - output: [], - }, - }; + const sanitizers = {}; return { get(path) { - return _.get(sanitizers, path); + return _.get(sanitizers, path, []); }, add(path, sanitizer) { this.get(path).push(sanitizer); + return this; + }, + set(path, value = []) { + _.set(sanitizers, path, value); + return this; }, has(path) { return _.has(sanitizers, path); diff --git a/packages/core/utils/lib/sanitize/index.js b/packages/core/utils/lib/sanitize/index.js index 1a23b95582..a97be07ca5 100644 --- a/packages/core/utils/lib/sanitize/index.js +++ b/packages/core/utils/lib/sanitize/index.js @@ -29,10 +29,9 @@ module.exports = { } // Apply sanitizers from registry if exists - const sanitizersRegistry = strapi.container.get('sanitizers').get('content-api.input'); - if (Array.isArray(sanitizersRegistry)) { - sanitizersRegistry.forEach(sanitizer => transforms.push(sanitizer(schema))); - } + strapi.sanitizers + .get('content-api.input') + .forEach(sanitizer => transforms.push(sanitizer(schema))); return pipeAsync(...transforms)(data); }, @@ -49,10 +48,9 @@ module.exports = { } // Apply sanitizers from registry if exists - const sanitizersRegistry = strapi.container.get('sanitizers').get('content-api.output'); - if (Array.isArray(sanitizersRegistry)) { - sanitizersRegistry.forEach(sanitizer => transforms.push(sanitizer(schema))); - } + strapi.sanitizers + .get('content-api.output') + .forEach(sanitizer => transforms.push(sanitizer(schema))); return pipeAsync(...transforms)(data); }, diff --git a/packages/plugins/users-permissions/server/register.js b/packages/plugins/users-permissions/server/register.js index e10329288d..b433ab622c 100644 --- a/packages/plugins/users-permissions/server/register.js +++ b/packages/plugins/users-permissions/server/register.js @@ -5,7 +5,7 @@ const sanitizers = require('./utils/sanitize/sanitizers'); module.exports = ({ strapi }) => { strapi.container.get('auth').register('content-api', authStrategy); - strapi.container.get('sanitizers').add('content-api.output', sanitizers.defaultSanitizeOutput); + strapi.sanitizers.add('content-api.output', sanitizers.defaultSanitizeOutput); if (strapi.plugin('graphql')) { require('./graphql')({ strapi }); From e918293fc413bfc97f81b8e84aa5bc2a423cd6ea Mon Sep 17 00:00:00 2001 From: harimkims Date: Sat, 7 May 2022 17:27:15 +0900 Subject: [PATCH 11/11] Update sanitizer mocking in i18n test --- .../controllers/__tests__/locales.test.js | 22 +++++++++---------- 1 file changed, 10 insertions(+), 12 deletions(-) diff --git a/packages/plugins/i18n/server/controllers/__tests__/locales.test.js b/packages/plugins/i18n/server/controllers/__tests__/locales.test.js index d5e23a55ed..d9ff092fd5 100644 --- a/packages/plugins/i18n/server/controllers/__tests__/locales.test.js +++ b/packages/plugins/i18n/server/controllers/__tests__/locales.test.js @@ -4,11 +4,9 @@ const { ApplicationError } = require('@strapi/utils').errors; const { listLocales, createLocale, updateLocale, deleteLocale } = require('../locales'); const localeModel = require('../../content-types/locale'); -const container = { +const sanitizers = { get() { - return { - get() {}, - }; + return []; }, }; @@ -32,7 +30,7 @@ describe('Locales', () => { }, }, }, - container, + sanitizers, }; const ctx = {}; @@ -70,7 +68,7 @@ describe('Locales', () => { }, }, }, - container, + sanitizers, }; const ctx = { request: { body: { ...locale, isDefault: true } }, state: { user: { id: 1 } } }; @@ -106,7 +104,7 @@ describe('Locales', () => { }, }, }, - container, + sanitizers, }; const ctx = { @@ -144,7 +142,7 @@ describe('Locales', () => { }, }, }, - container, + sanitizers, }; const ctx = { @@ -192,7 +190,7 @@ describe('Locales', () => { }, }, }, - container, + sanitizers, }; const ctx = { @@ -234,7 +232,7 @@ describe('Locales', () => { }, }, }, - container, + sanitizers, }; const ctx = { @@ -283,7 +281,7 @@ describe('Locales', () => { }, }, }, - container, + sanitizers, }; const ctx = { params: { id: 1 } }; @@ -317,7 +315,7 @@ describe('Locales', () => { }, }, }, - container, + sanitizers, }; const ctx = { params: { id: 1 } };