Skip to content
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

Master-Key-Only Role can be edited from the client JS SDK #3835

Closed
davidrichard23 opened this issue May 19, 2017 · 8 comments
Closed

Master-Key-Only Role can be edited from the client JS SDK #3835

davidrichard23 opened this issue May 19, 2017 · 8 comments
Labels
type:bug Impaired feature or lacking behavior that is likely assumed

Comments

@davidrichard23
Copy link

davidrichard23 commented May 19, 2017

Issue Description

Using role.getUsers().add(user) on a master-key-only Role will still add the user. Also produces the same result with a read-only Role. I've tried to replicate this on the User class and a custom class but the ACL's on those classes seem to be working as intended.

Steps to reproduce

var user = new Parse.User()
user.id = <user ID to add to role>
var role = <Master-Key-Only Role object>
role.getUsers().add(user)
role.save()

Expected Results

The user should not be added to the role

Actual Outcome

The user is added to the role despite being master key locked

Environment Setup

  • Server

    • parse-server version (Be specific! Don't say 'latest'.) : 2.3.8
    • Operating System: OS X 10.11.6
    • Hardware: Macbook Pro 2010
    • Localhost or remote server? (AWS, Heroku, Azure, Digital Ocean, etc): Localhost
  • Database

    • MongoDB version: 3.2.10
    • Storage engine: wiredTiger
    • Hardware: Macbook Pro 2010
    • Localhost or remote server? (AWS, mLab, ObjectRocket, Digital Ocean, etc): Localhost
@flovilmart
Copy link
Contributor

Can you provide the ACL from the dashboard on the Role ? Or the logs when running the server with VERBOSE=1?

@davidrichard23
Copy link
Author

So it's actually throwing an error in the logs but the user still gets added to the Role

verbose: REQUEST for [PUT] /parse/classes/_Role/sia4on3KCS: { "users": { "__op": "AddRelation", "objects": [ { "__type": "Pointer", "className": "_User", "objectId": "oNysdaUVwg" } ] } } method=PUT, url=/parse/classes/_Role/sia4on3KCS, host=localhost:1337, content-type=text/plain, connection=keep-alive, accept=*/*, user-agent=SafeSnap/1 CFNetwork/758.3.15 Darwin/15.6.0, content-length=269, accept-language=en-us, accept-encoding=gzip, deflate, __op=AddRelation, objects=[__type=Pointer, className=_User, objectId=oNysdaUVwg] error: Error generating response. ParseError { code: 101, message: 'Object not found.' } code=101, message=Object not found. error: Object not found. code=101, message=Object not found.

@flovilmart
Copy link
Contributor

Added on the client or also in the DB?

@davidrichard23
Copy link
Author

In the DB.
screen shot 2017-05-19 at 3 31 19 pm

@flovilmart
Copy link
Contributor

that's odd indeed.

@flovilmart
Copy link
Contributor

flovilmart commented May 19, 2017

Just written that small test:


  it('should be secure (#3835)', (done) => {
    const acl = new Parse.ACL();
    acl.getPublicReadAccess(true);
    const role = new Parse.Role('admin', acl);
    role.save().then(() => {
      const user = new Parse.User();
      return user.signUp('username', 'password');
    }).then((user) => {
      role.getUsers().add(user)
      return role.save();
    }).then(done.fail, () => {
      const query = role.getUsers().query();
      return query.find({useMasterKey: true});
    }).then((results) => {
      expect(results.length).toBe(0);
      done();
    })
    .catch(done.fail);
  });

And it's passing.

@davidrichard23
Copy link
Author

I ran your test and it passed as well. It seems the issue shows up when I set the id of the role directly like this:

var role = new Parse.Role()
role.id = '3CwA0C15G8'
role.getUsers().add(user)
role.save()

@flovilmart
Copy link
Contributor

Uhm I found something. Will get back to you soon

@flovilmart flovilmart mentioned this issue May 19, 2017
@flovilmart flovilmart added pr-submitted type:bug Impaired feature or lacking behavior that is likely assumed labels May 22, 2017
flovilmart added a commit that referenced this issue May 22, 2017
* Adds test for 3835

* Makes sure we run relational updates AFTER validating access to the object

* Always run relation udpates last
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
type:bug Impaired feature or lacking behavior that is likely assumed
Projects
None yet
Development

No branches or pull requests

2 participants