Allow users to have multiple roles

[spruce-bruce]

The one use case I have that doesn't seem to be solved by this library is I would like to be able to assign users to multiple roles.

The change I'm proposing would leave the current api intact, but updates would be made to allow the getRoleId() function to return an array of roles.

I'd like to submit a PR for this, let me know if you're open to this change. The new api would look like this:

// User class
var User = (function(){
  User = function(attribs) {
    this.id = attribs.id || null;
  }

  User.prototype.getRoleId = function() {
    return ['blog-admin', 'product-admin'];
  }

  return User;
})();

[djvirgen]

That sounds like a great idea, and it'd be something that role inheritance can't always elegantly solve. Another possible solution would be to allow a role to inherit from multiple parents instead of just one. Either way would be great. Thanks!

[spruce-bruce]

Cool! It'll probably take me a couple of weeks to get to it, but I'll have a PR for you soon.

I like the multiple inheritance idea, but my specific use case is better solved by multiple roles. When I'm working on this and learn more about the specifics of your implementation I'll see how hard it'll be to implement multiple inheritance as well.

[spruce-bruce]

Just an FYI - I'm starting to tackle this today and should have a PR for you soon

[djvirgen]

Awesome, thank you!

[spruce-bruce]

I'm curious if you have some thoughts on what I'm working on right now.

I've got the whole multi-role thing working, but I'm unsure how to resolve a particular case. What do you want to happen if I .deny() guest access to blog and then issue a query like this:

acl.query(['member', 'guest'], 'blog', 'view');

Right now my code is funny because the above is allowed, but if I change it to this:

acl.query(['guest', 'member'], 'blog', 'view')

it will be denied.

Anyway, on to the real question, I'm assuming that since you've explicitly called acl.deny('guest') that anything with 'guest' in it should fail, even if it's paired with something that otherwise would pass. Are you ok with that? Or would you rather allow ['guest', 'member'] when member is allowed and guest is denied?

[spruce-bruce]

I think my assumption is wrong - the preference should be to honor the LIFO rules.

so if you do:

this.acl = new Acl();
this.acl.allow();
this.acl.deny('guest');

then anything with guest should be denied, but if you do

this.acl = new Acl();
this.acl.deny('guest');
this.acl.allow();

then anything will be allowed, even guest.

[djvirgen]

Yea I think it makes sense to follow LIFO order, so at least we can say the behavior is defined (and we can verify it via unit tests).

[spruce-bruce]

Some things of note in the PR:

1. I updated should and mocha
2. I use Array.prototype.filter
3. I use template strings

Let me know if you want me not do those things to maintain support of older versions of node. I don't know when Array.prototype.filter was added to node, for example, might only be newer versions.

[spruce-bruce]

I actually broke something with that PR - i'm going to write a failing test and fix it. let me know if you want me to close the PR and submit a new one or if the current PR is ok

[spruce-bruce]

Alright that should fix that! Let me know if any of this doesn't look good to you.

[bobeagan]

@djvirgen any update on this?

[djvirgen]

It looks like the branch needs to be rebased due to conflicts.

[spruce-bruce]

Not seeing conflicts.

I'll address your comments, though, and double check that I have the most recent changes. Thanks!

[spruce-bruce]

Updated those tests - if I remember correctly passing in done and calling it is all you need to do to make a test async - if that's wrong let me know.

And let me know if you still see merge conflicts or anything? I went ahead and re-pulled from your master, didn't have any problems.

[djvirgen]

Thanks! I've merged your changes and published version 0.0.21 to npm.

[spruce-bruce]

Nice! Thanks man - again I like this library a lot. We've started using it in a new project of ours and its working out really nicely

[djvirgen]

Awesome! That's great to hear :)