Merge pull request #11960 from iicdii/fix/populate-user

Fix unable to populate User in Users-Permissions

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) {
@@ -304,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());
@@ -315,6 +325,7 @@ class Strapi {
   async register() {
     await Promise.all([
       this.loadApp(),
+      this.loadSanitizers(),
       this.loadPlugins(),
       this.loadAdmin(),
       this.loadAPIs(),

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'),
 };

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: [] });
+};

packages/core/strapi/lib/core/registries/sanitizers.js

@@ -0,0 +1,26 @@
+'use strict';
+
+const _ = require('lodash');
+
+const sanitizersRegistry = () => {
+  const sanitizers = {};
+
+  return {
+    get(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);
+    },
+  };
+};
+
+module.exports = sanitizersRegistry;

packages/core/utils/lib/sanitize/index.js

@@ -28,6 +28,11 @@ module.exports = {
         transforms.push(traverseEntity(visitors.removeRestrictedRelations(auth), { schema }));
       }
 
+      // Apply sanitizers from registry if exists
+      strapi.sanitizers
+        .get('content-api.input')
+        .forEach(sanitizer => transforms.push(sanitizer(schema)));
+
       return pipeAsync(...transforms)(data);
     },
 
@@ -42,6 +47,11 @@ module.exports = {
         transforms.push(traverseEntity(visitors.removeRestrictedRelations(auth), { schema }));
       }
 
+      // Apply sanitizers from registry if exists
+      strapi.sanitizers
+        .get('content-api.output')
+        .forEach(sanitizer => transforms.push(sanitizer(schema)));
+
       return pipeAsync(...transforms)(data);
     },
   },

packages/plugins/i18n/server/controllers/__tests__/locales.test.js

@@ -4,6 +4,12 @@ const { ApplicationError } = require('@strapi/utils').errors;
 const { listLocales, createLocale, updateLocale, deleteLocale } = require('../locales');
 const localeModel = require('../../content-types/locale');
 
+const sanitizers = {
+  get() {
+    return [];
+  },
+};
+
 describe('Locales', () => {
   describe('listLocales', () => {
     test('can get locales', async () => {
@@ -24,6 +30,7 @@ describe('Locales', () => {
             },
           },
         },
+        sanitizers,
       };
 
       const ctx = {};
@@ -61,6 +68,7 @@ describe('Locales', () => {
             },
           },
         },
+        sanitizers,
       };
 
       const ctx = { request: { body: { ...locale, isDefault: true } }, state: { user: { id: 1 } } };
@@ -96,6 +104,7 @@ describe('Locales', () => {
             },
           },
         },
+        sanitizers,
       };
 
       const ctx = {
@@ -133,6 +142,7 @@ describe('Locales', () => {
             },
           },
         },
+        sanitizers,
       };
 
       const ctx = {
@@ -180,6 +190,7 @@ describe('Locales', () => {
             },
           },
         },
+        sanitizers,
       };
 
       const ctx = {
@@ -221,6 +232,7 @@ describe('Locales', () => {
             },
           },
         },
+        sanitizers,
       };
 
       const ctx = {
@@ -269,6 +281,7 @@ describe('Locales', () => {
             },
           },
         },
+        sanitizers,
       };
 
       const ctx = { params: { id: 1 } };
@@ -302,6 +315,7 @@ describe('Locales', () => {
             },
           },
         },
+        sanitizers,
       };
 
       const ctx = { params: { id: 1 } };

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 });
   },

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 });
   },

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);
 
@@ -133,8 +133,8 @@ 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);
 
     ctx.body = await Promise.all(users.map(user => sanitizeOutput(user, ctx)));
   },
@@ -145,7 +145,9 @@ module.exports = {
    */
   async findOne(ctx) {
     const { id } = ctx.params;
-    let data = await getService('user').fetch({ id });
+    const { query } = ctx;
+
+    let data = await getService('user').fetch(id, query);
 
     if (data) {
       data = await sanitizeOutput(data, ctx);

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.sanitizers.add('content-api.output', sanitizers.defaultSanitizeOutput);
 
   if (strapi.plugin('graphql')) {
     require('./graphql')({ strapi });

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: [
         {

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',

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) {

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);
   },
 
   /**

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,
 };

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,
+};

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,
+};

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'),
+};

packages/plugins/users-permissions/server/utils/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);
+  }
+};

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 () => {