Fix unable to populate User in Users-Permissions #11960

iicdii · 2021-12-21T01:47:38Z

Signed-off-by: harimkims harimkims@gmail.com

What does it do?

Fix unable to populate User/Users in Users-Permissions plugin

Why is it needed?

To enable population for user/users

How to test it?

Create user
Send GET request http://localhost:1337/api/users?populate=*
Send GET request http://localhost:1337/api/users/<userId>?populate=*

Related issue(s)/PR(s)

Closes #11957

Signed-off-by: harimkims <harimkims@gmail.com>

codecov · 2021-12-21T01:54:03Z

Codecov Report

Merging #11960 (e918293) into master (1db90d8) will decrease coverage by 0.01%.
The diff coverage is 25.00%.

@@            Coverage Diff             @@
##           master   #11960      +/-   ##
==========================================
- Coverage   47.89%   47.88%   -0.02%     
==========================================
  Files         231      231              
  Lines        8629     8633       +4     
  Branches     1935     1935              
==========================================
+ Hits         4133     4134       +1     
- Misses       3697     3700       +3     
  Partials      799      799

Flag	Coverage Δ
front	`?`
unit	`47.88% <25.00%> (-0.02%)`	⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files	Coverage Δ
packages/core/utils/lib/sanitize/index.js	`53.84% <25.00%> (-5.25%)`	⬇️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 1db90d8...e918293. Read the comment docs.

derrickmehaffy · 2021-12-21T05:22:55Z

@iicdii you are amazing, I hope you know that. 🤗

derrickmehaffy · 2021-12-21T05:24:45Z

Alex/others I've added this to the v4.0.1 milestone just in hopes of pushing it through on release as we are getting -a lot- of questions about this.

If it's not possible feel free to remove.

derrickmehaffy · 2021-12-21T05:25:25Z

This pull request has been mentioned on Strapi Community Forum. There might be relevant details there:

https://forum.strapi.io/t/populate-users-permissions-to-get-user-avatar/13701/2

iicdii · 2021-12-21T06:12:13Z

Thanks! 😊 Yeah I've already seen many questions about this issue on Discord too. Hope it could be merged into v4.0.1!

derrickmehaffy · 2021-12-21T11:12:53Z

This pull request has been mentioned on Strapi Community Forum. There might be relevant details there:

https://forum.strapi.io/t/strapi-v4-no-relation-fields-in-user-collection-types/13226/8

Convly

Thanks for opening yet another PR! 🙌

Tbh, I don't think this logic belongs to the REST controller. Even if for now the GQL resolvers are directly calling UP controllers (it's temporary), the goal is to make them both call a common service where we could put common transformations etc...

Also, I'm kinda curious about why we're not using the entity service API to fetch the entries in the UP's user service. Since the convertPopulate & other transformers would be called automatically.

@alexandrebodin do you remember why this choice has been made? Is there a particular reason? Or could we replace those queries with entity service's ones?

tl;dr: this logic is already handled by the entity service, maybe we can make the UP user service use it instead of doing DB queries directly + it would create a common API for both REST controllers & GraphQL resolvers.

iicdii · 2021-12-21T14:00:48Z

Actually I've tried to convert them to entityService in previous PR, He said it like below which is the reason why I don't use entityService

I'm not sure this is something we want to move forward with as it only changes the find but doesn't change the findOne etc.

This API is not like the autogenerated ones by design and only supports filters passed at the root level of the query.
You are not supposed to be able to pass down sorting or limit for example.

I agree it would be better to make it consistent but we have some security concerns to take into account first.
#11795 (comment)

derrickmehaffy · 2021-12-21T14:24:33Z

@iicdii / @Convly I also don't know why we don't move to ES, the U&P plugin is quite a fews behind on rewrites to get it anywhere close to the other plugins.

Convly · 2021-12-21T15:10:38Z

Then we'll keep it that way and convert everything to ES later. But we'll definitely have to take care of this ASAP imo.

Convly · 2021-12-21T15:10:57Z

Could you update your branch with master, please?

Convly · 2021-12-21T15:11:47Z

