_User ACL not working correctly

[awgeorge]

Issue Description

Updates to _User record do not adhere to the ACL correctly. If a moderator is editing another _User record (i.e not their own data). With the correct ACL's in place, the server always denies permission due to this line

if (this.className === '_User' &&
      this.query &&
      !this.auth.couldUpdateUserId(this.query.objectId)) {
    throw new Parse.Error(Parse.Error.SESSION_MISSING, `Cannot modify user ${this.query.objectId}.`);
  }

As you can see the function will only return true if we're using the master key or the object is the currently logged in user.

// Whether this auth could possibly modify the given user id.
// It still could be forbidden via ACLs even if this returns true.
Auth.prototype.couldUpdateUserId = function(userId) {
  if (this.isMaster) {
    return true;
  }
  if (this.user && this.user.id === userId) {
    return true;
  }
  return false;
};

The comment implies that the ACL can still override, but this is not the case.

Steps to reproduce

1. Correctly assign ACLs to the user record so that the currently authenticated user should be able to edit the record.

2. Edit the _User record

Expected Results

A successful outcome.

Actual Outcome

{"code":206,"message":"Cannot modify user IyYUu90HdL.","level":"error","timestamp":"2017-03-01T17:41:56.373Z"}

Environment Setup

• Server

  • parse-server version (Be specific! Don't say 'latest'.) : 2.3.6
  • Operating System: OSX 10.12.1
  • Hardware: MBP-2015
  • Localhost or remote server? (AWS, Heroku, Azure, Digital Ocean, etc): localhost

• Database

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

Logs/Trace

Include all relevant logs. You can turn on additional logging by configuring VERBOSE=1 in your environment.

[flovilmart]

We don't allow users to modify other user's ACL for security reasons.

[awgeorge]

Then what's the point of ACL's? Without editing users, you're limiting Parse's functionality as it can't be used for anything user management related, you can't design a system that has moderators or administrators without splitting all user content into another table. Which for already established applications is upsetting. We've unfortunately had to use the master key which arguable is now worse for security reasons as anyone with access to the page can now update the user - instead of the ACLs which was the purpose of them to begin with.

[flovilmart]

I believe this restriction was set in parse-server as it was there on parse.com

[awgeorge]

Are you sure? - I continued to use the parse hosted solution up until January (as I couldn't find the time to transfer it sooner). And my Administrator ACL was able to update users.

[flovilmart]

I believe

it doesn't mean I'm sure.

[johanarnor]

@awsGeorge you could use a cloud function Parse.Cloud.define('someFunction',... to achieve this. In that function you can first check for role and then use the master key.

Also are you now using the master key in a publicly facing webpage? That would be a huge security issue.

[awgeorge]

@johanarnor Yes - it was a quick work around, naturally the page in question requires elevated privileges, but I would prefer not to use the master key, as it seems like a hack to reinstate it's original functionality.

[johanarnor]

Ok, but how is a user authenticated to reach that page? Is the page a part of a single page application that anyone can access from public internet? If so, your master key is publicly available.

[awgeorge]

No - It's using the PHP SDK.

[dkarviga]

The same issue. I have to perform access control checkings myself using Parse Cloud functions.
It'd be great if ACL works for _User objects as for other objects

[coryshaw]

+1

I would expect the User class to adhere to the same ACLs as everything else. I can understand setting the default ACLs for the User class differently for security reasons, but if I explicitly set the ACL of a user to a role ("Admin" read/write), I would expect that a logged in Admin can read and write that User.

The work arounds are either to POST to an endpoint and useMasterKey, or create a separate "User Meta" class and have to deal with the complexities of the extra join query.

[sdonaway]

+1 The User class should adhere to the same ACLs as any other class. Otherwise, user management at an elevated role is impossible without using a hack/workaround.