chaps-io / access-granted

Multi-role and whitelist based authorization gem for Rails (and not only Rails!)
MIT License
775 stars 41 forks source link

Higher level roles do not seem to inherit from lower level roles #41

Closed parc64 closed 7 years ago

parc64 commented 7 years ago

Looking at this line from documentation at https://blog.chaps.io/2015/11/13/role-based-authorization-in-rails.html Roles inherit from less important roles below them, but only from roles which apply to the given user. So we only need to add additional permissions on top of them. In this case, we will let members create posts and topics:

I have not been able to get this backward inheritance to work. I have something similar to this in my config.

class AccessPolicy
  include AccessGranted::Policy

  def configure
    role :admin, { role: 'admin' } do
       # can :view, Admin::DashboardController
     end

    role :staff, { role: 'staff' } do
      can :view, Admin::DashboardController
    end

    role :view_only, { role: 'view_only' } do

    end
  end
end

However calling can? :view, Admin::DashboardController only returns true for the :staff role, and returns false for the higher level :admin role. Unless I uncomment the line in the :admin block.

pokonski commented 7 years ago

Hey, @parc64.

AG will only inherit from roles which also apply to the user.

So in your example if your user has a role set to admin, it won't match with role staff because it expects staff to be inside the role attribute.

You should update the less important roles to also apply to admins, like this:

role :staff, proc { |user| user.role == "staff" || user.role == "admin" } do

this way admin will still have permissions granted by staff and will override them if there are any more important permissions in role :admin

parc64 commented 7 years ago

Hello @pokonski ,

Ok that makes sense. It would be nice to update the docs to show this example. The way the docs are now, it's difficult to understand if the gem is doing this behind the scenes or if the user needs to knit these together as your example shows. The docs also talk about the order of the roles in the access_policy.rb file, does the gem actually use the order for anything, or is that just a suggested good practice?

Thanks!

pokonski commented 7 years ago

You are absolutely right, it isn't backed by an example, but only a short sentence. I will expand that :)

And yes order is important because the first role which has a permission you are checking will stop further checks. So if a top role has 'cannot' defined for a permission, then Access Granted will not check the remaining roles.

pokonski commented 7 years ago

The order is explained in the Readme here https://github.com/chaps-io/access-granted/blob/master/README.md#roles-in-order-of-importance

parc64 commented 7 years ago

awesome, thank you

rozhok commented 7 years ago

Correct me if I'm wrong, but cant I set-up per-object permissions rather than roles and grant users access using something like bitmask?

pokonski commented 7 years ago

@rozhok you don't have to use roles. You can have one role for everything and do whatever check you like inside the can block.

rozhok commented 7 years ago

Oh really, missed last example, got it.

Okay, but it's not the best way to do that, isn't it?

pokonski commented 7 years ago

AG is made specifically to utilize roles, if you don't have roles then yes - it might not be the best way.

rozhok commented 7 years ago

I want roles, but I don't want inheritance. Is there way to avoid it?

pokonski commented 7 years ago

Yes, roles only inherit each other if they apply to the user.

For example this policy:


role :admin_role, proc { |user| user.role == :admin } do
  (... some permissions here..)
end

role :regular_role, proc { |user| user.role == :regular } do
  (...)
end

for an admin user like this

current_user.role #=> :admin

will not inherit permissions from regular_role because that role is not matching the user at all (it would only match if the conditions were satisfied.

but If you'd like admins to inherit permissions from regular roles you could write a condition proc that matches admins, like this:


role :admin_role, proc { |user| user.role == :admin } do
  (... some permissions here..)
end

role :regular_role, proc { |user| user.role == :regular || user.role == :admin } do
  (...)
end

You have full control over how roles match, so you can construct any inheritance you want.... or no inheritance at all.

I hope that example explains it this time.

rozhok commented 7 years ago

Oh, finally got it.

Thanks!

pokonski commented 7 years ago

Awesome, let me know if AG turned out to be useful :smile:

rozhok commented 7 years ago

Everything works as excepted! Awesome. Using it in prod now.

pokonski commented 7 years ago

@rozhok fantastic! I do love hearing success stories :D