@iicdii / @Convly I also don't know why we don't move to ES, the U&P plugin is quite a fews behind on rewrites to get it anywhere close to the other plugins.

Definitely a topic we'll need to tackle as soon as we have some bandwidth!

derrickmehaffy · 2021-12-21T22:11:30Z

@iicdii / @Convly I also don't know why we don't move to ES, the U&P plugin is quite a fews behind on rewrites to get it anywhere close to the other plugins.

Definitely a topic we'll need to tackle as soon as we have some bandwidth!

Ah might have gotten skipped in the rush for v4 launch. Can take that later on aye

alexandrebodin · 2021-12-22T08:18:07Z

FYI this API never was meant to have the same features as the auto-generated content APIs.

There are some security concerns around users and we just cannot expose a complete populate option without more considerations 1st. I'll flag the PR so we can review the logic thoroughly before moving forward

derrickmehaffy · 2021-12-26T04:03:38Z

This pull request has been mentioned on Strapi Community Forum. There might be relevant details there:

https://forum.strapi.io/t/strapi-v4-no-relation-fields-in-user-collection-types/13226/12

georgeiliadis91 · 2022-04-26T18:21:36Z

@jonasstams best bet is to create custom endpoints on the server-strapi.js file and override the existing ones.

Convly

Thanks so much for your commitment & your dedication to this @iicdii.
I've added some comments, tell me what you think 🙂

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

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

packages/plugins/users-permissions/server/register.js

Convly

LGTM, nice job, I think both the community & the Strapi team owes you a big one for this.
I'll play a bit with it before merging it. I'll try to do it fast so that it can be part of the next release tomorrow.

alexandrebodin

Thank you for this fix @iicdii 💯

We will probably have to rethink the sanitizers approach soon but this is a working solution that will please a lot of users so we will merge it now and handle the core code soon.

forntoh · 2022-05-11T08:56:05Z

Thank you for this fix @iicdii 💯

We will probably have to rethink the sanitizers approach soon but this is a working solution that will please a lot of users so we will merge it now and handle the core code soon.

We've been waiting for this and finally it's here, @iicdii you're amazing ☺️.

umair-me · 2022-05-11T09:05:48Z

Thank you @iicdii 🙏

Nikoxx99 · 2022-05-11T13:17:48Z

@iicdii Thank you! what a legend

M1CK431 · 2022-05-11T23:38:41Z

Hi @iicdii and thanks A LOT for your work! ❤️

However I have a question: how to handle extra fields in User since using

register({ strapi }) {
    const extensionService = strapi.plugin("graphql").service("extension");
    extensionService.use(({ nexus }) => ({
      types: [
        nexus.extendType({
          type: "UsersPermissionsMe",
          definition(t) {
            // here define fields you need
            t.string("firstName");
            t.string("lastName");
            t.field("acl", { type: "JSON" });
            t.dateTime("deletedAt");
          }
        })
      ]
    }));
  },

in index.js then trying to mutate acl (for example) produce this error:

