DeadAlready/easy-rbac

Question: multiple roles

Closed this issue · 7 comments

Thank you for the great article at https://blog.nodeswat.com/implement-access-control-in-node-js-8567e7b484d1
I have looked through the repo and tried to use this package - it looks great, however I am missing an ability to provide multiple roles for can method.

I have implemented a naive solution for this use case at https://github.com/agoldis/easy-rbac/blob/multi_roles/lib/rbac.js#L180

Would that be a suitable case for PR?

Would you to share your opinion on getting multiple roles for a user properly for RBAC.

Thank you!

Hi

Your question triggered my long time plan to rewrite this module.
I released v3 - it no longer has Q dependency and also supports role input as array. It does however use async/await so Node >=v8.x is required.

If you need it in v2 branch however then your pull request is lacking in some parts:

  1. I would not create a separate function for it - it would be easier to integrate into the old one by checking type and using Q.any -> in the lines of
if(Array.isArray(role) && role.length === 1) {
    role = role[0];
  }

  if(Array.isArray(role)) {
    promise = Q.any(role.map(function (r) {
      return $this.can(r, operation, params);
    }));
  }
  else {
    promise = Q.Promise(function(resolve, reject) {
  1. Tests - the functionality should also be included in tests.
  2. v2 is meant to run on old versions of node, so arrow functions should not be used

oh, that's indeed what I need - multi roles, and I am running node 8+. Do you plan to release it soon?

I will implement your recommended changes for v2 as well (tests already done btw)

I released v3 yesterday.

You closed your pull request. Why?
I would have merged it into v2 and released array handling for older versions as well.

@DeadAlready cool, we are already using the new v3! thank you very much!

I have canceled because of the conflicts - I can only open PR to master which is already v3.
If you had a branch or tag for v2 I would have opened the PR to the appropriate code

Ah... Fair point... Forgot to branch those things...

I have now created a separate branch (v2) - for handling v2 fixes and updates.

I refined some tests (you still had arrow functions there and only sync tests), added a readme and released v2.2.0 for older Node versions.

Thank you for your contribution.

@DeadAlready oh, sorry for not completing the PR properly! And thank you for collaboration!