class inheritance, and class as test subject

[jrochkind]

I found what I think is an inconvenient inconsistency around inheritance; I also have a simple PR to fix it.

access_granted normally works great with inheritance. Imagine we have a Vehicle and a Bicycle < Vehicle. And we have a policy:

    class AccessPolicy
      include AccessGranted::Policy

      role :user do
        can :read, Vehicle
      end
    end

AccessPolicy.new(user).can? :read, Vehicle.new # true, of course
AccessPolicy.new(user).can? :read, Bicycle.new # still true, it works with the sub-class

But access_granted also has a convenient feature where you can check on the class instead of an instance, intended to be used with the meaning of sort of generic/any/all objects, like often for create, but you can use it for anything:

AccessPolicy.new(user).can? :read, Vehicle # this is true too, okay

That one does NOT work with inheritance:

AccessPolicy.new(user).can? :read, Bicycle # FALSE
# I believe it should be true

I think this is a bug, or would be an improvement if above were true also. I think I have a simple PR to make it so.

Thoughts?

[pokonski]

Hello, Jonathan!

Thanks for the suggestion, from the top of my head I cannot think of any downsides to this BUT I'd treat it as a breaking change warranting a major version bump to to the gem.

Did you encounter this in a real life project? I'd love to hear about that use case.

In any case please do send a PR 😁

[jrochkind]

Thanks for the response!

Yes, I encountered this in a real project.

We use model inheritance (Rails single-table inheritance, but it doesn't really matter). So we have a class hieararchy.

We want to express policies on the superclass if it's the same for all of them, say (and this is still a simplified extraction of course):

role :admin, proc { |user| user.admin? } do
   can :manage, Vehicle # all of them
end

role :contributor do 
   can :manage, Vehicle do |model, user|
       model.owner == user
   end
end

But Vehicle is a superclass, maybe there's also Car and Bicycle. And sometimes we want to check on the generic class of a sub-class: can? :create, Bicycle -- right now this will be false, but we wish it would be true. We can of course everywhere instead ask can? :create, Vehicle, which would be true, but we're actually in a certain case checking specifically for Bicycle, maybe in the future there will be a role that can only create Bicycles but not Cars.

Yep, this has come up in a real project. Right now, we just need to remember "out of band", "You have to ask can? :create, Vehicle, it won't work if you ask can? :create, Bicycle" -- and the semantics are for now sufficient for us (maybe not in the future), but I don't like that you always have to remember this and if you slip up and ask for Bicycle you'll get a wrong answer. It's a weird extra non-obvious convention to remember, for now -- and if in the future we change to have some people that can universally manage Bicycle but not Car, then we have to change convention again and change all the can? calls.

By the way, i really loooove access-granted. It is so simple and composeable and extendable while being so flexible and powerful.

Anyway, as you see, PR filed. I have no opinion on backwards compat/major version... while I can see the argument it is technically a backwards incompat, I have trouble imagining a real scenario that it would break... but I guess I reluctantly admit that is not a great argument for not considering it a backwards incompat.

Hmm, if I really needed to work around this in present access-granted, I could define my own wrapper class that redefines ==.... when I get into the details it's not quite as trivial as my first fantasy to keep everything working, but I guess it could be done.

[pokonski]

Thanks for the detailed expalnation, maybe making a major bump for this is indeed an overkill 😬

I'll release a new version most likely next week, I noticed some possible performance improvements around the code you modified so I'll tweak stuff a bit after merging this :)

By the way, i really loooove access-granted. It is so simple and composeable and extendable while being so flexible and powerful.

I love to hear that 😁

[jrochkind]

Any further thoughts, @pokonski?