Skip to content

LiveQuery on Object Role Based don't send events for some users #5131

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
Moumouls opened this issue Oct 18, 2018 · 9 comments
Closed

LiveQuery on Object Role Based don't send events for some users #5131

Moumouls opened this issue Oct 18, 2018 · 9 comments

Comments

@Moumouls
Copy link
Member

Issue Description

When a Parse.User is linked in a more than 100 Parse.Role, a LiveQuery on a Role based Parse.Object send events in an unpredictable way, or even not sending events any more !

Steps to reproduce

Make a LiveQuery on Role Based Object with a Parse.User linked in more than 100 Parse.Role

Expected Results

The LiveQuery from the Parse.User receive events correctly

Actual Outcome

The LiveQuery from Parse.User not receive events or receive events in an unpredictable way

Environment Setup

  • Server

    • parse-server version : 3, 3.1
    • Operating System: Ubuntu LTS
    • Hardware: VPS
    • Localhost or remote server? (AWS, Heroku, Azure, Digital Ocean, etc): Remote
  • Database

    • MongoDB version: 3
    • Storage engine: AWS
    • Hardware: VPS
    • Localhost or remote server? (AWS, mLab, ObjectRocket, Digital Ocean, etc): remote

Logs/Trace

Logs For a User that have more than 100 Parse.Role

show that the matchACL return false value

2018-10-17T19:05:06.909Z - Original null | Current {...,"ACL":{"role:xxxxxxxxx":{"read":true,"write":true}},"__type":"Object","className":"Aptitude","objectId":"xxxxxxxx"} | Match: false, true, false, false(isCurrentMatched) | Query: ClassName:fieldOfTheQuery|["valueForTheQuery"]

The ACL function fail because isCurrentMatched equal to false

Logs For a User that have less than 100 Parse.Role

show that the matchACL return true value

2018-10-17T19:05:06.909Z - Original null | Current {...,"ACL":{"role:xxxxxxxxx":{"read":true,"write":true}},"__type":"Object","className":"Aptitude","objectId":"xxxxxxxx"} | Match: false, true, false, true(isCurrentMatched) | Query: ClassName:fieldOfTheQuery|["valueForTheQuery"]

The ACL function success because isCurrentMatched equal to true

Investigation on Parse.Server 3.1.0

  • Target File : Auth.js
  • Target Function : getRolesForUser()
  • Bug Identified : Parse.Query return just 100 results by default
Auth.prototype.getRolesForUser = function() {
  if (this.config) {
    const restWhere = {
      users: {
        __type: 'Pointer',
        className: '_User',
        objectId: this.user.id,
      },
    };
    const query = new RestQuery(
      this.config,
      master(this.config),
      '_Role',
      restWhere,
      {}
    );
    return query.execute().then(({ results }) => results);
  }
  
  //!!!!!!The Bug is right there
  
  return new Parse.Query(Parse.Role)
    .equalTo('users', this.user)
    .find({ useMasterKey: true })
    .then(results => results.map(obj => obj.toJSON()));
};
  • Fast Option for fix that works (tested on my env) : add a .limit(99999)
  • Disadvantages : For a Parse.User with a lot (a lot) of Parse.Role, memory leak can appear
Auth.prototype.getRolesForUser = function() {
  if (this.config) {
    const restWhere = {
      users: {
        __type: 'Pointer',
        className: '_User',
        objectId: this.user.id,
      },
    };
    const query = new RestQuery(
      this.config,
      master(this.config),
      '_Role',
      restWhere,
      {}
    );
    return query.execute().then(({ results }) => results);
  }
  
  //!!!!!!The Bug is right there
  
  return new Parse.Query(Parse.Role)
  	.limit(999999999)
    .equalTo('users', this.user)
    .find({ useMasterKey: true })
    .then(results => results.map(obj => obj.toJSON()));
};

@flovilmart what I was talking about on Twitter

@flovilmart
Copy link
Contributor

You should probably not have that many roles on a single user

@flovilmart
Copy link
Contributor

Jokes aside, we should use the .each instead of the query you’re right :) and use the iteration callback to populate an array of all roles. Is it something you’re be willing to make a Pr for?

@Moumouls
Copy link
Member Author

Moumouls commented Oct 18, 2018

In effect .each() is the appropriate solution. :)

@acinader
Copy link
Contributor

acinader commented Oct 18, 2018

This is about as good an example of a very simple, contained improvement imaginable.

@flovilmart
Copy link
Contributor

@Moumouls so what do you expect to happen? My best guest, the stale bot will close it :)

@Moumouls
Copy link
Member Author

Just to get back to the fact that a user should not have so much roles. I may not have fully understood the concept of roles.
In my case we could see it like this:
A user create a folder, this creates a corresponding role, then the person create a file in the current folder (child object) and the role referenced in the folder is then saved in the ACL of the file.
Then the user shares the folder with another user, this allows the new user to automatically access the files in the folder without performing a serial update of the ACLs of the files and potentially creating security holes
I use the Roles that way!
Is this a good use?

@Moumouls
Copy link
Member Author

@flovilmart we can mark it as a future improvement! but currently I can't do a patch, when I have more time, I could do a clean PR;)

@flovilmart
Copy link
Contributor

I don’t have the time to give you a clean answer

Moumouls pushed a commit to Moumouls/parse-server that referenced this issue Oct 18, 2018
Allow to manage Live Query with User that have more than 100 Parse.Roles
flovilmart pushed a commit that referenced this issue Oct 20, 2018
* Fix Limitation Role #5131

Allow to manage Live Query with User that have more than 100 Parse.Roles

* Clean Up

* Add Custom Config Support and Test

* Fix Auth Test

* Switch to Async Function

* Fix restWhere

* Fix Test

* Clean Final Commit

* Lint Fix

* Need to Fix Test Callback

* Fixes broken test

* Restore find() method in spy

* adds restquery-each

* small nit

* adds changelog
@Moumouls
Copy link
Member Author

Fixed by #5132

UnderratedDev pushed a commit to UnderratedDev/parse-server that referenced this issue Mar 21, 2020
…unity#5132)

* Fix Limitation Role parse-community#5131

Allow to manage Live Query with User that have more than 100 Parse.Roles

* Clean Up

* Add Custom Config Support and Test

* Fix Auth Test

* Switch to Async Function

* Fix restWhere

* Fix Test

* Clean Final Commit

* Lint Fix

* Need to Fix Test Callback

* Fixes broken test

* Restore find() method in spy

* adds restquery-each

* small nit

* adds changelog
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants