Roles that inherit from parents fail to inherit parents roles

[facultymatt]

Looks like the code in https://github.com/djvirgen/virgen-acl/blob/master/lib/permission_list.js#L23 doesn't quote work as planned. This function is only ever run one time. If the first role fails it doesn't properly increment and check the other roles.

[facultymatt]

Actually, this seems to only be the case when first setting acl.deny(); as in the example to deny all. If this is removed the inheritance from parents seems to work...

[djvirgen]

Hi facultymatt,

I'm not sure if I understand the problem. I just added two unit tests to verify that child resources are denied access when they and their parents are not explicitly granted access (see 472b9a).

I may be missing something. Do I need to use a custom assertion with some async to reproduce the problem?

[facultymatt]

Thanks for the quick response and tests! I'll need to look into this further, but I was getting the error when I first set acl.deny(); which should deny all by default. Then I set my allow statements.

I'll try to create some tests later today that demonstrate this.

[djvirgen]

Hi facultymatt,

I've looked into this some more and it turns out that assertions can fail if more than one permission rule matches the query (e.g. when using inheritance) and one of those permissions uses a custom assertion that calls next() synchronously.

When that happens, the position value in the permission list never increments and causes an infinite loop. :(

I've fixed this by wrapping the call to each individual permission's query() in a setTimeout() to force async. See 18e4f4

Thanks for raising this issue. I'll release an updated version with this fix in the next few minutes.

[djvirgen]

Version 0.0.20 has been published to npm. Please let me know if this fixes the issue for you. Thanks again!

[nervgh]

@djvirgen , I'm not sure but this may be a solution:

PermissionList.prototype.next = function() {
  if (this.count == this.position) {
    return this.done(null, false); // No more permissions to check, DENIED!!1
  }

  var permission = this.permissions[this.position];

  // Ensure custom assertions are run async, fixes issue #7
  permission.query(
    this.role,
    this.resource,
    this.action,
    this.done,
    function() {
      this.position++; // <-- (!)
      this.next();
    }.bind(this) // ensure proper scope?
  );

  //this.position++;
};