[2022-05-12 01:26:46.519] error: update `up_users` set `blocked` = false, `acl` = '{\"userManagement\":false}', `updated_at` = '2022-05-12 01:26:46.489' where (`id` = 1) - ER_BAD_FIELD_ERROR: Unknown column 'acl' in 'field list'
Error: ER_BAD_FIELD_ERROR: Unknown column 'acl' in 'field list'
    at Query.Sequence._packetToError (/home/tuut/Projects/biip/api/node_modules/mysql/lib/protocol/sequences/Sequence.js:47:14)
    at Query.ErrorPacket (/home/tuut/Projects/biip/api/node_modules/mysql/lib/protocol/sequences/Query.js:79:18)
    at Protocol._parsePacket (/home/tuut/Projects/biip/api/node_modules/mysql/lib/protocol/Protocol.js:291:23)
    at Parser._parsePacket (/home/tuut/Projects/biip/api/node_modules/mysql/lib/protocol/Parser.js:433:10)
    at Parser.write (/home/tuut/Projects/biip/api/node_modules/mysql/lib/protocol/Parser.js:43:10)
    at Protocol.write (/home/tuut/Projects/biip/api/node_modules/mysql/lib/protocol/Protocol.js:38:16)
    at Socket.<anonymous> (/home/tuut/Projects/biip/api/node_modules/mysql/lib/Connection.js:88:28)
    at Socket.<anonymous> (/home/tuut/Projects/biip/api/node_modules/mysql/lib/Connection.js:526:10)
    at Socket.emit (node:events:390:28)
    at Socket.emit (node:domain:475:12)
    at addChunk (node:internal/streams/readable:315:12)
    at readableAddChunk (node:internal/streams/readable:289:9)
    at Socket.Readable.push (node:internal/streams/readable:228:10)
    at TCP.onStreamRead (node:internal/stream_base_commons:199:23)
    --------------------
    at Protocol._enqueue (/home/tuut/Projects/biip/api/node_modules/mysql/lib/protocol/Protocol.js:144:48)
    at Connection.query (/home/tuut/Projects/biip/api/node_modules/mysql/lib/Connection.js:198:25)
    at /home/tuut/Projects/biip/api/node_modules/knex/lib/dialects/mysql/index.js:132:18
    at new Promise (<anonymous>)
    at Client_MySQL._query (/home/tuut/Projects/biip/api/node_modules/knex/lib/dialects/mysql/index.js:126:12)
    at executeQuery (/home/tuut/Projects/biip/api/node_modules/knex/lib/execution/internal/query-executioner.js:37:17)
    at Client_MySQL.query (/home/tuut/Projects/biip/api/node_modules/knex/lib/client.js:146:12)
    at Runner.query (/home/tuut/Projects/biip/api/node_modules/knex/lib/execution/runner.js:130:36)
    at ensureConnectionCallback (/home/tuut/Projects/biip/api/node_modules/knex/lib/execution/internal/ensure-connection-callback.js:13:17)
    at Runner.ensureConnection (/home/tuut/Projects/biip/api/node_modules/knex/lib/execution/runner.js:307:20)
    at runMicrotasks (<anonymous>)
    at processTicksAndRejections (node:internal/process/task_queues:96:5)
    at async Runner.run (/home/tuut/Projects/biip/api/node_modules/knex/lib/execution/runner.js:30:19)
    at async Object.execute (/home/tuut/Projects/biip/api/node_modules/@strapi/database/lib/query/query-builder.js:354:22)
    at async Object.update (/home/tuut/Projects/biip/api/node_modules/@strapi/database/lib/entity-manager.js:247:9)
    at async Object.update (/home/tuut/Projects/biip/api/node_modules/@strapi/strapi/lib/services/entity-service/index.js:219:18)

Also, the added fields are still present in GraphQL documentation but doesn't return any data while fetched.
For now, because of that, I'm forced to revert on v4.1.9 despite I was waiting for a very long time after this issue fix 😞

EDIT: after rollbacking on v4.1.9, I was still facing the same issue. Deeper debug reveal that my acl and deletedAt fields where dropped from database! After re-creating them, everything works as expected (incl. in v4.1.10). However, I dunno what happens exactly which leads to drop some columns in my database... 🤷‍♂️

msoler75 · 2022-05-17T18:42:35Z

I'm not sure if I'am doing well but even after version update to 4.1.10 I can't populate fields with the /api/users/me controller

e.g.:

/api/users/me?[populate][relation]=*
/api/users/me?relation=*

So I put this code based on some code I found somewhere here:

// ISSUE: 
// https://github.com/strapi/strapi/issues/11957

const Query = require("mysql/lib/protocol/sequences/Query");

// BUGFIX
module.exports = plugin => {
    const sanitizeOutput = (user) => {
      const {
        password,
        resetPasswordToken,
        confirmationToken,
        ...sanitizedUser
      } = user; // be careful, you need to omit other private attributes yourself
      return sanitizedUser;
    };
  
    plugin.controllers.user.me = async (ctx) => {
      if (!ctx.state.user) {
        return ctx.unauthorized();
      }
      const pop = ctx.query.populate
      const user = await strapi.entityService.findOne(
        'plugin::users-permissions.user',
        ctx.state.user.id, {
          populate: pop ? pop : ['role' /* extra default populated fields here */]
        }
      );
  
      ctx.body = sanitizeOutput(user);
    };
  
    return plugin;
  };

