Loopback: Role.id and Role.name mixup in resolving ACLs and Role.isInRole

Created on 9 Jan 2015 · 9Comments · Source: strongloop/loopback

The documentation for the ACL class states that the principalId for a Role should be "ID of the principal - such as appId, userId or roleId". When a role is resolved in ACL.checkAccessForContext we see that it is called with roleModel.isInRole(acl.principalId, ...
However, when looking at the documentation for Role.isInRole:

  /**
   * Check if a given principal is in the role
   *
   * @param {String} role The role name
   * @param {Object} context The context object
   * @callback {Function} callback
   * @param {Error} err
   * @param {Boolean} isInRole
   */
  Role.isInRole = function(role, context, callback)

And later on it is used as a name, not as an id: this.findOne({where: {name: role}} ...

I had hooked up all my ACLs to map on IDs, so I changed the code to use findById, and I could not find any other place that referred to it so that worked fine for me. Another option would be to change the calling code to use name, or have the find look for both name OR id.

As I don't know how you would like to have it, I figured I mention it to you guys without trying to provide a patch or something first.

Thanks for the great work and keep it up!

bug

Source

marcuspl

All 9 comments

@raymondfeng you are more familiar with ACLs than I am. Could you PTAL?

bajtos on 2 Jun 2016

@marcuspl Thank you for spotting this issue. We confirm with @raymondfeng that it should be role name. This should be a bug in code and not doc issue .

So we should pass in the principalName instead of the principalId
Do you want to provide a patch?
Thanks

```

    // Check role matches
    if (acl.principalType === ACL.ROLE) {
      inRoleTasks.push(function(done) {
        roleModel.isInRole(acl.principalId, context,
          function(err, inRole) {
            if (!err && inRole) {
              effectiveACLs.push(acl);
            }
            done(err, acl);
          });
      });

```