`matchesQuery` and pointer permissions

[christianpbrink]

Issue Description

I've got a class, PlaylistItem, that has a pointer to a User. I'm using pointer permissions to give that User full read and write permissions on the PlaylistItem that points to it. The only public CLP on PlaylistItem is 'create'.

My use of pointer permissions is new. Until today I had full public 'read' CLPs on PlaylistItem.

This is an iOS app. One of the PFQuerys I run against PlaylistItem has a matchesQuery condition, where the inner query is on a different class called ArticleVersion. ArticleVersion has public 'read' CLPs.

The matchesQuery condition always worked fine until now. It stops working when I whack the public 'read' CLP on PlaylistItem and rely on pointer permissions only. By "stops working" I mean it causes my outer query to return no results.

Steps to reproduce

I can write a clear repro if need be, but first I'd like some confirmation as to whether this is a known issue (or perhaps is even expected behavior for reasons I'm not thinking of). I searched for a good while and found nothing about it.

• Server
  • parse-server version: 2.2.17
  • Operating System: Um whatever's the default on Heroku?
  • Hardware: Heroku dyno, the default
• Database
  • MongoDB version: 3.0.12
  • Storage engine: mLab default
  • Hardware: mLab default

Logs/Trace

verbose: REQUEST for [GET] /parse/classes/PlaylistItem: {
  "include": "articleVersion",
  "order": "index",
  "where": {
    "user": {
      "__type": "Pointer",
      "className": "_User",
      "objectId": "-----"
    },
    "articleVersion": {
      "$inQuery": {
        "include": "publication",
        "order": "index",
        "className": "ArticleVersion",
        "where": {
          "index": {
            "$exists": true
          },
          "active": true
        }
      }
    }
  }
} method=GET, url=/parse/classes/PlaylistItem, host=----, x-parse-client-version=i1.13.0, accept=*/*, x-parse-session-token=-----, x-parse-application-id=-----, x-parse-client-key=------, x-parse-installation-id=------, x-parse-os-version=9.3 (15F34), accept-language=en-us, accept-encoding=gzip, deflate, content-type=application/json; charset=utf-8, content-length=292, user-agent=Audm/6326 CFNetwork/758.3.15 Darwin/15.5.0, connection=keep-alive, x-parse-app-build-version=6326, x-parse-app-display-version=1.0, include=articleVersion, order=index, __type=Pointer, className=_User, objectId=-----, include=publication, order=index, className=ArticleVersion, $exists=true, active=true
verbose: RESPONSE from [GET] /parse/classes/PlaylistItem: {
  "response": {
    "results": []
  }
} results=[]

Thank you!

[christianpbrink]

I figured this out, at least partially. RestQuery.prototype.getUserAndRole includes this line:

roles.push(_this3.auth.user.id);

Since roles is a var that was passed into this function -- passed by reference -- what this is doing is adding the user's ID to the cached list of roles. That won't have any effect in a query that has no nested queries. But if the query does have a nested query, you end up with the user's ID appearing twice in the user ACL that's sent to the db adapter. (And if you've got two nested queries then her ID gets included three times, and so on.)

This is all only a problem if you're using pointer permissions on the class to which the outer query is addressed. In DatabaseController.prototype.addPointerPermissions, if there's a public read CLP, the pointer 'read' permissions are never checked. But if there isn't a public 'read' CLP, there's a check to ensure that the ACL that was passed in has only one non-role member. If it has more than one non-role member, the whole query is rejected (silently, it seems!) as invalid.

My solution is simply to change RestQuery.prototype.getUserAndRole so that the array onto which the User objectId is pushed is a copy of the cached roles list, not a reference to the original.

[christianpbrink]

With all the babel stuff, I don't even know where to start in preparing a PR. But here's my fixed source: https://github.com/audm/parse-server/blob/master/lib/RestQuery.js#L212

[flovilmart]

Nice pin pointing of the issue, check the getUserAndRoleACL method in src/RestQuery.js

You should be able to do the same trick.

Also, can you add a test that covers that case alongside with the fix?

If you're unable to patch and open the PR, then just open a PR with the failing test, I'll take care of the fix

[christianpbrink]

I actually don't have the slightest idea how to write a test for this.

On Mon, Aug 1, 2016 at 7:32 PM, Florent Vilmart notifications@github.com
wrote:

Nice pin pointing of the issue, check the getUserAndRoleACL method in
src/RestQuery.js

You should be able to do the same trick.

Also, can you add a test that covers that case alongside with the fix?

If you're unable to patch and open the PR, then just open a PR with the
failing test, I'll take care of the fix

—
You are receiving this because you authored the thread.
Reply to this email directly, view it on GitHub
#2431 (comment),
or mute the thread
https://github.com/notifications/unsubscribe-auth/AAbap-caivx5Ot-ljU0aiSh60b1ogY1Uks5qboIpgaJpZM4JZUsh
.

[FlorianSchuler]

Yesterday I've also stumbled over this annoying bug and searched hours for mistakes in my queries, which could result in empty query results with no success until i found this issue here.
I've also no slightest idea how to write a test for this but it's a really annoying bug which should be fixed as fast as possible.
@flovilmart can you please take care of add a test to the fix of @christianpbrink and open a PR.
That would be awesome!

[flovilmart]

@christianpbrink 's patch is on the generated sources, nothing much I can do here.

This is where the patch should be done:
https://github.com/ParsePlatform/parse-server/blob/master/src/RestQuery.js#L152

@FlorianSchuler why don't you take care of the fix and the test?

[FlorianSchuler]

Sorry @flovilmart I think I'm not a help here, I'm an absolute newbie to this whole project.
I stumbled about this bug while using a managed parse-server.

[flovilmart]

the best way to not be a newbie is to jump into the project. You have a case that reproduces the problem, and I pointed you to the right file/position to put the fix...

[flovilmart]

@christianpbrink the fix is gonna land in the next release, the issue will close when the PR gets merged! Thanks for your patience.

[flovilmart]

(released with 2.2.19)