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

Role Hierarchy not working #683

Closed
davidruisinger opened this issue Feb 26, 2016 · 11 comments · Fixed by #689
Closed

Role Hierarchy not working #683

davidruisinger opened this issue Feb 26, 2016 · 11 comments · Fixed by #689
Assignees

Comments

@davidruisinger
Copy link

I just did some dirty tests where I have a Cloud Function that initially does following:

  1. Creates a superuser
  2. Creates superuser role that can only be modified by the superuser
  3. Adds the superuser to the superuser role
  4. Creates an admin role that can only be modified by the superuser role
  5. Adds the superuser role to the admin role (superusers have the same rights as admins)
  6. Creates a customer role that can only be modified by the superuser role
  7. Adds the admin role to the customer role (admins have the same rights as customers)

(For testing purposes I have all this wrapped into this ugly spaghetti code

I also have a Class of Product and a beforeSave Hook that adds
setPublicReadAccess(true);
and
productACL.setRoleWriteAccess("admin", true);
to each product that is saved.

In my DB it seems that everything is setup properly:
Product:

{
  "_id": "6w74MRh8Kr",
  "_rperm": [
    "*"
  ],
  "_wperm": [
    "role:admin"
  ],
  ...
}

_User

{
  "_id": "GtYEf8IUE2",
  ...
}

Role

{
  "_id": "ipvDGI3CaF",
  ...
  "name": "admin",
  ...
}

and

{
  "_id": "9XUOMFg9eC",
  ...
  "name": "superuser",
  ...
}

_Join:roles:_Role

{
  "relatedId": "9XUOMFg9eC",
  "owningId": "ipvDGI3CaF"
}

_Join:users:_Role

{
  "relatedId": "GtYEf8IUE2",
  "owningId": "9XUOMFg9eC"
}

When I now login as this superuser, I can't update a Product even though the superuser role is a child of admin and should have ALL the admin rights which include write permission to Product.
(According to this doc https://parse.com/docs/js/guide#roles-role-hierarchy)

@gfosco
Copy link
Contributor

gfosco commented Feb 26, 2016

Known issue, the current role logic only supports nesting roles 1-level deep. PRs welcome

https://github.com/ParsePlatform/parse-server/blob/master/src/Auth.js#L141

@flovilmart
Copy link
Contributor

@gfosco implemented it in there: https://github.com/flovilmart/__AppStack__/blob/master/controllers/core/RoleController.js do you think that would do recursively?

@gfosco
Copy link
Contributor

gfosco commented Feb 29, 2016

Merged #689, this should be fixed in 2.1.4.

@davidruisinger
Copy link
Author

I just updated to the latest release but my superuser is still not able to modify a product.
Strange thing is that since updating to 2.1.4 my customers can now modify products as well (see #827) even though their role does NOT allow that...

@davidruisinger
Copy link
Author

@gfosco I just did a test and I think the _Join:roles:_Role objects are stored wrong (switched relation).
As you can see in my (still ugly) spaghetti code I'm adding the superuser role to the admin role:

...
adminRole.getRoles().add(superUserRole);
...

Which, according to how I understand the docs means that superuser has same rights as admin.

The _Join:roles:_Role object looks like this:

{
  "relatedId": "ZsRF5ckXsZ", // admin Role
  "owningId": "Bul4LG2TXK" // superuser Role
}

If I now just manually switch the IDs in my DB to

{
  "relatedId": "Bul4LG2TXK",
  "owningId": "ZsRF5ckXsZ"
}

everything is working as expected.

This would also explains #827 because for customer roles I'm doing the same:

customerRole.getRoles().add(adminRole);

@gfosco
Copy link
Contributor

gfosco commented Mar 7, 2016

I'm not able to reproduce these permissions issues with the current code. Created a test in #878 which covers a mix of what is reported here and in #827.

I'd suggest trying to build a test case, and if you're able to find a problem, please report it in a new issue.

@davidruisinger
Copy link
Author

@gfosco THX for the Tests. But for some reasons the issue still appears and is NOT covered by the tests.

To better illustrate the issue, I adjusted the Spec to exactly cover my Case:

  it('should properly handle role permissions on objects', (done) => {
    var superUser, adminUser, customerUser;
    var superRole, adminRole, customerRole;
    var superRoleACL, adminRoleACL, customerRoleACL;
    var obj;
    var objACL;

    createTestUser().then((x) => {
      superUser = x;
      adminUser = new Parse.User();
      return adminUser.save({ username: 'adminUser', password: 'omgbbq' });
    }).then((x) => {
      customerUser = new Parse.User();
      return customerUser.save({ username: 'customerUser', password: 'omgbbq' });
    }).then((x) => {

      // Create a Super Role
      superRoleACL = new Parse.ACL();
      superRoleACL.setReadAccess(superUser, true);
      superRoleACL.setWriteAccess(superUser, true);
      superRole = new Parse.Role('Super', superRoleACL);
      // Add the superUser to the Super Role
      superRole.getUsers().add(superUser);
      return superRole.save({}, { useMasterKey: true });
    }).then(() => {

      // Create an Admin Role
      adminRoleACL = new Parse.ACL();
      adminRoleACL.setRoleReadAccess(superRole, true);
      adminRoleACL.setRoleWriteAccess(superRole, true);
      adminRole = new Parse.Role('Admin', adminRoleACL);
      // Add the superuser role to the admin role ==> superuser has same rights as admin
      adminRole.getRoles().add(superRole);
      // Add the adminUser to the Admin Role
      adminRole.getUsers().add(adminUser);
      return adminRole.save({}, { useMasterKey: true });
    }).then(() => {

      // Create a Customer Role
      customerRoleACL = new Parse.ACL();
      customerRoleACL.setRoleReadAccess(superRole, true);
      customerRoleACL.setRoleWriteAccess(superRole, true);
      customerRole = new Parse.Role("Customer", customerRoleACL);
      // Add the admin role to the customer role ==> admin has same rights as customerRole
      customerRole.getRoles().add(adminRole);
      return customerRole.save({}, { useMasterKey: true });
    }).then(() => {
      var query = new Parse.Query('_Role');
      return query.find({ useMasterKey: true });
    }).then((x) => {
      expect(x.length).toEqual(3);

      // Create an Object that is editable by Admin (and Super) only
      // Create a new ACL with write permission for Admin (and Super) role and read permission to public
      objACL = new Parse.ACL();
      objACL.setRoleWriteAccess('Admin', true);
      objACL.setPublicReadAccess(true);
      obj = new Parse.Object('TestObjectRoles');
      obj.setACL(objACL);
      return obj.save(null, { useMasterKey: true });
    }).then(() => {

      // Above, the Super role was added to the Admin role.
      // An object secured by the Admin Role should be able to be edited by the Super user.
      obj.set('changedBySuper', true);
      return obj.save(null, { sessionToken: superUser.getSessionToken() });
    }).then((obj) => {
      // An object secured by the Admin ACL should of course allow an Admin Role user to edit it.
      obj.set('changedByAdmin', true);
      return obj.save(null, { sessionToken: adminUser.getSessionToken() });
    }, (e) => {
      fail('Super user should have been able to save.');
      done();
    }).then((obj) => {
      // An object secured by the Admin ACL should not be able to be edited by a Customer role user.
      obj.set('changedByCustomer', true);
      return obj.save(null, { sessionToken: customerUser.getSessionToken() });
    }, (e) => {
      fail('Admin user should have been able to save.');
      done();
    }).then((obj) => {
      fail('Customer user should not have been able to save.');
      done();
    }, (e) => {
      expect(e.code).toEqual(101);
      done();
    })
  });

Running this test works just fine - NO ERRORS.

The I take almost the exact same code and put it into cloud function:

Parse.Cloud.define('rolesTest', function(req, res) {

  var superUser, adminUser, customerUser;
  var superRole, adminRole, customerRole;
  var superRoleACL, adminRoleACL, customerRoleACL;
  var obj;
  var objACL;

  superUser = new Parse.User();
  superUser.save({ username: 'superUser', password: 'omgbbq' }).then((x) => {
    adminUser = new Parse.User();
    return adminUser.save({ username: 'adminUser', password: 'omgbbq' });
  }).then((x) => {
    customerUser = new Parse.User();
    return customerUser.save({ username: 'customerUser', password: 'omgbbq' });
  }).then((x) => {

    // Create a Super Role
    superRoleACL = new Parse.ACL();
    superRoleACL.setReadAccess(superUser, true);
    superRoleACL.setWriteAccess(superUser, true);
    superRole = new Parse.Role('Super', superRoleACL);
    // Add the superUser to the Super Role
    superRole.getUsers().add(superUser);
    return superRole.save({}, { useMasterKey: true });
  }).then(() => {

    // Create an Admin Role
    adminRoleACL = new Parse.ACL();
    adminRoleACL.setRoleReadAccess(superRole, true);
    adminRoleACL.setRoleWriteAccess(superRole, true);
    adminRole = new Parse.Role('Admin', adminRoleACL);
    // Add the superuser role to the admin role ==> superuser has same rights as admin
    adminRole.getRoles().add(superRole);
    // Add the adminUser to the Admin Role
    adminRole.getUsers().add(adminUser);
    return adminRole.save({}, { useMasterKey: true });
  }).then(() => {

    // Create a Customer Role
    customerRoleACL = new Parse.ACL();
    customerRoleACL.setRoleReadAccess(superRole, true);
    customerRoleACL.setRoleWriteAccess(superRole, true);
    customerRole = new Parse.Role("Customer", customerRoleACL);
    // Add the admin role to the customer role ==> admin has same rights as customerRole
    customerRole.getRoles().add(adminRole);
    return customerRole.save({}, { useMasterKey: true });
  }).then((x) => {

    // Create an Object that is editable by Admin (and Super) only
    // Create a new ACL with write permission for Admin (and Super) role and read permission to public
    objACL = new Parse.ACL();
    objACL.setRoleWriteAccess('Admin', true);
    objACL.setPublicReadAccess(true);
    obj = new Parse.Object('Product');
    obj.set('name', 'Test Product #1');
    obj.setACL(objACL);
    return obj.save(null, { useMasterKey: true });
  }).then((x) => {

    // Above, the Super role was added to the Admin role.
    // An object secured by the Admin Role should be able to be edited by the Super user.
    obj.set('changedBySuper', true);
    return obj.save(null, { sessionToken: superUser.getSessionToken() });
  }).then((obj) => {
    // An object secured by the Admin ACL should of course allow an Admin Role user to edit it.
    obj.set('changedByAdmin', true);
    return obj.save(null, { sessionToken: adminUser.getSessionToken() });
  }, (e) => {
    res.error('Super user should have been able to save.');
  }).then((obj) => {
    // An object secured by the Admin ACL should not be able to be edited by a Customer role user.
    obj.set('changedByCustomer', true);
    return obj.save(null, { sessionToken: customerUser.getSessionToken() });
  }, (e) => {
    res.error('Admin user should have been able to save.');
  }).then((obj) => {
    res.error('Customer user should not have been able to save.');
  }, (e) => {
    res.success('All tests succeeded!!!');
  });
});

And for some reasons, this Cloud function fails with the attempt of the Super User to update the Product...

As mentioned in my comment above, I could manually fixed the issue by just switching the IDs of the 2 role relations in the _Join:roles:_Role collection.

@flovilmart
Copy link
Contributor

@flavordaaave you should not switch manually the ids :) what version are you running? master, 2.4? because there was a problem in the recursive roles resolution where the query was looking for $relatedTo instead of "users" which is what you describe as switching the ID's.

The fix was introduce with #841

@davidruisinger
Copy link
Author

@flovilmart I know that manually switching the IDs is not a solution, I just recognized that those IDs seem to be the reason ;)

According to serverInfo I'm running "parseServerVersion": "2.1.4" I'll try out the current master version. THX for the info.

@davidruisinger
Copy link
Author

@flovilmart THX again. I just tested with the latest master branch and everything is working now!!! 👍 👍 👍

@flovilmart
Copy link
Contributor

Thanks for your debugging! That put me on the right direction!

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

Successfully merging a pull request may close this issue.

3 participants