Meteor-Community-Packages / meteor-roles

Authorization package for Meteor, compatible with built-in accounts packages
http://meteor-community-packages.github.io/meteor-roles/
MIT License
921 stars 166 forks source link

Allow filtering for scope only #298

Closed jamgold closed 4 years ago

jamgold commented 4 years ago

The Roles.getUsersInRole method always returns Roles.GLOBAL_GROUP as well as roles for a particular scope/group, which can be confusing.

A simple change in the method _getUserInRoleCursor could fix this, by accepting the option onlyScope: true. Now the method can be called with

Roles.getUsersInRole('some-role',{scope:'some-scope', onlyScope: true})

Which will only return the users having some-role in scope some-scope now.

  var selector

  options = Roles._normalizeOptions(options)

  options = Object.assign({
    anyScope: false
  }, options)

  // ensure array to simplify code
  if (!Array.isArray(roles)) roles = [roles]

  Roles._checkScopeName(options.scope)

  filter = Object.assign({
    fields: { 'user._id': 1 }
  }, filter)

  selector = {
    'inheritedRoles._id': { $in: roles }
  }

  if (!options.anyScope) {
    selector.scope = { $in: [options.scope, null] }
  }

  if (options.onlyScope) {
    selector.scope = options.scope;
  }

  return Meteor.roleAssignment.find(selector, filter)
};
SimonSimCity commented 4 years ago

Already solved in v3, please upgrade.

jamgold commented 4 years ago

This is from v3

SimonSimCity commented 4 years ago

Ah sorry - I misread the function name ... It's for implementing the option onlyScoped which I added on getRolesForUser() now also on getUsersInRole(). Would you like to create a PR and include some tests for it? I'd be thrilled to merge it!

jamgold commented 4 years ago

No problem. I would like to help out, maybe I can create a PR this weekend.

I also noticed that the new role hierarchy is powerful, but confusing. There doesn't seem to be an easy way to find out why a user has a particular role, is it global or inherited ...

SimonSimCity commented 4 years ago

Well, every role the user has assigned will create a new document in the role-assignment collection. It contains the assigned role (role._id) and the roles inherited at any level by this assignment (inheritedRoles._id). The role itself is also part of the list of inherited roles, which is why I can simply query on inheritedRoles._id here. In case you want to only get a list of directly assigned roles, you could query for role._id - but I guess this would belong to a different issue.

jamgold commented 4 years ago

Is there anything you need to do for me to create a pull-request? This is my first and I can't seem to figure it out

SimonSimCity commented 4 years ago

Not that I would know of ... you have to fork this project, push your code into the fork and create a pull-request ... If you're stuck there, this might help: https://blog.scottlowe.org/2015/01/27/using-fork-branch-git-workflow/

jamgold commented 4 years ago

Created https://github.com/Meteor-Community-Packages/meteor-roles/pull/299

SimonSimCity commented 4 years ago

Will be released as 3.2.0 any minute.