bjorelid · 2022-05-28T18:28:43Z

I'm not sure if I'am doing well but even after version update to 4.1.10 I can't populate fields with the /api/users/me controller

e.g.:

/api/users/me?[populate][relation]=* /api/users/me?relation=*

So I put this code based on some code I found somewhere here:

// ISSUE: 
// https://github.com/strapi/strapi/issues/11957

const Query = require("mysql/lib/protocol/sequences/Query");

// BUGFIX
module.exports = plugin => {
    const sanitizeOutput = (user) => {
      const {
        password,
        resetPasswordToken,
        confirmationToken,
        ...sanitizedUser
      } = user; // be careful, you need to omit other private attributes yourself
      return sanitizedUser;
    };
  
    plugin.controllers.user.me = async (ctx) => {
      if (!ctx.state.user) {
        return ctx.unauthorized();
      }
      const pop = ctx.query.populate
      const user = await strapi.entityService.findOne(
        'plugin::users-permissions.user',
        ctx.state.user.id, {
          populate: pop ? pop : ['role' /* extra default populated fields here */]
        }
      );
  
      ctx.body = sanitizeOutput(user);
    };
  
    return plugin;
  };

I'm experiencing the same problem with version 4.1.12. Populating works on 'api/users' but not on 'api/users/me'.

rvachev · 2022-06-23T05:23:24Z

I'm not sure if I'am doing well but even after version update to 4.1.10 I can't populate fields with the /api/users/me controller
e.g.:
/api/users/me?[populate][relation]=* /api/users/me?relation=*
So I put this code based on some code I found somewhere here:

// ISSUE: 
// https://github.com/strapi/strapi/issues/11957

const Query = require("mysql/lib/protocol/sequences/Query");

// BUGFIX
module.exports = plugin => {
    const sanitizeOutput = (user) => {
      const {
        password,
        resetPasswordToken,
        confirmationToken,
        ...sanitizedUser
      } = user; // be careful, you need to omit other private attributes yourself
      return sanitizedUser;
    };
  
    plugin.controllers.user.me = async (ctx) => {
      if (!ctx.state.user) {
        return ctx.unauthorized();
      }
      const pop = ctx.query.populate
      const user = await strapi.entityService.findOne(
        'plugin::users-permissions.user',
        ctx.state.user.id, {
          populate: pop ? pop : ['role' /* extra default populated fields here */]
        }
      );
  
      ctx.body = sanitizeOutput(user);
    };
  
    return plugin;
  };

I'm experiencing the same problem with version 4.1.12. Populating works on 'api/users' but not on 'api/users/me'.

The same with 4.2.0😫

Convly · 2022-06-30T09:27:02Z

Hey folks, I've tried reproducing the issue but everything seems to work fine on the /users/me endpoint with a populate.

Does anyone have a repository or a small video/gif I could look at to try to find the issue?

rvachev · 2022-06-30T10:19:17Z

Hey folks, I've tried reproducing the issue but everything seems to work fine on the /users/me endpoint with a populate.

Does anyone have a repository or a small video/gif I could look at to try to find the issue?

before the strapi version 4.2.2 populate on /users/me didn't work

Puvvl · 2022-07-27T12:18:01Z

Seems it had been fixed in 4.2.2

derrickmehaffy · 2022-07-27T21:48:47Z

Going to lock this as it's pretty much confirmed that it's fixed as of v4.2.2+

If anyone else hits this issue or something like it please open a new bug report.

Fix unable to populate user

5ba0d2d

Signed-off-by: harimkims <harimkims@gmail.com>

Fix e2e test

54d10ec

derrickmehaffy requested review from Convly and alexandrebodin December 21, 2021 05:23

derrickmehaffy added this to the 4.0.1 milestone Dec 21, 2021

derrickmehaffy added the issue: bug Issue reporting a bug label Dec 21, 2021

Convly reviewed Dec 21, 2021

View reviewed changes

joh-sch mentioned this pull request Dec 21, 2021

Population does not work for Users in Users-Permissions #11957

Closed

Merge branch 'master' into fix/populate-user

8ad9185

alexandrebodin added the flag: don't merge This PR should not be merged at the moment label Dec 22, 2021

alexandrebodin modified the milestones: 4.0.1, 4.0.2, 4.0.3 Dec 22, 2021

alexandrebodin modified the milestones: 4.0.3, 4.0.4 Jan 5, 2022

Convly self-assigned this May 2, 2022

Convly requested changes May 2, 2022

View reviewed changes

Convly added source: plugin:users-permissions Source is plugin/users-permissions package and removed flag: don't merge This PR should not be merged at the moment labels May 2, 2022

iicdii added 3 commits May 7, 2022 16:45

Merge branch 'strapi:master' into fix/populate-user

4429381

Refactor sanitizers module to be maintained easily

5959788

Update sanitizer mocking in i18n test

e918293

Convly approved these changes May 10, 2022

View reviewed changes

Convly added this to the 4.1.10 milestone May 10, 2022

alexandrebodin approved these changes May 11, 2022

View reviewed changes

Convly merged commit a727b1f into strapi:master May 11, 2022

klikkn mentioned this pull request May 11, 2022

Fix email confirmation #13292

Merged

klikkn mentioned this pull request May 12, 2022

Email confirmation is failed #13297

Closed

M1CK431 mentioned this pull request May 12, 2022

Can't add field in user (users-permissions) #12016

Closed

2 tasks

derrickmehaffy mentioned this pull request Jun 1, 2022

Filters not working on the users-persmission plugin #12303

Closed

christhoperbernard mentioned this pull request Jun 15, 2022

Request to support getting media similarly to the referenced fix #13559

Closed

Convly mentioned this pull request Jun 30, 2022

User needs populate the role information #11990

Closed

strapi locked and limited conversation to collaborators Jul 27, 2022

Uh oh!

Fix unable to populate User in Users-Permissions #11960

Fix unable to populate User in Users-Permissions #11960

Uh oh!

Conversation

iicdii commented Dec 21, 2021

What does it do?

Why is it needed?

How to test it?

Related issue(s)/PR(s)

Uh oh!

codecov bot commented Dec 21, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

derrickmehaffy commented Dec 21, 2021

Uh oh!

derrickmehaffy commented Dec 21, 2021

Uh oh!

derrickmehaffy commented Dec 21, 2021

Uh oh!

iicdii commented Dec 21, 2021

Uh oh!

derrickmehaffy commented Dec 21, 2021

Uh oh!

Convly left a comment

Choose a reason for hiding this comment

Uh oh!

iicdii commented Dec 21, 2021

Uh oh!

derrickmehaffy commented Dec 21, 2021

Uh oh!

Convly commented Dec 21, 2021

Uh oh!

Convly commented Dec 21, 2021

Uh oh!

Convly commented Dec 21, 2021

Uh oh!

derrickmehaffy commented Dec 21, 2021

Uh oh!

alexandrebodin commented Dec 22, 2021

Uh oh!

derrickmehaffy commented Dec 26, 2021

Uh oh!

georgeiliadis91 commented Apr 26, 2022

Uh oh!

Convly left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Convly left a comment

Choose a reason for hiding this comment

Uh oh!

alexandrebodin left a comment

Choose a reason for hiding this comment

Uh oh!

forntoh commented May 11, 2022

Uh oh!

umair-me commented May 11, 2022

Uh oh!

Nikoxx99 commented May 11, 2022

Uh oh!

M1CK431 commented May 11, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

msoler75 commented May 17, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

bjorelid commented May 28, 2022

Uh oh!

rvachev commented Jun 23, 2022

Uh oh!

Convly commented Jun 30, 2022

codecov bot commented Dec 21, 2021 •

edited

Loading

M1CK431 commented May 11, 2022 •

edited

Loading

msoler75 commented May 17, 2022 •

edited

Loading